Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .claude/rules/prune-engine.md
Original file line number Diff line number Diff line change
Expand Up @@ -64,13 +64,16 @@ Issue #121 made the compiler emit valid **Snowflake** SQL by extending `Dialect`
- `identifier_case` — **graduated from declared-but-unused to load-bearing in #121.** `_fold_identifier`/`_quote` fold every identifier (columns AND each qualified-name component) per `"upper"`/`"lower"`/`"preserve"` BEFORE quoting. Snowflake folds to UPPER (a conventional `CREATE TABLE(customer_id)` stores `CUSTOMER_ID`, so emitting `"customer_id"` would fail); BigQuery `"preserve"` is a no-op so its snapshots stay byte-identical. Folding runs on an already-`validate_identifier`'d ASCII token, so it cannot introduce a quote-breaking char (DEC-024 still gates).
- `quote_qualified_per_component` — `True` quotes each component separately (`"DB"."SCH"."T"` — Snowflake/Postgres); `False` wraps the whole dotted path in one quote pair (BigQuery `` `p.d.t` ``). A single quoted string spanning dots would read as one identifier literally named `db.schema.table`.
- `sample_row_hash_expr` — the deterministic-sample row-hash expression dropped into `MOD(<expr>, <bucket>) < 1`. BigQuery `ABS(FARM_FINGERPRINT(TO_JSON_STRING(t)))`; Snowflake `ABS(HASH(*))`.
- `sample_hash_in_projection` + `sample_hash_alias` (issue #139) — the structural sample SHAPE switch. `sample_hash_in_projection=False` (BigQuery) emits the row hash inline in `WHERE`/`ORDER BY`; `True` (Snowflake) emits the **projection-subquery** form — `SELECT * EXCLUDE (<alias>) FROM (SELECT t.*, <hash_expr> AS <alias> FROM <src> AS t) WHERE MOD(<alias>, b) < 1 …` — because Snowflake rejects `HASH(*)` as a predicate (`002079`) and accepts it only in the SELECT projection. `sample_hash_alias` defaults to `"_sf_sample_hash"`. **The compiler's sample CTE delegates to `signalforge.warehouse._sample_sql.render_sample_select` (shared with the `SnowflakeAdapter` sample methods, calling it with `order_by_hash=False`)**, so the CTE body and the adapter sample SELECT stay byte-consistent; the helper switches on the boolean field, never `dialect.name`, so the import-guard stays green. BigQuery's emitted bytes are unchanged (default `False` reproduces the prior inline CTE body).
- `timestamp_literal_template` / `date_literal_template` — `str.format(value=…)` templates for partition-filter literals. BigQuery `TIMESTAMP('{value}')` / `DATE('{value}')`; Snowflake `'{value}'::TIMESTAMP` / `'{value}'::DATE`. Only the `datetime`/`date` branches format; the `str` branch routes through `escape_bq_string_literal` (no `.format`).
- `sample_cte_alias` — the identifier the sample CTE is bound to (`WITH <alias> AS (...) ... FROM <alias>`). BigQuery bare `sample`; **Snowflake quoted `"sample"`** because `SAMPLE` is a Snowflake reserved keyword (`TABLESAMPLE`) and an unquoted `WITH sample AS` is a syntax error there. This bug was caught in #121's Quality Gate by the gated `sqlglot`-parse guard, NOT by snapshots — a snapshot can pin invalid SQL byte-for-byte. **Lesson: a new dialect's SQL needs a parser/executor in the loop, not just snapshot equality.**

`supports_qualify` stays **declared-but-unconsumed forward-compat metadata** (DEC-004 of #121): `unique` keeps the dialect-portable `GROUP BY … HAVING COUNT(*) > 1`; a `QUALIFY` rewrite would be a semantics change (returns failing rows vs the duplicated key) and BigQuery supports QUALIFY too, so it isn't Snowflake-specific. Don't add a QUALIFY codepath without a separate decision.

**Adding a new vendor dialect:** extend the `Dialect` value object with a declarative field and read it in the compiler — never `if dialect.name == …`. Keep BigQuery-shaped defaults on new fields so existing construction sites + the 11 BigQuery snapshot fixtures stay byte-identical (the load-bearing regression gate). Validate the emitted SQL through a real parser (`sqlglot`) or executor (`fakesnow`/live), gated behind a maintainer-only marker — snapshot equality alone certifies shape, not validity.

**A single inline SQL-fragment STRING can't express a clause-POSITION constraint (issue #139, generalising #121).** `sample_row_hash_expr` was a string, but Snowflake's `HASH(*)` is projection-only — legal in `SELECT`, rejected (`002079`) in `WHERE`/`ORDER BY`. When a vendor needs a different SQL *shape* (not just a different expression in the same slot), add a **structural** `Dialect` field (here `sample_hash_in_projection: bool`) + a **shared renderer** (`warehouse/_sample_sql.render_sample_select`) so every emitter agrees on the shape — don't try to encode the shape inside a richer template string. And note the cert gap: `sqlglot` *parses* `SELECT * EXCLUDE (col) … ORDER BY col` happily but cannot certify Snowflake *accepts* it, so a shape this subtle is only truly validated by the live-gated (`@pytest.mark.snowflake` + `SF_RUN_SNOWFLAKE=1`) test — that live run is the real merge gate, not the snapshot/sqlglot tier.

NULL-exclusion pattern matches dbt-core verbatim (DEC-023): `unique` adds `WHERE col IS NOT NULL`; `accepted_values` adds `WHERE col IS NOT NULL AND col NOT IN (...)`; `relationships` adds child-side `WHERE child.col IS NOT NULL`. Snapshot fixtures pin the exact SQL bytes.

## Total-budget semantics (DEC-011)
Expand Down
6 changes: 3 additions & 3 deletions .claude/rules/warehouse-adapters.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,14 @@ The ABC is warehouse-agnostic. v0.2 Snowflake/Postgres slot under `adapters/` wi
**Snowflake sampling + connection-bound session (issue #122).** `SnowflakeAdapter` implements the sampling surface — `sample_rows`, `materialise_sample`, `run_test_sql` (`column_stats` stays `NotImplementedError`; `estimate_query_bytes` was the ABC degrade pending #123 at the time of #122, then graduated to a real `EXPLAIN USING JSON` override in #130 — see the `estimate_query_bytes` graduation note below). Five load-bearing conventions, most diverging from BigQuery because Snowflake's session model differs:

- **The connection IS the session (no `session_id` string).** BigQuery threads a server-side `session_id` via `connection_properties` on every query (§ "Session/connection state"); Snowflake's *connection* holds the session, so `self._active_session` stores the **connection object** itself and every op runs on the one connection. A `connection=` injection seam on `__init__` (mirrors BigQuery's `client=`) + lazy `_get_connection()` (builds via the shim's `make_real_client` on first use) is the testability seam. `_get_connection()` sets `_active_session` on first open.
- **Reuse the `SNOWFLAKE_DIALECT` SQL-fragment fields — never hard-code `HASH(*)`.** `sample_rows` and the `materialise_sample` CTAS build `MOD(<dialect.sample_row_hash_expr>, <bucket>) < 1` + `ORDER BY <…>` and render partition filters via `dialect.timestamp_literal_template` / `date_literal_template`. Reading the dialect (not hard-coding) keeps the adapter's sample SQL byte-consistent with the prune compiler's sample CTE (Architectural Commitment #5).
- **Reuse the `SNOWFLAKE_DIALECT` SQL-fragment fields — never hard-code `HASH(*)`.** `sample_rows` and the `materialise_sample` CTAS delegate to `signalforge.warehouse._sample_sql.render_sample_select(..., order_by_hash=True)` (shared with the prune compiler's sample CTE — issue #139), which reads `dialect.sample_row_hash_expr`, `dialect.sample_hash_in_projection`, `dialect.sample_hash_alias`; partition filters are rendered by the caller via `dialect.timestamp_literal_template` / `date_literal_template` and passed in as `extra_where`. Reading the dialect (not hard-coding) keeps the adapter's sample SQL byte-consistent with the prune compiler's sample CTE (Architectural Commitment #5). **Since #139 Snowflake emits the projection-subquery shape** — `SELECT * EXCLUDE (_sf_sample_hash) FROM (SELECT t.*, ABS(HASH(*)) AS _sf_sample_hash FROM <src> AS t) WHERE MOD(_sf_sample_hash, b) < 1 [AND <pf>] ORDER BY _sf_sample_hash LIMIT n` — because `HASH(*)` is rejected as a `WHERE`/`ORDER BY` predicate (`002079`); BigQuery keeps the inline form (`sample_hash_in_projection=False`).
- **Table size via `INFORMATION_SCHEMA.TABLES.ROW_COUNT`, case-insensitive.** `_get_num_rows` queries `SELECT ROW_COUNT FROM <db>.INFORMATION_SCHEMA.TABLES WHERE UPPER(TABLE_SCHEMA)=UPPER('…') AND UPPER(TABLE_NAME)=UPPER('…')` (schema/name embedded as escaped string literals via `escape_bq_string_literal` — Snowflake uses backslash escaping too; when `project is None` the query is left **unqualified** — `INFORMATION_SCHEMA.TABLES`, resolved against the session's current database — NOT `CURRENT_DATABASE().INFORMATION_SCHEMA`, which is invalid Snowflake since `CURRENT_DATABASE()` is a scalar function, not a namespace qualifier). `ROW_COUNT` is NULL for views/MVs → routes through the *same* fail-loud sizing decision BigQuery uses (unknown+no-filter → `UnknownTableSizeError`; unknown+filter → `bucket=1000`; `>= _LARGE_TABLE_THRESHOLD` (100M, re-declared not imported) + no-filter → `SamplingRequiresPartitionFilterError`; else `max(num_rows//n, 1)`). `num_rows == 0` follows the unknown pathway (pin a test — easy to split from `None` by accident).
- **`materialise_sample` returns a fully-qualified temp `TableRef`; reuse the shared `run_id` recipe.** `CREATE TEMPORARY TABLE "<src db>"."<src schema>"."_sf_sample_<run_id>" AS SELECT …` colocated with the source; returns `TableRef(project=table.project, dataset=table.dataset, name="_sf_sample_<run_id>")`. `run_id` comes from `signalforge.warehouse._sample_id` (`_compute_run_id` / `_canonical_partition_filter` / `_hash_session_id` were **hoisted there from `adapters/bigquery.py` in #122** so the recipe is byte-identical across vendors — both adapters import it; relocation is a pure move, BigQuery snapshots unchanged). The #116 materialised-sample-substitution gotcha (`prune-engine.md`) is pinned with a **`custom_sql` `{{ this }}`** test at `scope="full"` — NOT `not_null`, which trivially `FROM`s `table_ref` and so can never bypass substitution.
- **Cleanup-boundary fail-soft, reshaped for Snowflake (no manual command).** `__exit__` → `_cleanup_active_session()` closes the connection (reaping its session-scoped temp tables), swallows failure, emits ONE operator-actionable WARNING. Unlike BigQuery's `bq query … CALL BQ.ABORT_SESSION()` remediation, **there is no manual drop command** — a temp table is unreachable outside its owning session, so the honest durable fallback is Snowflake's server-side idle-session reap. The WARNING quotes **no `auto-expire in <N>s` countdown** (the timeout is server-side/account-config; `ttl_seconds` is accepted for ABC parity but ignored; `_session_started_at` is provenance only). Raw `session_id` appears only in the failure WARNING (DEC-014 narrow exception); the success INFO uses `_hash_session_id`. State resets in `finally` for idempotency — but **do NOT null `self._connection`** there: idempotency comes from the `_active_session is None` early-return, and nulling an injected connection would silently route a re-entry into a real lazy-build (mirrors BigQuery, which never nulls its client).

Minimal `map_snowflake_exception` (in the shim, lazy SDK-error import) maps auth → `WarehouseAuthError`, programming → `QuerySyntaxError`, else passthrough; the full taxonomy + fakesnow harness + gated live e2e + ops docs are #124.

**`Dialect` now carries prune-compiler SQL-fragment templates (issue #121).** The `Dialect` value object (`warehouse/models.py`) graduated from pure capability flags to also carrying the declarative SQL fragments the prune compiler reads so it can emit warehouse-correct SQL without branching on `dialect.name`: `sample_row_hash_expr`, `timestamp_literal_template`, `date_literal_template`, `quote_qualified_per_component`, and `sample_cte_alias` (all BigQuery-defaulted so existing constants/snapshots are byte-unchanged). `SNOWFLAKE_DIALECT` sets them to Snowflake forms (`ABS(HASH(*))`, `'{value}'::TIMESTAMP/::DATE`, per-component quoting, the quoted `"sample"` CTE alias — `SAMPLE` is reserved in Snowflake). When a future vendor adapter ships its own `Dialect`, populate these fields too — the compiler is the consumer and it never name-branches (see `prune-engine.md` § "Compiler is dialect-driven"). `identifier_case` graduated from declared-but-unused to load-bearing in the same change (the compiler folds identifiers per it before quoting). `POSTGRES_DIALECT` keeps BigQuery defaults for the new fields with a docstring note — the Postgres stub never invokes the compiler, so they're corrected when its ops land.
**`Dialect` now carries prune-compiler SQL-fragment templates (issue #121, extended #139).** The `Dialect` value object (`warehouse/models.py`) graduated from pure capability flags to also carrying the declarative SQL fragments the prune compiler reads so it can emit warehouse-correct SQL without branching on `dialect.name`: `sample_row_hash_expr`, `timestamp_literal_template`, `date_literal_template`, `quote_qualified_per_component`, `sample_cte_alias`, and (issue #139) `sample_hash_in_projection: bool` + `sample_hash_alias: str` (all BigQuery-defaulted so existing constants/snapshots are byte-unchanged). `SNOWFLAKE_DIALECT` sets them to Snowflake forms (`ABS(HASH(*))`, `'{value}'::TIMESTAMP/::DATE`, per-component quoting, the quoted `"sample"` CTE alias — `SAMPLE` is reserved in Snowflake — and `sample_hash_in_projection=True` + `sample_hash_alias="_sf_sample_hash"` so the shared `_sample_sql.render_sample_select` emits the projection-subquery sample shape rather than an inline `HASH(*)` predicate). When a future vendor adapter ships its own `Dialect`, populate these fields too — the compiler is the consumer and it never name-branches (see `prune-engine.md` § "Compiler is dialect-driven"). `identifier_case` graduated from declared-but-unused to load-bearing in the same change (the compiler folds identifiers per it before quoting). `POSTGRES_DIALECT` keeps BigQuery defaults for the new fields with a docstring note — the Postgres stub never invokes the compiler, so they're corrected when its ops land.

## Unified multi-warehouse `DbtProfileTarget` + per-type cross-field validator (issue #120)

Expand Down Expand Up @@ -235,7 +235,7 @@ When introducing a new fail-soft cleanup boundary (Postgres temp-table cleanup,

The gated `snowflake` marker now spans offline (`fakesnow`/`sqlglot`, run with no env vars) AND live (`SF_RUN_SNOWFLAKE=1` + conn vars, +`SNOWFLAKE_DATABASE`/`SCHEMA` writable for the engineered prune-live table, +`ANTHROPIC_API_KEY` for the full pipeline) tests under one `uv run pytest -m snowflake --no-cov`. The `docs/warehouse-adapter-ops.md` § "Snowflake adapter (v0.2)" carries the consolidated operator story incl. the **cost guidance** (resource monitor FIRST, XS warehouse, aggressive auto-suspend) that any live Snowflake test must surface in its module docstring.

**Live-harness findings — fakesnow/fakes hid multiple live-only adapter bugs (the #124 payoff).** Running the gated suite against a real warehouse surfaced bugs that the offline `fakesnow` + hand-fakes could not, because fakesnow's DuckDB cursor/result shapes diverge from the real connector's. The lesson generalises the #121 "parser/executor in the loop" rule: a vendor adapter is NOT certified until a live run exercises every path; fakes prove behaviour-against-a-contract, not behaviour-against-the-vendor. Concretely #124's live run found, in order: (1) **`_quote` case-folding** (fixed here — the adapter must fold-then-quote identically to the compiler, else CREATE-vs-REFERENCE diverge); (2) **`ARRAY_AGG(OBJECT_CONSTRUCT(*))` capture-failures samples come back as a JSON *string*** (a VARIANT), not a Python list — `run_test_sql` must `json.loads` it before iterating (fakesnow returned a list, so offline passed); (3) **`HASH(*)` is invalid in WHERE/ORDER BY** on Snowflake → all sample-mode SQL is rejected (bead `bd_1-scaffolding-cdp`); (4) **oneshot sample-bucket row-count routes through a BigQuery-only `_get_client`** the engine never made vendor-neutral (bead `bd_1-scaffolding-tft`). Net operator contract: **live Snowflake works today only with `safety: schema-only` + `prune.scope: full`** — both sample strategies + `safety: sample` + `aggregate-only` are deferred/beaded. When the next vendor adapter ships, budget a live-debugging pass; expect VARIANT/array columns as JSON strings and identifier case-folding mismatches as the first two things to break.
**Live-harness findings — fakesnow/fakes hid multiple live-only adapter bugs (the #124 payoff).** Running the gated suite against a real warehouse surfaced bugs that the offline `fakesnow` + hand-fakes could not, because fakesnow's DuckDB cursor/result shapes diverge from the real connector's. The lesson generalises the #121 "parser/executor in the loop" rule: a vendor adapter is NOT certified until a live run exercises every path; fakes prove behaviour-against-a-contract, not behaviour-against-the-vendor. Concretely #124's live run found, in order: (1) **`_quote` case-folding** (fixed here — the adapter must fold-then-quote identically to the compiler, else CREATE-vs-REFERENCE diverge); (2) **`ARRAY_AGG(OBJECT_CONSTRUCT(*))` capture-failures samples come back as a JSON *string*** (a VARIANT), not a Python list — `run_test_sql` must `json.loads` it before iterating (fakesnow returned a list, so offline passed); (3) **`HASH(*)` is invalid in WHERE/ORDER BY** on Snowflake → all sample-mode SQL was rejected (bead `bd_1-scaffolding-cdp`) — **FIXED by #139**: the shared `_sample_sql.render_sample_select` now emits the projection-subquery shape (`SELECT * EXCLUDE (_sf_sample_hash) FROM (SELECT t.*, ABS(HASH(*)) AS _sf_sample_hash …)`) driven by `Dialect.sample_hash_in_projection`, so `materialise_sample` / `sample_rows` / the compiler sample CTE all compute the hash in the SELECT projection rather than the rejected predicate position; (4) **oneshot sample-bucket row-count routes through a BigQuery-only `_get_client`** the engine never made vendor-neutral (bead `bd_1-scaffolding-tft`, still open). Net operator contract: **live Snowflake works today with `safety: schema-only` + `prune.scope: full` OR `prune.scope: sample` + `prune.sample_strategy: materialised`** (the #139 projection-subquery fix) — `prune.sample_strategy: oneshot` (bead `bd_1-scaffolding-tft`) + `aggregate-only` (`column_stats`) remain deferred/beaded. When the next vendor adapter ships, budget a live-debugging pass; expect VARIANT/array columns as JSON strings, identifier case-folding mismatches, AND vendor clause-position constraints (a SQL fragment legal in one clause may be rejected in another — the #139 `HASH(*)` lesson) as the first things to break.

## Reference

Expand Down
Loading
Loading