#121: prune compiler Snowflake dialect support - #127
Conversation
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (4)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR implements complete Snowflake SQL code generation for the prune compiler by extending the ChangesSnowflake Dialect Support for Prune Compiler
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ields Add four BigQuery-defaulted declarative fields to the frozen Dialect dataclass (DEC-001 of #121): sample_row_hash_expr, timestamp_literal_template, date_literal_template, and quote_qualified_per_component. Defaults reproduce BigQuery SQL byte-for-byte so every existing construction site stays valid unedited. Set SNOWFLAKE_DIALECT's four values (DEC-002): ABS(HASH(*)), '{value}'::TIMESTAMP, '{value}'::DATE, per-component quoting. Leave POSTGRES_DIALECT at the BigQuery defaults with a docstring note (DEC-007) that they are corrected when the Postgres adapter's warehouse ops land. Refresh the Dialect class docstring to describe what the prune compiler reads each new field for. Add unit tests pinning the SNOWFLAKE_DIALECT values, the BIGQUERY_DIALECT defaults, and that constructing a Dialect with only the five original fields still succeeds. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Make signalforge.prune.compiler emit warehouse-correct SQL purely from the
Dialect value object — no branching on dialect name, no warehouse-SDK import
under prune/.
- _quote/_qualified_table_name now take the Dialect: fold identifiers per
identifier_case (upper/lower/preserve) before quoting (DEC-003); branch on
quote_qualified_per_component (Snowflake "DB"."SCH"."T" vs BigQuery whole-
path) (DEC-002).
- _render_sample_cte renders the hash-mod predicate from
dialect.sample_row_hash_expr (Snowflake ABS(HASH(*)) vs BigQuery
FARM_FINGERPRINT).
- _render_partition_filter renders datetime/date via the dialect's literal
templates (Snowflake '...'::TIMESTAMP vs BigQuery TIMESTAMP('...')).
- Thread dialect (not bare quote_char) through _wrap_with_sample_or_partition
and every _compile_* helper.
BigQuery's identifier_case="preserve" + quote_qualified_per_component=False
make the refactor a no-op for BigQuery: all 11 compiled_sql/*.sql snapshots
stay byte-identical (zero fixture edits) — the regression gate (DEC-001).
Adds tests/prune/test_compiler_import_guard.py (DEC-008): AST scan asserting
no snowflake / google.cloud import under src/signalforge/prune/, with a
planted-violation self-check and a no-false-positive check.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add byte-exact Snowflake snapshot fixtures (captured from real compiler
output with SNOWFLAKE_DIALECT) and snapshot tests covering the four
built-in test types (full + sample modes) and custom_sql (single-table
full, single-table sample, multi-table full-scan).
- 11 new fixtures under tests/fixtures/prune/compiled_sql/snowflake/:
"-quoted, per-component qualified names ("FAKE_PROJECT"."DATASET"."ORDERS"),
UPPER-folded identifiers, MOD(ABS(HASH(*)), <bucket>) < 1 sample predicate.
- Snapshot tests assert compiled == fixture (byte-exact) per variant.
- custom_sql single-table sample test confirms the #116 materialised-sample
substitution invariant under the Snowflake quote char: the body reads from
the `sample` CTE alias and never the source table.
- Guard tests: Snowflake fixtures contain `"` and never a backtick; sample
fixtures contain HASH(*) and never FARM_FINGERPRINT.
The 11 existing BigQuery fixtures are unchanged; compiler logic untouched
(US-002). Full validation green (2245 passed).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add `fakesnow` as a dev/test dependency behind a NEW gated `@pytest.mark.snowflake` marker and a gated test that feeds the compiler's real emitted Snowflake SQL into an in-memory fakesnow connection. - pyproject.toml: add `fakesnow>=0.9` to `[dependency-groups].dev` and `[project.optional-dependencies].dev` (kept in sync); register the `snowflake` marker; add `and not snowflake` to the default `addopts` deselection so the default `pytest` run does NOT collect it. - tests/prune/test_compiler_fakesnow.py: for each built-in test type (not_null / unique / accepted_values / relationships), compile the failing-rows SELECT with SNOWFLAKE_DIALECT, create a tiny matching fakesnow table, run the SQL wrapped as the adapter does (`SELECT COUNT(*) AS failures FROM (<sql>) AS t`), and assert it executes with the engineered failing-row shape. Determinism engineered by RULE SEMANTICS, not value-equality (testing-signal.md): a NULL row → failures >= 1, a duplicate → failures >= 1, etc. No HASH() value assertions. scope="full" only — fakesnow's DuckDB backend rejects the sample-mode `HASH(*)` predicate and the `sample` CTE name (DEC-005 caveat; sample-mode shape is gated by the US-003 byte-exact snapshots). Confirmed: default `uv run pytest` deselects all 8 snowflake tests; `uv run pytest -m snowflake --no-cov` runs them green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The deterministic-sample CTE was bound to the bare alias `sample`, but `SAMPLE` is a Snowflake reserved keyword (TABLESAMPLE) — `WITH sample AS (...)` is a syntax error on real Snowflake (confirmed via sqlglot). This is on the default `scope=sample` prune path, not an edge case. Fix: add `Dialect.sample_cte_alias` (BigQuery default bare `sample` → BQ snapshots byte-identical; Snowflake `"sample"` quoted to bypass the reserved word). Thread it through `_render_sample_cte`, the built-in `target`, the relationships `child_target`, and the custom_sql sample substitution. Regenerated the 5 Snowflake sample fixtures; added a gated sqlglot parse-guard over every Snowflake fixture (the guard that catches this class of bug, since fakesnow can't execute sample-mode SQL due to HASH(*)). Found during US-005 Quality Gate; surfaced by the US-004 fakesnow validation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ard hardening From the 4 code-review passes (no blocking bugs found beyond the already-fixed sample-CTE reserved-word bug): - Doc-count drift: the QG sample-CTE fix added a 5th Dialect field, so the "four fields" wording in the Dialect / POSTGRES docstrings and three test_models docstrings was stale -> "five". Added the missing sample_cte_alias default assertion to test_dialect_constructs_without_new_field_args. Clarified the POSTGRES note (sample_cte_alias="sample" is already Postgres-correct; quote_qualified_per_component + cast templates are what need correcting when Postgres ops land). - Import-guard hardening: the AST detector missed the namespace-split `from google import cloud` form (module='google' alone isn't forbidden but the imported name completes 'google.cloud'). Now flagged; planted self-check extended to 9 violations. Resolved the reviewers' split on string-literal escaping: Snowflake DOES honor backslash escapes (\') in single-quoted string literals, so the shared escape_bq_string_literal is valid for both dialects (no change). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…+ identifier_case graduation 5-surface graduation of identifier_case (declared -> active) + Snowflake compiler dialect documentation: - .claude/rules/prune-engine.md: full Dialect-field list the compiler reads; identifier_case graduation; sample_cte_alias reserved-word lesson (snapshots pin shape not validity — keep a parser/executor in the loop); supports_qualify stays forward-compat; new #121 Snowflake subsection + refs. - docs/prune-ops.md: Snowflake compiler dialect section (case-folding + residual, HASH() cross-version reproducibility caveat, QUALIFY not used, validation tiers); updated multi-warehouse deferral (Snowflake landed). - CLAUDE.md: public-API surface — compiler emits Snowflake SQL from SNOWFLAKE_DIALECT; five new Dialect fields; identifier_case graduated. - .claude/rules/warehouse-adapters.md: Dialect now carries prune-compiler SQL-fragment templates; populate them for future vendor dialects. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR operationalizes Snowflake-compatible SQL generation for the prune compiler by making warehouse-specific SQL emission fully dialect-driven via Dialect (rather than any warehouse SDK imports or branching on dialect name), while keeping BigQuery output byte-identical through defaults and snapshot regression gates.
Changes:
- Extended
Dialectwith declarative SQL-fragment fields (BigQuery-defaulted) and set Snowflake-specific values onSNOWFLAKE_DIALECT. - Refactored the prune compiler to consume the dialect fields for identifier folding/quoting, qualified-name rendering, sampling hash predicate, and date/timestamp literals.
- Added Snowflake snapshot fixtures + gated fakesnow/sqlglot validation, plus an AST-based import confinement guard for
src/signalforge/prune/.
Reviewed changes
Copilot reviewed 23 out of 24 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| uv.lock | Adds locked dependencies for Snowflake SQL validation stack (fakesnow/duckdb/pyarrow/sqlglot). |
| pyproject.toml | Adds fakesnow to dev deps; registers/excludes snowflake marker by default. |
| src/signalforge/warehouse/models.py | Extends Dialect with SQL fragment fields; configures SNOWFLAKE_DIALECT values. |
| src/signalforge/prune/compiler.py | Makes compilation fully dialect-driven (folding, quoting, sampling predicate, literals, qualified names). |
| tests/warehouse/test_models.py | Pins new Dialect field defaults + Snowflake-specific values. |
| tests/prune/test_compiler.py | Adds unit coverage for folding/quoting + Snowflake snapshot suite and guards. |
| tests/prune/test_compiler_import_guard.py | AST scan ensuring no snowflake/google.cloud imports under src/signalforge/prune/. |
| tests/prune/test_compiler_fakesnow.py | Gated fakesnow execution tests + sqlglot Snowflake parse guard for fixtures. |
| tests/fixtures/prune/compiled_sql/snowflake/not_null.sql | Snowflake snapshot fixture for not_null (full). |
| tests/fixtures/prune/compiled_sql/snowflake/not_null_sample.sql | Snowflake snapshot fixture for not_null (sample). |
| tests/fixtures/prune/compiled_sql/snowflake/unique.sql | Snowflake snapshot fixture for unique (full). |
| tests/fixtures/prune/compiled_sql/snowflake/unique_sample.sql | Snowflake snapshot fixture for unique (sample). |
| tests/fixtures/prune/compiled_sql/snowflake/accepted_values.sql | Snowflake snapshot fixture for accepted_values (full). |
| tests/fixtures/prune/compiled_sql/snowflake/accepted_values_sample.sql | Snowflake snapshot fixture for accepted_values (sample). |
| tests/fixtures/prune/compiled_sql/snowflake/relationships.sql | Snowflake snapshot fixture for relationships (full). |
| tests/fixtures/prune/compiled_sql/snowflake/relationships_sample.sql | Snowflake snapshot fixture for relationships (sample). |
| tests/fixtures/prune/compiled_sql/snowflake/custom_sql.sql | Snowflake snapshot fixture for custom SQL (single-table, full). |
| tests/fixtures/prune/compiled_sql/snowflake/custom_sql_sample.sql | Snowflake snapshot fixture for custom SQL (single-table, sample). |
| tests/fixtures/prune/compiled_sql/snowflake/custom_sql_fullscan.sql | Snowflake snapshot fixture for custom SQL (multi-table full-scan). |
| plans/super/121-prune-snowflake-dialect.md | Plan document capturing decisions and breakdown for #121. |
| docs/prune-ops.md | Documents Snowflake compiler behavior and validation strategy. |
| CLAUDE.md | Updates public API surface notes for new Dialect fields + Snowflake compiler support. |
| .claude/rules/warehouse-adapters.md | Documents new Dialect SQL-fragment fields as part of adapter conventions. |
| .claude/rules/prune-engine.md | Updates pruning rules to reflect new dialect-driven compiler surfaces and Snowflake validation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- test_compiler_import_guard.py: fix confusing `googleftover` example in the prefix-match comment -> `google_leftover` (a name that starts with `google` but is not `google.cloud`). - plans/super/121-prune-snowflake-dialect.md: align the Phase line with reality (implementation complete, PR open awaiting review) so it no longer reads as a plan-only review. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PR Review SummaryFixed (2 items)
False Positives (0 items)None — both comments were real and fixed inline. |
Codecov flagged 1 uncovered patch line — compiler.py:149, the
`identifier_case == "lower"` branch of `_fold_identifier`. No compiler test
exercised a lower-folding dialect (POSTGRES_DIALECT isn't used in the
snapshot suite). Added test_quote_folds_lower_for_postgres_dialect pinning
`_quote("CustomerId", POSTGRES_DIALECT) == '"customerid"'`.
Patch coverage now 100% for the changeset; the remaining compiler.py:312
miss is pre-existing #116 custom_sql code, outside this PR's diff.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
…flake-dialect # Conflicts: # .claude/rules/warehouse-adapters.md # CLAUDE.md
Summary
Make the prune compiler emit valid Snowflake SQL purely from the
Dialectvalue object — no branching on dialect name, no warehouse SDK import undersignalforge/prune/. Part of epic #118; depends on the #119 skeleton.Closes #121.
Changes
Dialectextended with five declarative SQL-fragment fields (all BigQuery-defaulted so BigQuery output is byte-identical):sample_row_hash_expr,timestamp_literal_template,date_literal_template,quote_qualified_per_component,sample_cte_alias.SNOWFLAKE_DIALECTsets the Snowflake forms.identifier_case(graduated from declared-but-unused → load-bearing; Snowflake folds to UPPER), per-component qualified-name quoting ("DB"."SCH"."T"),HASH(*)sample predicate,::TIMESTAMP/::DATEliteral casts.sample_cte_alias— Snowflake uses the quoted"sample"becauseSAMPLEis a reserved keyword (WITH sample ASis a syntax error there). This bug was caught in the Quality Gate by a gatedsqlglotparse-guard, not by snapshots.QUALIFYdeliberately NOT wired —uniquekeeps the dialect-portableGROUP BY … HAVING;supports_qualifystays forward-compat metadata.prune/; gated@pytest.mark.snowflakefakesnow execution + sqlglot parse validation.Testing
uv run pytest -m snowflake --no-cov→ 19 passed (fakesnow rule-semantic execution of the 4 built-ins + sqlglot parse of all 11 fixtures).HASH(*)semantics / case-folding / sampling deferred to test+docs: fakesnow harness + FakeSnowflakeClient + gated live e2e (TPCH_SF1) + ops docs #124's live harness (DEC-005).Compounding Update
.claude/rules/prune-engine.md— full Dialect-field list the compiler reads;identifier_casegraduation; the snapshot-pins-shape-not-validity lesson; Snowflake subsection.docs/prune-ops.md— Snowflake compiler dialect section (case-folding + residual,HASH()cross-version reproducibility caveat, validation tiers).CLAUDE.md— public-API surface for the new Dialect fields + compiler Snowflake path..claude/rules/warehouse-adapters.md—Dialectnow carries prune-compiler SQL-fragment templates.🤖 Generated with Claude Code