Skip to content

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

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

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

Conversation

@keenzarate213

@keenzarate213 keenzarate213 commented Aug 4, 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.

Note

This is the edu_edfi_source version of the same CI tool in edu_wh here: edanalytics/edu_wh#263

For each dialect, the workflow:

  • Compiles every model and test in this project (package:edu_edfi_source) 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 none do.
  • Enables tpdm, tpdmcommunity, and sedm domain models, which are disabled by default, so they get linted too.
  • Lints the compiled SQL with sqlfluff and reports pass/fail counts split between models and tests.

Also I added a check for known Databricks incompatible function that sqlfluff can't detect on its own, since it only validates syntax and doesn't
detect whether a given function or type actually exists in a given dialect. Currently checks for try_to_date() and casting as time, both
with no Databricks equivalent. This lives as a list directly in sql_dialect_linter.sh (databricks_incompatible_patterns) so new ones are a one-line addition.

Versions (sqlfluff, dbt-core, dbt-databricks, dbt-snowflake) are pinned in .github/workflows/requirements/sql-linter.txt.

Also had to:

  • Raise sqlfluff's max_parse_depth (default 255 -> 2000). This is to parse deeply nested logics, I was having issues with some models that have long chains conditions and nested calls.
  • Add noqa: PRS (see documentation here) workaround (scoped to a single known macro) for an unsupported sqlfluff parsing gap around Databricks' variant_explode syntax.
  • Add caching of virtualenv and dbt_packages in the workflow so dependencies don't get reinstalled/re-downloaded on every push in the branch.

Pressure testing the linter against the models surfaced a few Databricks incompatibilities, fixed as part of this PR (see below).

Breaking changes introduced by this PR:

No changes to CI/tooling behavior for anything already working. Though 3 small model fixes below do change the output column type on TPDM models (disabled by default via src:domain:tpdm:enabled), bringing them in line with the type already used everywhere else in the codebase for the similar field not expected to change behavior for any current implementation, but just flagging here since it IS a type change:

  • certification_exam_score: number -> float
  • actual_time: time -> string
  • 6 columns in stg_tpdm__candidates__other_names: varchar -> string

PR Merge Priority:

  • Low
  • Medium
  • High

Changes to existing files:

Important

  • base_tpdm__certification_exam_results :
    - certificationExamScore cast from ::number to ::float. NUMBER isn't a valid Databricks type; ::float.
  • base_tpdm__performance_evaluation_ratings :
    - actualTime cast from ::time to ::string. Databricks has no TIME type. so we match with existing similar field and they are casted to ::string.
  • stg_tpdm__candidates__other_names :
    - all ::varchar casts changed to ::string. sqlfluff was erroring and found that Databricks doesn't let you cast to VARCHAR in a query at all
  • stg_ef3__staff_school_associations, stg_ef3__student_discipline_incident_behavior_associations, stg_ef3__student_discipline_incident_non_offender_associations :
    - replaced dbt_utils.star() with edu_edfi_source.star(), since dbt_utils.star() requires live warehouse introspection to resolve columns and can't compile against the linter's dummy profile + we should just align it with everything else.
  • .sqlfluff : added max_parse_depth = 2000

New files created:

  • .github/scripts/sql_dialect_linter.sh: compiles + lints edu_edfi_source 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:

  • Ran the linter locally for both dialects: 269/269 models and 93/93 tests pass for Snowflake, and 269/269 models and 93/93 tests pass for Databricks.
  • Pressure tested the linter by temporarily reintroducing the 3 known-bad casts; confirmed it correctly flags each one (except try_to_date(), which needed the explicit pattern check since sqlfluff doesn't verify whether a function actually exists in a given dialect).

see most recent run here: https://github.com/edanalytics/edu_edfi_source/actions/runs/30939902150

@keenzarate213
keenzarate213 requested a review from rlittle08 August 4, 2026 21:01
@keenzarate213
keenzarate213 marked this pull request as ready for review August 4, 2026 21:01
@keenzarate213 keenzarate213 changed the title Feature/auto linting Build SQL dialect auto linter / add in github workflows Aug 4, 2026
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.

2 participants