diff --git a/.github/scripts/sql_dialect_linter.sh b/.github/scripts/sql_dialect_linter.sh new file mode 100644 index 00000000..32f1a7e7 --- /dev/null +++ b/.github/scripts/sql_dialect_linter.sh @@ -0,0 +1,291 @@ +#!/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 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 %} + +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 %} +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, +# 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 \ + | 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 +} > 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 . -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_]+['\"]" . --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. +# +# 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) +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_edfi_source \ + "${exclude_flags[@]}" \ + --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 + + # 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 + +# 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/" +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_edfi_source/} + is_test=false + [[ "$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' "$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 + 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 [[ ${#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 +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 +exit 1 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..cf9867dd --- /dev/null +++ b/.github/workflows/sql_dialect_linter.yml @@ -0,0 +1,60 @@ +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" + + # 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 + 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') }} + + # 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 }}) + run: bash .github/scripts/sql_dialect_linter.sh ${{ matrix.dialect }} 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/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 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