Skip to content

Build SQL dialect auto linter / add in github workflows - #263

Open
keenzarate213 wants to merge 30 commits into
mainfrom
feature/auto_linting
Open

Build SQL dialect auto linter / add in github workflows#263
keenzarate213 wants to merge 30 commits into
mainfrom
feature/auto_linting

Conversation

@keenzarate213

@keenzarate213 keenzarate213 commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Description & motivation

Adds a new CI check (sql_dialect_linter) that catches Databricks/Snowflake SQL
incompatibilities in the PR.

For each dialect, the workflow:

  • Compiles every model and test in the edu_wh package against a dummy dbt
    profile and dummy macro overrides for anything that
    would require a database connection during compile (e.g. is_incremental,
    get_column_values, union_relations).
  • Auto detects and excludes any model/test that still needs a warehouse
    connection to compile; currently
    bld_ef3__student_programs, bld_ef3__student_indicators,
    bld_ef3__student_assessments_long_results, and cfg_assessment_scores.
  • Enables tpdm_warehouse and finance_warehouse, which are disabled by default, so
    they get checked linted as well.
  • Lints the compiled SQL with sqlfluff and reports pass/fail counts split between
    models and tests.

Also added a check for known Databricks-incompatible function/type usage that
sqlfluff can't catch on its own since it only validates syntax, doesnt detect
a given function or type actually exists. Currently checks for
try_to_date() and casting as time, both valid syntax everywhere but with no
Databricks equivalent. I added a list directly in sql_dialect_linter.sh
(databricks_incompatible_patterns) so new ones are a one-line addition (we could also maybe store in a yaml?)

Versions (sqlfluff, dbt-core, dbt-databricks, dbt-snowflake) are pinned in
.github/workflows/requirements/sql-linter.txt.
Also had to add noqa: PRS (see documntation here) workaround (scoped to a single known macro) for an unsupported sqlfluff parsing gap around Databricks' variant_explode syntax,
and also allow for caching of virtualenv and dbt_packages in workflow so dependencies don't get
reinstalled/re-downloaded on every push in the branch.

Breaking changes introduced by this PR:

None, this is a new CI/tooling, no changes to warehouse models or logic.

PR Merge Priority:

  • Low
  • Medium
  • High

Changes to existing files:

New files created:

  • .github/scripts/sql_dialect_linter.sh: compiles + lints edu_wh against a dummy
    Databricks/Snowflake profile for the given dialect argument.
  • .github/workflows/sql_dialect_linter.yml: runs the above on PRs for both dialects.
  • .github/workflows/requirements/sql-linter.txt: pinned tool versions

Tests and QC done:

  • Did a pressure test on a couple models, linter successfuly detected Databricks incompatibility (except for the try_to_date() function ~ which I had to add as a specific flag to error out because sqlfluff doesn't really check whether or not a function exist in a dialect

edu_wh PR Review Checklist:

Make sure the following have been completed before approving this PR:

  • Description of changes has been added to Unreleased section of CHANGELOG.md. Add under ## New Features for features, etc.
  • Code has been tested/checked for Databricks and Snowflake compatibility - EA engineers see Databricks checklist here
  • Reviewer confirms the grain of all tables are unchanged, OR any changes are expected, communicated, and this PR is flagged as a breaking change (not for patch release)
  • If a new configuration xwalk was added:
    • The code is written such that the xwalk is optional (preferred), and this behavior was tested, OR
    • The code is written such that the xwalk is required, and the required xwalk is added to edu_project_template, and this PR is flagged as breaking change (not for patch release)
    • A description for the new xwalk has been added to EDU documentation site here
  • If a new configuration variable was added:
    • The code is written such that the variable is optional (preferred), and this behavior was tested, OR
    • The code is written such that the variable is required, and a default value was added to edu_project_template, and this PR is flagged as breaking change (not for patch release)
    • A description for the new variable has been added to EDU documentation site here

@keenzarate213
keenzarate213 requested a review from rlittle08 July 24, 2026 20:15
@keenzarate213 keenzarate213 changed the title Feature/auto linting Build SQL dialect auto linter / add in github workflows Aug 3, 2026
@keenzarate213
keenzarate213 marked this pull request as ready for review August 3, 2026 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant