From b0750708c784a31852bb51fdb0ac89060dc71085 Mon Sep 17 00:00:00 2001 From: Keen Zarate Date: Fri, 19 Jun 2026 14:46:07 -0500 Subject: [PATCH 01/11] linter draft --- .sqlfluff | 19 +++++++++++++++++++ dbt_lint.sh | 47 +++++++++++++++++++++++++++++++++++++++++++++++ requirements.txt | 4 ++++ 3 files changed, 70 insertions(+) create mode 100644 .sqlfluff create mode 100644 dbt_lint.sh create mode 100644 requirements.txt diff --git a/.sqlfluff b/.sqlfluff new file mode 100644 index 00000000..8a7113b6 --- /dev/null +++ b/.sqlfluff @@ -0,0 +1,19 @@ +[sqlfluff] +# global default +templater = dbt +dialect = snowflake +exclude_rules = layout, convention, capitalisation, aliasing, references, ambiguous, structure + +# Settings for --templater dbt +[sqlfluff:templater:dbt] +# If dbt compilation fails on a model, skip it rather than erroring out. +dbt_skip_compilation_error = True + +[sqlfluff:templater:jinja] +apply_dbt_builtins = true +load_macros_from_path = macros + +# Stubs only for dbt_utils macros, those live in dbt_packages +[sqlfluff:templater:jinja:macros] +generate_surrogate_key = {% macro generate_surrogate_key(fields) %}null{% endmacro %} +deduplicate = {% macro deduplicate(relation, partition_by, order_by) %}select * from stub_relation{% endmacro %} diff --git a/dbt_lint.sh b/dbt_lint.sh new file mode 100644 index 00000000..456cff05 --- /dev/null +++ b/dbt_lint.sh @@ -0,0 +1,47 @@ +dbt_lint() { + local templater="raw" # compiled SQL is plain SQL + local dialect="snowflake" # override with --dialect databricks or other to check compatibility + + while [[ "$1" == --* ]]; do # consume any leading --flag value pairs + case "$1" in + --templater) templater="$2"; shift 2 ;; + --dialect) dialect="$2"; shift 2 ;; + esac + done + + [[ $# -eq 0 ]] && { echo "Usage: dbt_lint [--dialect X] [--templater X] model1 [model2 ...]"; return 1; } + + echo "Compiling: $*" + local compile_log + compile_log=$(dbt compile --select "$@" --quiet 2>&1) || { + echo "dbt compile failed:" + echo "$compile_log" + return 1 + } + + local paths=() + for model in "$@"; do + local found + found=$(find -L ./target/compiled -name "${model}.sql" 2>/dev/null | head -1) + [[ -z "$found" ]] && { echo "ERROR: compiled file not found for '${model}'"; continue; } + paths+=("$found") + done + + [[ ${#paths[@]} -eq 0 ]] && return 1 + + local config + config="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/.sqlfluff" + + echo "Linting: ${paths[*]}" + sqlfluff lint \ + --config "$config" \ + --templater "$templater" \ + --dialect "$dialect" \ + "${paths[@]}" + + if [[ $? -eq 0 ]]; then + echo "Compatible with $dialect" + else + echo "Not compatible with $dialect. See errors above!" + fi +} diff --git a/requirements.txt b/requirements.txt new file mode 100644 index 00000000..c4faabb9 --- /dev/null +++ b/requirements.txt @@ -0,0 +1,4 @@ +# Pinned so local versions match +# Install with: pip install -r requirements.txt +sqlfluff==3.3.1 +sqlfluff-templater-dbt==3.3.1 From a61fd6bc5626e5b23ac707eac42a845db37a9c72 Mon Sep 17 00:00:00 2001 From: Alex Chen Date: Wed, 29 Jul 2026 12:51:38 -0700 Subject: [PATCH 02/11] Update GH Actions to match current working state on edu_wh --- .github/scripts/sql_dialect_linter.sh | 273 ++++++++++++++++++ .github/workflows/requirements/sql-linter.txt | 7 + .github/workflows/sql_dialect_linter.yml | 38 +++ 3 files changed, 318 insertions(+) create mode 100644 .github/scripts/sql_dialect_linter.sh create mode 100644 .github/workflows/requirements/sql-linter.txt create mode 100644 .github/workflows/sql_dialect_linter.yml diff --git a/.github/scripts/sql_dialect_linter.sh b/.github/scripts/sql_dialect_linter.sh new file mode 100644 index 00000000..3c719d1b --- /dev/null +++ b/.github/scripts/sql_dialect_linter.sh @@ -0,0 +1,273 @@ +#!/usr/bin/env bash + +# Usage: lint_dbt.sh where dialect is `databricks` or `snowflake`. + +set -euo pipefail + +# Which warehouse are we pretending to be: databricks or snowflake? +dialect="${1:-}" +case "$dialect" in + databricks|snowflake) ;; + *) echo "usage: $0 " >&2; exit 2 ;; +esac + +# Read the project's name so we can build a fake connection for it below. +profile=$(grep "^profile:" dbt_project.yml | awk '{print $2}' | tr -d "'\"") +# Keep each dialect's compiled output in its own folder so they don't overwrite each other. +target_path="target/lint-${dialect}" +profiles_dir="${target_path}/dummy_profile" +mkdir -p "$profiles_dir" + +# per-dialect dummy profile. +if [[ "$dialect" == "databricks" ]]; then + cat > "$profiles_dir/profiles.yml" << EOF +$profile: + target: dry_run + outputs: + dry_run: + type: databricks + host: 127.0.0.1 + http_path: /sql/1.0/warehouses/dummy + token: dummy + schema: dummy + catalog: dummy + threads: 8 + connect_timeout: 1 + connect_retries: 0 + retry_all: false + connection_parameters: + _retry_stop_after_attempts_count: 1 + _socket_timeout: 3 +EOF +else + cat > "$profiles_dir/profiles.yml" << EOF +$profile: + target: dry_run + outputs: + dry_run: + type: snowflake + account: dummy-account + user: dummy + password: dummy + role: dummy + database: dummy + warehouse: dummy + schema: dummy + threads: 8 + connect_retries: 0 + connect_timeout: 3 + retry_all: false +EOF +fi + +# Each of these normally hits the real warehouse during compile, so replace +# them with fake versions that just return an empty result: +# get_filtered_columns_in_relation / unpivot -> list a real table's columns +# get_column_values -> select distinct values +# get_intervals_between -> a real date-diff query +# is_incremental -> checks if the table exists +# union_relations -> also lists a table's columns +cat > macros/_ci_lint_stub.sql << EOF +{% macro ${dialect}__get_filtered_columns_in_relation(from, except=[]) %} + {{ return([]) }} +{% endmacro %} + +{% macro ${dialect}__union_relations(relations, column_override=none, include=[], exclude=[], source_column_name='_dbt_source_relation', where=none) %} + {{ return('') }} +{% endmacro %} + +{% macro ${dialect}__get_column_values(table, column, order_by='count(*) desc', max_records=none, default=none, where=none) %} + {{ return(default if default is not none else []) }} +{% endmacro %} + +{% macro is_incremental() %} + {{ return(False) }} +{% endmacro %} + +{% macro ${dialect}__get_intervals_between(start_date, end_date, datepart) %} + {{ return(1) }} +{% endmacro %} + +{% macro ${dialect}__unpivot(relation=none, cast_to='varchar', exclude=none, remove=none, field_name='field_name', value_name='value', quote_identifiers=False) %} + select + {% for col in (exclude or []) %} + cast(null as {{ cast_to }}) as {{ col }}, + {% endfor %} + cast(null as {{ cast_to }}) as {{ field_name }}, + cast(null as {{ cast_to }}) as {{ value_name }} + where false +{% endmacro %} + +{#- edu_edfi_source's own databricks__json_flatten emits "lateral variant_explode(...)", +which is real, working Databricks SQL (Databricks' variant type + lateral table +functions), but sqlfluff's databricks dialect can't parse it yet. Reproduce the +exact same SQL and just tell sqlfluff to skip the parse check on that line -#} +{% macro databricks__json_flatten(column, alias, outer) -%} +, lateral variant_explode{% if outer %}_outer{% endif %}({{ column }}) {% if alias != '' %} as {{ alias }} {% endif %} -- noqa: PRS +{%- endmacro %} +EOF + + +# get_single_value (this repo) and extract_descriptor (edu_edfi_source) also +# run real queries against the warehouse, but can't be faked the same way as +# above. So we replace their whole file with a simple version that skips the +# query and just returns a default answer. +cat > macros/get_single_value.sql << 'EOF' +{% macro get_single_value(query, default) %} + {{ return(default) }} +{% endmacro %} +EOF + +cat > dbt_packages/edu_edfi_source/macros/extract_descriptor.sql << 'EOF' +{% macro extract_descriptor(col, descriptor_name=None) -%} + split_part({{ col }}, '#', -1) +{%- endmacro %} +EOF + +# Generate a placeholder covering every resource name actually referenced, +# with a fake database/schema, so source() resolves without needing a real one. +resources=$(grep -rhoE "source_edfi3\(\s*['\"][a-zA-Z0-9_]+['\"]" \ + dbt_packages/edu_edfi_source/models dbt_packages/edu_edfi_source/macros 2>/dev/null \ + | sed -E "s/source_edfi3\(\s*['\"]//; s/['\"]//" | sort -u) + +{ + echo "sources:" + echo " - name: raw_edfi_3" + echo " database: dummy" + echo " schema: dummy_raw" + echo " tables:" + echo " - name: _deletes" + for r in $resources; do + echo " - name: $r" + done +} > dbt_packages/edu_edfi_source/models/_ci_lint_sources.yml + +# Generate an empty placeholder seed for every ref() that isn't defined +# anywhere else, so ref() resolves without needing the real crosswalk data. +mkdir -p seeds +{ + find . dbt_packages/edu_edfi_source -iname "*.sql" -path "*/models/*" 2>/dev/null | sed -E 's#.*/##; s/\.sql$//' + find . dbt_packages/edu_edfi_source -iname "*.csv" -path "*/seeds/*" 2>/dev/null | sed -E 's#.*/##; s/\.csv$//' +} | sort -u > /tmp/_ci_defined_nodes.txt + +grep -rhoE "ref\(['\"][a-zA-Z0-9_]+['\"]" . dbt_packages/edu_edfi_source --include="*.sql" 2>/dev/null \ + | sed -E "s/ref\(['\"]//; s/['\"]//" | sort -u > /tmp/_ci_all_refs.txt + +for missing in $(comm -23 /tmp/_ci_all_refs.txt /tmp/_ci_defined_nodes.txt); do + printf 'placeholder\nx\n' > "seeds/${missing}.csv" +done + +# Some models talk to the warehouse directly for real data or column info, +# so they can't compile against a fake connection. Any OTHER kind of failure (a real bug, a missing +# config) stops the script. +# +# fct_student_program_service also needs an "extensions" answer for each of +# these 7 program names, or it errors out asking for a setting that's +# normally supplied by implementation. +# +# tpdm_warehouse and finance_warehouse (and their edu_edfi_source staging +# models) are disabled by default. turn them on so they get linted too. +models_needing_warehouse=() +tests_needing_warehouse=() +compile_log=$(mktemp) +trap 'rm -f "$compile_log"' EXIT + +while true; do + exclude_flags=() + all_excluded=("${models_needing_warehouse[@]}" "${tests_needing_warehouse[@]}") + [[ ${#all_excluded[@]} -gt 0 ]] && exclude_flags=(--exclude "${all_excluded[@]}") + + dbt compile --no-introspect --no-populate-cache \ + --select package:edu_wh \ + "${exclude_flags[@]}" \ + --vars '{"edu:tpdm:enabled": true, "src:domain:tpdm:enabled": true, "src:domain:tpdmcommunity:enabled": true, "edu:finance:enabled": true, "src:domain:finance:enabled": true, "extensions": { + "stg_ef3__stu_spec_ed__program_services": {}, + "stg_ef3__stu_lang_instr__program_services": {}, + "stg_ef3__stu_homeless__program_services": {}, + "stg_ef3__stu_title_i_part_a__program_services": {}, + "stg_ef3__stu_cte__program_services": {}, + "stg_ef3__stu_migrant_edu__program_services": {}, + "stg_ef3__stu_school_food_service__program_services": {} + }}' \ + --profiles-dir "$profiles_dir" --target dry_run --target-path "$target_path" \ + > "$compile_log" 2>&1 && break + + # dbt's error looks like "Runtime Error in model some_model_name (path/to/file.sql)" + # or "...in test some_test_name (...)". Pull out both the node type and its name. + culprit="" + culprit_type="" + grep -q "connection never acquired for thread" "$compile_log" \ + && culprit_type=$(grep -oE "Runtime Error in (model|test)" "$compile_log" | head -1 | awk '{print $NF}') \ + && culprit=$(grep -oE "Runtime Error in (model|test) [a-zA-Z0-9_]+" "$compile_log" | head -1 | awk '{print $NF}') + + if [[ -z "$culprit" ]] || [[ " ${all_excluded[*]} " == *" $culprit "* ]]; then + echo "❌ dbt compile failed:" + cat "$compile_log" + exit 1 + fi + + echo "⚠️ $culprit needs a live warehouse connection to compile, adding $culprit to the exclude list and recompiling" + if [[ "$culprit_type" == "test" ]]; then + tests_needing_warehouse+=("$culprit") + else + models_needing_warehouse+=("$culprit") + fi +done + +# Lint each compiled model/test on its own so one failure doesn't stop the +# rest. Passing ones print one line; failing ones get a collapsible group +# with the full sqlfluff output inside. Generic tests (auto-generated from +# schema .yml files, like unique/not_null) compile into a ".yml/" +# subfolder — split those out from real models so the counts mean what they say. +mapfile -d '' -t compiled_files < <(find "$target_path/compiled" -path "*/edu_wh/models/*" -name "*.sql" -print0) +model_total=0; model_pass=0; model_fail=0; model_failed=() +test_total=0; test_pass=0; test_fail=0; test_failed=() +for f in "${compiled_files[@]}"; do + name=$(basename "$f") + path=${f#*/edu_wh/} + is_test=false + [[ "$path" == *.yml/* ]] && is_test=true + + if out=$(sqlfluff lint --config .sqlfluff --templater raw --dialect "$dialect" "$f" 2>&1); then + echo "Linting $path ✅" + if $is_test; then test_pass=$((test_pass + 1)); else model_pass=$((model_pass + 1)); fi + else + printf '::group::Linting %s ❌\n%s\n::endgroup::\n' "$path" "$out" + if $is_test; then + test_fail=$((test_fail + 1)); test_failed+=("${name%.sql}") + else + model_fail=$((model_fail + 1)); model_failed+=("${name%.sql}") + fi + fi + if $is_test; then test_total=$((test_total + 1)); else model_total=$((model_total + 1)); fi +done + +echo "" +model_known=$((model_total + ${#models_needing_warehouse[@]})) +printf '✅ %d/%d models compatible with %s\n' "$model_pass" "$model_known" "$dialect" +if [[ ${#models_needing_warehouse[@]} -gt 0 ]]; then + printf '⚠️ %d of those %d cannot be compiled here and requires live warehouse, lint these models locally instead:\n' "${#models_needing_warehouse[@]}" "$model_known" + printf ' - %s\n' "${models_needing_warehouse[@]}" +fi + +echo "" +test_known=$((test_total + ${#tests_needing_warehouse[@]})) +printf '✅ %d/%d tests compatible with %s\n' "$test_pass" "$test_known" "$dialect" +if [[ ${#tests_needing_warehouse[@]} -gt 0 ]]; then + printf '⚠️ %d of those %d cannot be compiled here and requires live warehouse, lint these tests locally instead:\n' "${#tests_needing_warehouse[@]}" "$test_known" + printf ' - %s\n' "${tests_needing_warehouse[@]}" +fi + +if [[ ${#model_failed[@]} -eq 0 && ${#test_failed[@]} -eq 0 ]]; then + exit 0 +fi +echo "" +if [[ ${#model_failed[@]} -gt 0 ]]; then + printf '❌ %d models are NOT compatible with %s:\n' "${#model_failed[@]}" "$dialect" + printf ' - %s\n' "${model_failed[@]}" +fi +if [[ ${#test_failed[@]} -gt 0 ]]; then + printf '❌ %d tests are NOT compatible with %s:\n' "${#test_failed[@]}" "$dialect" + printf ' - %s\n' "${test_failed[@]}" +fi +exit 1 \ No newline at end of file diff --git a/.github/workflows/requirements/sql-linter.txt b/.github/workflows/requirements/sql-linter.txt new file mode 100644 index 00000000..e777449b --- /dev/null +++ b/.github/workflows/requirements/sql-linter.txt @@ -0,0 +1,7 @@ +# Pinned so CI and local dev always use the same versions. +# Install with: pip install -r .github/workflows/requirements/sql-linter.txt +sqlfluff==4.2.2 +sqlfluff-templater-dbt==4.2.2 +dbt-core==1.10.19 +dbt-databricks==1.10.19 +dbt-snowflake==1.10.8 diff --git a/.github/workflows/sql_dialect_linter.yml b/.github/workflows/sql_dialect_linter.yml new file mode 100644 index 00000000..a74bf12c --- /dev/null +++ b/.github/workflows/sql_dialect_linter.yml @@ -0,0 +1,38 @@ +name: sql_dialect_linter + +on: + pull_request: + # Adds a manual "Run workflow" button in the Actions tab. GitHub only + # shows that button after this file is merged to main. + workflow_dispatch: + +jobs: + lint: + name: sql_dialect_linter (${{ matrix.dialect }}) + runs-on: ubuntu-latest + timeout-minutes: 20 + strategy: + # Run both dialects even if one fails, so we get both results. + fail-fast: false + matrix: + dialect: [databricks, snowflake] + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-python@v5 + with: + python-version: "3.11" + + # Everything (sqlfluff, dbt-core, both adapters) is pinned in this one + # file so CI always matches local dev — an older sqlfluff previously + # produced parsing failures that this pinned version doesn't have. + - name: Install dbt + sqlfluff + run: pip install -r .github/workflows/requirements/sql-linter.txt + + # The lint script edits files inside dbt_packages/edu_edfi_source, + # so those need to be downloaded first. + - name: dbt deps + run: dbt deps + + - name: Compile and lint (${{ matrix.dialect }}) + run: bash .github/scripts/sql_dialect_linter.sh ${{ matrix.dialect }} From ef25c921ca716e031b429de5530a87fdc7a2704c Mon Sep 17 00:00:00 2001 From: Alex Chen Date: Wed, 29 Jul 2026 13:09:04 -0700 Subject: [PATCH 03/11] Fix references to edu_edfi_source as child dbt package --- .github/scripts/sql_dialect_linter.sh | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/.github/scripts/sql_dialect_linter.sh b/.github/scripts/sql_dialect_linter.sh index 3c719d1b..157126e7 100644 --- a/.github/scripts/sql_dialect_linter.sh +++ b/.github/scripts/sql_dialect_linter.sh @@ -118,7 +118,7 @@ cat > macros/get_single_value.sql << 'EOF' {% endmacro %} EOF -cat > dbt_packages/edu_edfi_source/macros/extract_descriptor.sql << 'EOF' +cat > macros/extract_descriptor.sql << 'EOF' {% macro extract_descriptor(col, descriptor_name=None) -%} split_part({{ col }}, '#', -1) {%- endmacro %} @@ -127,7 +127,7 @@ EOF # Generate a placeholder covering every resource name actually referenced, # with a fake database/schema, so source() resolves without needing a real one. resources=$(grep -rhoE "source_edfi3\(\s*['\"][a-zA-Z0-9_]+['\"]" \ - dbt_packages/edu_edfi_source/models dbt_packages/edu_edfi_source/macros 2>/dev/null \ + models macros 2>/dev/null \ | sed -E "s/source_edfi3\(\s*['\"]//; s/['\"]//" | sort -u) { @@ -140,17 +140,17 @@ resources=$(grep -rhoE "source_edfi3\(\s*['\"][a-zA-Z0-9_]+['\"]" \ for r in $resources; do echo " - name: $r" done -} > dbt_packages/edu_edfi_source/models/_ci_lint_sources.yml +} > models/_ci_lint_sources.yml # Generate an empty placeholder seed for every ref() that isn't defined # anywhere else, so ref() resolves without needing the real crosswalk data. mkdir -p seeds { - find . dbt_packages/edu_edfi_source -iname "*.sql" -path "*/models/*" 2>/dev/null | sed -E 's#.*/##; s/\.sql$//' - find . dbt_packages/edu_edfi_source -iname "*.csv" -path "*/seeds/*" 2>/dev/null | sed -E 's#.*/##; s/\.csv$//' + find . -iname "*.sql" -path "*/models/*" 2>/dev/null | sed -E 's#.*/##; s/\.sql$//' + find . -iname "*.csv" -path "*/seeds/*" 2>/dev/null | sed -E 's#.*/##; s/\.csv$//' } | sort -u > /tmp/_ci_defined_nodes.txt -grep -rhoE "ref\(['\"][a-zA-Z0-9_]+['\"]" . dbt_packages/edu_edfi_source --include="*.sql" 2>/dev/null \ +grep -rhoE "ref\(['\"][a-zA-Z0-9_]+['\"]" . --include="*.sql" 2>/dev/null \ | sed -E "s/ref\(['\"]//; s/['\"]//" | sort -u > /tmp/_ci_all_refs.txt for missing in $(comm -23 /tmp/_ci_all_refs.txt /tmp/_ci_defined_nodes.txt); do @@ -270,4 +270,4 @@ if [[ ${#test_failed[@]} -gt 0 ]]; then printf '❌ %d tests are NOT compatible with %s:\n' "${#test_failed[@]}" "$dialect" printf ' - %s\n' "${test_failed[@]}" fi -exit 1 \ No newline at end of file +exit 1 From b56870a578fd448309b99785c82105dce878f16d Mon Sep 17 00:00:00 2001 From: Alex Chen Date: Wed, 29 Jul 2026 13:15:27 -0700 Subject: [PATCH 04/11] Fix duplicate databricks__json_flatten --- .github/scripts/sql_dialect_linter.sh | 35 ++++++++++++++++++++++++--- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/.github/scripts/sql_dialect_linter.sh b/.github/scripts/sql_dialect_linter.sh index 157126e7..736b5c55 100644 --- a/.github/scripts/sql_dialect_linter.sh +++ b/.github/scripts/sql_dialect_linter.sh @@ -97,13 +97,40 @@ cat > macros/_ci_lint_stub.sql << EOF cast(null as {{ cast_to }}) as {{ value_name }} where false {% endmacro %} +EOF + + +# edu_edfi_source's own databricks__json_flatten emits "lateral variant_explode(...)", +# which is real, working Databricks SQL (Databricks' variant type + lateral table +# functions), but sqlfluff's databricks dialect can't parse it yet. Reproduce the +# exact same SQL and just tell sqlfluff to skip the parse check on that line + +cat > macros/json_flatten.sql << EOF +{# +Flatten JSON arrays to rows. + +Arguments: + column: column name of array to flatten + alias: alias assigned to flattened object + outer: Keep rows with empty lists? Default false. +#} + +{% macro json_flatten(column, alias='', outer=False) %} + {{ return(adapter.dispatch('json_flatten', 'edu_edfi_source')(column, alias, outer)) }} +{% endmacro %} + +{% macro snowflake__json_flatten(column, alias, outer) -%} + +, lateral flatten(input=>{{ column }}, outer=>{{ outer }}) {% if alias != '' %} as {{ alias }} + {% endif %} + +{%- endmacro %} + -{#- edu_edfi_source's own databricks__json_flatten emits "lateral variant_explode(...)", -which is real, working Databricks SQL (Databricks' variant type + lateral table -functions), but sqlfluff's databricks dialect can't parse it yet. Reproduce the -exact same SQL and just tell sqlfluff to skip the parse check on that line -#} {% macro databricks__json_flatten(column, alias, outer) -%} + , lateral variant_explode{% if outer %}_outer{% endif %}({{ column }}) {% if alias != '' %} as {{ alias }} {% endif %} -- noqa: PRS + {%- endmacro %} EOF From e76443eb237d46eb46193e9f0442390a9eb0c6c3 Mon Sep 17 00:00:00 2001 From: Alex Chen Date: Wed, 29 Jul 2026 13:47:33 -0700 Subject: [PATCH 05/11] Test replacing the problematic dbt_utils.star call --- .../staging/edfi_3/stage/stg_ef3__staff_school_associations.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/staging/edfi_3/stage/stg_ef3__staff_school_associations.sql b/models/staging/edfi_3/stage/stg_ef3__staff_school_associations.sql index 1d109e7a..dac0f490 100644 --- a/models/staging/edfi_3/stage/stg_ef3__staff_school_associations.sql +++ b/models/staging/edfi_3/stage/stg_ef3__staff_school_associations.sql @@ -10,7 +10,7 @@ keyed as ( -- which means this table almost never contains year data, so we have to -- add it back in where possible coalesce(school_year, api_year) as school_year, - {{ dbt_utils.star(from=ref('base_ef3__staff_school_associations'), except=['school_year']) }} + {{ edu_edfi_source.star(from=ref('base_ef3__staff_school_associations'), except=['school_year']) }} {{ extract_extension(model_name=this.name, flatten=True) }} from base_staff_school_assoc From d3e8c1f694b4875706cb30f72298749030d1ce44 Mon Sep 17 00:00:00 2001 From: Alex Chen Date: Wed, 29 Jul 2026 13:51:51 -0700 Subject: [PATCH 06/11] Replace other dbt_utils.star calls --- ...ef3__student_discipline_incident_behavior_associations.sql | 2 +- ..._student_discipline_incident_non_offender_associations.sql | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/models/staging/edfi_3/stage/stg_ef3__student_discipline_incident_behavior_associations.sql b/models/staging/edfi_3/stage/stg_ef3__student_discipline_incident_behavior_associations.sql index 673e73b5..6b7b6f85 100644 --- a/models/staging/edfi_3/stage/stg_ef3__student_discipline_incident_behavior_associations.sql +++ b/models/staging/edfi_3/stage/stg_ef3__student_discipline_incident_behavior_associations.sql @@ -18,7 +18,7 @@ dedupe_base_student_discipline_incident as ( format_student_discipline_incident as ( -- note: the deprecated model needs to be flattened to match the grain of the new model select - {{ dbt_utils.star(ref('base_ef3__student_discipline_incident_associations'), + {{ edu_edfi_source.star(ref('base_ef3__student_discipline_incident_associations'), except=['student_participation_code', 'v_behaviors', 'v_ext', 'discipline_incident_reference', 'student_reference']) }}, value:behaviorDetailedDescription::string as behavior_detailed_description, {{ extract_descriptor('value:behaviorDescriptor::string') }} as behavior_type, diff --git a/models/staging/edfi_3/stage/stg_ef3__student_discipline_incident_non_offender_associations.sql b/models/staging/edfi_3/stage/stg_ef3__student_discipline_incident_non_offender_associations.sql index 6041d97b..1cd3b551 100644 --- a/models/staging/edfi_3/stage/stg_ef3__student_discipline_incident_non_offender_associations.sql +++ b/models/staging/edfi_3/stage/stg_ef3__student_discipline_incident_non_offender_associations.sql @@ -17,7 +17,7 @@ dedupe_base_student_discipline_incident as ( -- projects should only ever have one of the two models in use format_student_discipline_incident as ( select - {{ dbt_utils.star(ref('base_ef3__student_discipline_incident_associations'), + {{ edu_edfi_source.star(ref('base_ef3__student_discipline_incident_associations'), except=['student_participation_code', 'v_behaviors', 'v_ext']) }}, {{ json_array_agg( @@ -65,4 +65,4 @@ deduped as ( }} ) select * from deduped -where not is_deleted \ No newline at end of file +where not is_deleted From 9994db8c4a1b13c6b5a43d82cb5db7a021b70b1d Mon Sep 17 00:00:00 2001 From: Alex Chen Date: Wed, 29 Jul 2026 13:55:08 -0700 Subject: [PATCH 07/11] Update paths for edu_edfi_source --- .github/scripts/sql_dialect_linter.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/scripts/sql_dialect_linter.sh b/.github/scripts/sql_dialect_linter.sh index 736b5c55..0a4521f8 100644 --- a/.github/scripts/sql_dialect_linter.sh +++ b/.github/scripts/sql_dialect_linter.sh @@ -205,7 +205,7 @@ while true; do [[ ${#all_excluded[@]} -gt 0 ]] && exclude_flags=(--exclude "${all_excluded[@]}") dbt compile --no-introspect --no-populate-cache \ - --select package:edu_wh \ + --select package:edu_edfi_source \ "${exclude_flags[@]}" \ --vars '{"edu:tpdm:enabled": true, "src:domain:tpdm:enabled": true, "src:domain:tpdmcommunity:enabled": true, "edu:finance:enabled": true, "src:domain:finance:enabled": true, "extensions": { "stg_ef3__stu_spec_ed__program_services": {}, @@ -246,12 +246,12 @@ done # with the full sqlfluff output inside. Generic tests (auto-generated from # schema .yml files, like unique/not_null) compile into a ".yml/" # subfolder — split those out from real models so the counts mean what they say. -mapfile -d '' -t compiled_files < <(find "$target_path/compiled" -path "*/edu_wh/models/*" -name "*.sql" -print0) +mapfile -d '' -t compiled_files < <(find "$target_path/compiled" -path "*/edu_edfi_source/models/*" -name "*.sql" -print0) model_total=0; model_pass=0; model_fail=0; model_failed=() test_total=0; test_pass=0; test_fail=0; test_failed=() for f in "${compiled_files[@]}"; do name=$(basename "$f") - path=${f#*/edu_wh/} + path=${f#*/edu_edfi_source/} is_test=false [[ "$path" == *.yml/* ]] && is_test=true From 51307e0bbc1536a4354c7fb8cbcaad493b81e7f7 Mon Sep 17 00:00:00 2001 From: Keen Zarate <51969358+keenzarate213@users.noreply.github.com> Date: Tue, 4 Aug 2026 11:30:12 -0500 Subject: [PATCH 08/11] Update sql-linter.txt From d81074a0cb69adbc796a0184cb5d0ab178b525ed Mon Sep 17 00:00:00 2001 From: Keen Zarate <51969358+keenzarate213@users.noreply.github.com> Date: Tue, 4 Aug 2026 11:30:39 -0500 Subject: [PATCH 09/11] Update sql_dialect_linter.yml --- .github/workflows/sql_dialect_linter.yml | 34 +++++++++++++++++++----- 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/.github/workflows/sql_dialect_linter.yml b/.github/workflows/sql_dialect_linter.yml index a74bf12c..cf9867dd 100644 --- a/.github/workflows/sql_dialect_linter.yml +++ b/.github/workflows/sql_dialect_linter.yml @@ -23,15 +23,37 @@ jobs: with: python-version: "3.11" - # Everything (sqlfluff, dbt-core, both adapters) is pinned in this one - # file so CI always matches local dev — an older sqlfluff previously - # produced parsing failures that this pinned version doesn't have. + # Skip reinstalling everything if nothing's changed since last time. + - name: Cache virtualenv + id: cache-venv + uses: actions/cache@v6 + with: + path: .venv + key: venv-${{ runner.os }}-${{ hashFiles('.github/workflows/requirements/sql-linter.txt') }} + + # Installs the exact versions pinned in sql-linter.txt. Skipped + # entirely when the cache above already has a matching virtualenv. - name: Install dbt + sqlfluff - run: pip install -r .github/workflows/requirements/sql-linter.txt + if: steps.cache-venv.outputs.cache-hit != 'true' + run: | + python -m venv .venv + .venv/bin/pip install -r .github/workflows/requirements/sql-linter.txt + + - name: Put virtualenv on PATH + run: echo "$PWD/.venv/bin" >> "$GITHUB_PATH" + + # Skips the dbt deps step below entirely if packages.yml hasn't changed. + - name: Cache dbt packages + id: cache-dbt-packages + uses: actions/cache@v6 + with: + path: dbt_packages + key: dbt-packages-${{ hashFiles('packages.yml') }} - # The lint script edits files inside dbt_packages/edu_edfi_source, - # so those need to be downloaded first. + # Downloads edu_edfi_source and dbt_utils, unless the cache above + # already has them. - name: dbt deps + if: steps.cache-dbt-packages.outputs.cache-hit != 'true' run: dbt deps - name: Compile and lint (${{ matrix.dialect }}) From 27e458c83ff89bf5509458b2abcc162e52e3ae59 Mon Sep 17 00:00:00 2001 From: Keen Zarate Date: Tue, 4 Aug 2026 12:24:56 -0500 Subject: [PATCH 10/11] update sh with latest version --- .github/scripts/sql_dialect_linter.sh | 115 ++++++++++++-------------- dbt_lint.sh | 47 ----------- requirements.txt | 4 - 3 files changed, 53 insertions(+), 113 deletions(-) delete mode 100644 dbt_lint.sh delete mode 100644 requirements.txt diff --git a/.github/scripts/sql_dialect_linter.sh b/.github/scripts/sql_dialect_linter.sh index 0a4521f8..32f1a7e7 100644 --- a/.github/scripts/sql_dialect_linter.sh +++ b/.github/scripts/sql_dialect_linter.sh @@ -97,61 +97,43 @@ cat > macros/_ci_lint_stub.sql << EOF cast(null as {{ cast_to }}) as {{ value_name }} where false {% endmacro %} -EOF - - -# edu_edfi_source's own databricks__json_flatten emits "lateral variant_explode(...)", -# which is real, working Databricks SQL (Databricks' variant type + lateral table -# functions), but sqlfluff's databricks dialect can't parse it yet. Reproduce the -# exact same SQL and just tell sqlfluff to skip the parse check on that line -cat > macros/json_flatten.sql << EOF -{# -Flatten JSON arrays to rows. +{#- edu_edfi_source's databricks__json_flatten emits "lateral variant_explode(...)", +but sqlfluff's databricks dialect can't parse this for some reason, even though it +is an actual Databricks call. When a where clause follows directly after this variant_explode, +sqlfluff errors. Adding trailing comma to force sqlfluff to look at this line instead of the +where clause. This redefines the same macro as macros/json_flatten.sql below, which is fine +since that file only keeps the dispatch + snowflake__ versions #} +{% macro databricks__json_flatten(column, alias, outer) -%} +, lateral variant_explode{% if outer %}_outer{% endif %}({{ column }}) {% if alias != '' %} as {{ alias }} {% endif %}, -- noqa: PRS +{%- endmacro %} -Arguments: - column: column name of array to flatten - alias: alias assigned to flattened object - outer: Keep rows with empty lists? Default false. -#} +EOF +# json_flatten's databricks__ implementation is stubbed above (to work around +# the sqlfluff parse issue), so drop it here to avoid a duplicate macro +# definition; keep the dispatch macro and the snowflake__ implementation. +cat > macros/json_flatten.sql << 'EOF' {% macro json_flatten(column, alias='', outer=False) %} {{ return(adapter.dispatch('json_flatten', 'edu_edfi_source')(column, alias, outer)) }} {% endmacro %} {% macro snowflake__json_flatten(column, alias, outer) -%} - -, lateral flatten(input=>{{ column }}, outer=>{{ outer }}) {% if alias != '' %} as {{ alias }} - {% endif %} - -{%- endmacro %} - - -{% macro databricks__json_flatten(column, alias, outer) -%} - -, lateral variant_explode{% if outer %}_outer{% endif %}({{ column }}) {% if alias != '' %} as {{ alias }} {% endif %} -- noqa: PRS - +, lateral flatten(input=>{{ column }}, outer=>{{ outer }}) {% if alias != '' %} as {{ alias }} {% endif %} {%- endmacro %} EOF - -# get_single_value (this repo) and extract_descriptor (edu_edfi_source) also -# run real queries against the warehouse, but can't be faked the same way as -# above. So we replace their whole file with a simple version that skips the -# query and just returns a default answer. -cat > macros/get_single_value.sql << 'EOF' -{% macro get_single_value(query, default) %} - {{ return(default) }} -{% endmacro %} -EOF - +# extract_descriptor also runs a real query against the warehouse (via +# run_query), which can't use a dummy the same way as above. So replace its +# whole file with a simple version that skips the query and just returns +# the raw descriptor code. cat > macros/extract_descriptor.sql << 'EOF' {% macro extract_descriptor(col, descriptor_name=None) -%} split_part({{ col }}, '#', -1) {%- endmacro %} EOF -# Generate a placeholder covering every resource name actually referenced, +# Generate a placeholder covering every resource name actually referenced, # with a fake database/schema, so source() resolves without needing a real one. resources=$(grep -rhoE "source_edfi3\(\s*['\"][a-zA-Z0-9_]+['\"]" \ models macros 2>/dev/null \ @@ -188,12 +170,8 @@ done # so they can't compile against a fake connection. Any OTHER kind of failure (a real bug, a missing # config) stops the script. # -# fct_student_program_service also needs an "extensions" answer for each of -# these 7 program names, or it errors out asking for a setting that's -# normally supplied by implementation. -# -# tpdm_warehouse and finance_warehouse (and their edu_edfi_source staging -# models) are disabled by default. turn them on so they get linted too. +# tpdm, tpdmcommunity, and sedm staging models are disabled by default. +# turn them on so they get linted too. models_needing_warehouse=() tests_needing_warehouse=() compile_log=$(mktemp) @@ -207,15 +185,7 @@ while true; do dbt compile --no-introspect --no-populate-cache \ --select package:edu_edfi_source \ "${exclude_flags[@]}" \ - --vars '{"edu:tpdm:enabled": true, "src:domain:tpdm:enabled": true, "src:domain:tpdmcommunity:enabled": true, "edu:finance:enabled": true, "src:domain:finance:enabled": true, "extensions": { - "stg_ef3__stu_spec_ed__program_services": {}, - "stg_ef3__stu_lang_instr__program_services": {}, - "stg_ef3__stu_homeless__program_services": {}, - "stg_ef3__stu_title_i_part_a__program_services": {}, - "stg_ef3__stu_cte__program_services": {}, - "stg_ef3__stu_migrant_edu__program_services": {}, - "stg_ef3__stu_school_food_service__program_services": {} - }}' \ + --vars '{"src:domain:tpdm:enabled": true, "src:domain:tpdmcommunity:enabled": true, "src:domain:sedm:enabled": true}' \ --profiles-dir "$profiles_dir" --target dry_run --target-path "$target_path" \ > "$compile_log" 2>&1 && break @@ -241,11 +211,17 @@ while true; do fi done -# Lint each compiled model/test on its own so one failure doesn't stop the -# rest. Passing ones print one line; failing ones get a collapsible group -# with the full sqlfluff output inside. Generic tests (auto-generated from +# sqlfluff only catches syntax that doesn't parse, it can't tell that a +# function or type doesn't exist in that dialect, so check for +# these known Databricks gaps by name instead. Add new ones here as they come up. +databricks_incompatible_patterns=( + 'try_to_date\s*\(' # function has no Databricks equivalent + '\bas\s+time\b' # Databricks has no TIME datatype +) +incompatible_regex=$(IFS='|'; echo "${databricks_incompatible_patterns[*]}") + +# Lint each compiled model/test. Generic tests (auto-generated from # schema .yml files, like unique/not_null) compile into a ".yml/" -# subfolder — split those out from real models so the counts mean what they say. mapfile -d '' -t compiled_files < <(find "$target_path/compiled" -path "*/edu_edfi_source/models/*" -name "*.sql" -print0) model_total=0; model_pass=0; model_fail=0; model_failed=() test_total=0; test_pass=0; test_fail=0; test_failed=() @@ -256,10 +232,25 @@ for f in "${compiled_files[@]}"; do [[ "$path" == *.yml/* ]] && is_test=true if out=$(sqlfluff lint --config .sqlfluff --templater raw --dialect "$dialect" "$f" 2>&1); then + sqlfluff_ok=0 + else + sqlfluff_ok=1 + fi + + bad_function="" + if [[ "$dialect" == "databricks" ]]; then + match=$(grep -inE "$incompatible_regex" "$f" | head -1 || true) + [[ -n "$match" ]] && bad_function="No Databricks equivalent: $match" + fi + + if [[ $sqlfluff_ok -eq 0 && -z "$bad_function" ]]; then echo "Linting $path ✅" if $is_test; then test_pass=$((test_pass + 1)); else model_pass=$((model_pass + 1)); fi else - printf '::group::Linting %s ❌\n%s\n::endgroup::\n' "$path" "$out" + printf '::group::Linting %s ❌\n' "$path" + [[ -n "$bad_function" ]] && echo "$bad_function" + echo "$out" + echo "::endgroup::" if $is_test; then test_fail=$((test_fail + 1)); test_failed+=("${name%.sql}") else @@ -284,6 +275,10 @@ if [[ ${#tests_needing_warehouse[@]} -gt 0 ]]; then printf '⚠️ %d of those %d cannot be compiled here and requires live warehouse, lint these tests locally instead:\n' "${#tests_needing_warehouse[@]}" "$test_known" printf ' - %s\n' "${tests_needing_warehouse[@]}" fi +if [[ ${#test_failed[@]} -gt 0 ]]; then + printf '❌ %d tests are NOT compatible with %s:\n' "${#test_failed[@]}" "$dialect" + printf ' - %s\n' "${test_failed[@]}" +fi if [[ ${#model_failed[@]} -eq 0 && ${#test_failed[@]} -eq 0 ]]; then exit 0 @@ -293,8 +288,4 @@ if [[ ${#model_failed[@]} -gt 0 ]]; then printf '❌ %d models are NOT compatible with %s:\n' "${#model_failed[@]}" "$dialect" printf ' - %s\n' "${model_failed[@]}" fi -if [[ ${#test_failed[@]} -gt 0 ]]; then - printf '❌ %d tests are NOT compatible with %s:\n' "${#test_failed[@]}" "$dialect" - printf ' - %s\n' "${test_failed[@]}" -fi exit 1 diff --git a/dbt_lint.sh b/dbt_lint.sh deleted file mode 100644 index 456cff05..00000000 --- a/dbt_lint.sh +++ /dev/null @@ -1,47 +0,0 @@ -dbt_lint() { - local templater="raw" # compiled SQL is plain SQL - local dialect="snowflake" # override with --dialect databricks or other to check compatibility - - while [[ "$1" == --* ]]; do # consume any leading --flag value pairs - case "$1" in - --templater) templater="$2"; shift 2 ;; - --dialect) dialect="$2"; shift 2 ;; - esac - done - - [[ $# -eq 0 ]] && { echo "Usage: dbt_lint [--dialect X] [--templater X] model1 [model2 ...]"; return 1; } - - echo "Compiling: $*" - local compile_log - compile_log=$(dbt compile --select "$@" --quiet 2>&1) || { - echo "dbt compile failed:" - echo "$compile_log" - return 1 - } - - local paths=() - for model in "$@"; do - local found - found=$(find -L ./target/compiled -name "${model}.sql" 2>/dev/null | head -1) - [[ -z "$found" ]] && { echo "ERROR: compiled file not found for '${model}'"; continue; } - paths+=("$found") - done - - [[ ${#paths[@]} -eq 0 ]] && return 1 - - local config - config="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/.sqlfluff" - - echo "Linting: ${paths[*]}" - sqlfluff lint \ - --config "$config" \ - --templater "$templater" \ - --dialect "$dialect" \ - "${paths[@]}" - - if [[ $? -eq 0 ]]; then - echo "Compatible with $dialect" - else - echo "Not compatible with $dialect. See errors above!" - fi -} diff --git a/requirements.txt b/requirements.txt deleted file mode 100644 index c4faabb9..00000000 --- a/requirements.txt +++ /dev/null @@ -1,4 +0,0 @@ -# Pinned so local versions match -# Install with: pip install -r requirements.txt -sqlfluff==3.3.1 -sqlfluff-templater-dbt==3.3.1 From 4d64b58ed158fe28e4de133fa3d04f7c89d364d9 Mon Sep 17 00:00:00 2001 From: Keen Zarate Date: Tue, 4 Aug 2026 13:42:35 -0500 Subject: [PATCH 11/11] make adjustment on parse depth and then also update data type for some fields --- .sqlfluff | 3 +++ .../base/base_tpdm__certification_exam_results.sql | 2 +- .../base_tpdm__performance_evaluation_ratings.sql | 2 +- .../tpdm/stage/stg_tpdm__candidates__other_names.sql | 12 ++++++------ 4 files changed, 11 insertions(+), 8 deletions(-) diff --git a/.sqlfluff b/.sqlfluff index 8a7113b6..a7fd6e99 100644 --- a/.sqlfluff +++ b/.sqlfluff @@ -3,6 +3,9 @@ templater = dbt dialect = snowflake exclude_rules = layout, convention, capitalisation, aliasing, references, ambiguous, structure +# extend max parse depth default is 255 so we get +# error "Maximum parse depth exceeded" even though the SQL is valid. +max_parse_depth = 2000 # Settings for --templater dbt [sqlfluff:templater:dbt] diff --git a/models/staging/tpdm/base/base_tpdm__certification_exam_results.sql b/models/staging/tpdm/base/base_tpdm__certification_exam_results.sql index b2ce0362..92612fa6 100644 --- a/models/staging/tpdm/base/base_tpdm__certification_exam_results.sql +++ b/models/staging/tpdm/base/base_tpdm__certification_exam_results.sql @@ -21,7 +21,7 @@ renamed as ( -- non-identity components v:attemptNumber::int as attempt_number, v:certificationExamPassIndicator::boolean as certification_exam_pass_indicator, - v:certificationExamScore::number as certification_exam_score, + v:certificationExamScore::float as certification_exam_score, -- descriptors {{ extract_descriptor('v:certificationExamStatusDescriptor::string') }} as certification_exam_status_descriptor, {{ extract_descriptor('v:personReference:sourceSystemDescriptor::string') }} as person_source_system_descriptor, diff --git a/models/staging/tpdm/base/base_tpdm__performance_evaluation_ratings.sql b/models/staging/tpdm/base/base_tpdm__performance_evaluation_ratings.sql index ff26d3a8..dfa91fd3 100644 --- a/models/staging/tpdm/base/base_tpdm__performance_evaluation_ratings.sql +++ b/models/staging/tpdm/base/base_tpdm__performance_evaluation_ratings.sql @@ -22,7 +22,7 @@ renamed as ( {{ extract_descriptor('v:personReference:sourceSystemDescriptor::string') }} as source_system, {{ extract_descriptor('v:performanceEvaluationReference:termDescriptor::string')}} as academic_term, -- non-identity components - v:actualTime::time as actual_time, + v:actualTime::string as actual_time, v:scheduleDate::date as schedule_date, v:actualDate::date as actual_date, v:comments::string as comments, diff --git a/models/staging/tpdm/stage/stg_tpdm__candidates__other_names.sql b/models/staging/tpdm/stage/stg_tpdm__candidates__other_names.sql index 890e7947..0b793715 100644 --- a/models/staging/tpdm/stage/stg_tpdm__candidates__other_names.sql +++ b/models/staging/tpdm/stage/stg_tpdm__candidates__other_names.sql @@ -7,12 +7,12 @@ flattened as ( api_year, k_candidate, k_candidate_xyear, - {{ extract_descriptor('value:otherNameTypeDescriptor::varchar') }} as other_name_type, - value:personalTitlePrefix::varchar as personal_title_prefix, - value:firstName::varchar as first_name, - value:middleName::varchar as middle_name, - value:lastSurname::varchar as last_surname, - value:generationCodeSuffix::varchar as generation_code_suffix + {{ extract_descriptor('value:otherNameTypeDescriptor::string') }} as other_name_type, + value:personalTitlePrefix::string as personal_title_prefix, + value:firstName::string as first_name, + value:middleName::string as middle_name, + value:lastSurname::string as last_surname, + value:generationCodeSuffix::string as generation_code_suffix from candidates {{ json_flatten('v_other_names') }} )