Build SQL dialect auto linter / add in github workflows - #206
Open
keenzarate213 wants to merge 11 commits into
Open
Build SQL dialect auto linter / add in github workflows#206keenzarate213 wants to merge 11 commits into
keenzarate213 wants to merge 11 commits into
Conversation
keenzarate213
marked this pull request as ready for review
August 4, 2026 21:01
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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_sourceversion of the same CI tool in edu_wh here: edanalytics/edu_wh#263For each dialect, the workflow:
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).tpdm,tpdmcommunity, andsedmdomain models, which are disabled by default, so they get linted too.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 castingas time, bothwith 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:
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.noqa: PRS(see documentation here) workaround (scoped to a single known macro) for an unsupported sqlfluff parsing gap around Databricks'variant_explodesyntax.dbt_packagesin 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->floatactual_time:time->stringstg_tpdm__candidates__other_names:varchar->stringPR Merge Priority:
Changes to existing files:
Important
base_tpdm__certification_exam_results:-
certificationExamScorecast from::numberto::float.NUMBERisn't a valid Databricks type;::float.base_tpdm__performance_evaluation_ratings:-
actualTimecast from::timeto::string. Databricks has noTIMEtype. so we match with existing similar field and they are casted to::string.stg_tpdm__candidates__other_names:- all
::varcharcasts changed to::string. sqlfluff was erroring and found that Databricks doesn't let you cast to VARCHAR in a query at allstg_ef3__staff_school_associations,stg_ef3__student_discipline_incident_behavior_associations,stg_ef3__student_discipline_incident_non_offender_associations:- replaced
dbt_utils.star()withedu_edfi_source.star(), sincedbt_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: addedmax_parse_depth = 2000New files created:
.github/scripts/sql_dialect_linter.sh: compiles + lintsedu_edfi_sourceagainst 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:
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