#224: Databricks deterministic sampling + materialise_sample + get_row_count

.claude/rules/warehouse-adapters.md

@@ -34,7 +34,21 @@ The ABC is warehouse-agnostic. v0.2 Snowflake/Postgres slot under `adapters/` wi
 
 **`DATABRICKS_DIALECT` values are offline-certified as of #223; live execution deferred to #226.** At the #221 skeleton stage the dialect's SQL-fragment fields (the `xxhash64` hash, Spark `DATE_TRUNC('unit', date)` / `DAYOFWEEK(date)` / `PERCENTILE_CONT … WITHIN GROUP` date-arithmetic, `TIMESTAMP '{value}'` literals) were grounded-but-unexecuted. **#223 has since landed** the prune-compiler cert: the compiler emits valid Databricks SQL purely from `DATABRICKS_DIALECT` (no field-value change was needed — the #221 values were correct), pinned by 32 byte-exact fixtures (`tests/fixtures/prune/compiled_sql/databricks/` + `…/anomaly/databricks/`) and an **ungated** `sqlglot` `databricks`-dialect parse-guard (`tests/prune/test_compiler_databricks.py`, DEC-002 — runs in the default suite because `sqlglot` is a base dep and Databricks has no offline execution fake; see `prune-engine.md` § "Databricks compiler dialect"). Snapshot equality certifies *shape*, the parse-guard certifies *syntactic validity*; real-Spark execution semantics + a real `CREATE TABLE` round-trip (the `identifier_case='lower'` lower-fold assumption, load-bearing for anomaly-grain column matching) remain deferred to the **#226** gated live Free-Edition run — snapshot/parse equality alone does not certify the engine *accepts* the SQL (the #121/#171 lesson).
 
-**⚠️ `TableRef.project` length blocks Unity Catalog catalog names (gotcha for #224).** `TableRef.project` validates as a GCP project id (`validate_project_id`, 6–30 chars), so a Unity Catalog catalog like `main` (4 chars) **fails `TableRef` construction**. The #221 skeleton tests sidestep with `project=None` (two-part `dataset.name`). #224's sampling path threads catalog names into `TableRef.project` (three-part `catalog.schema.table` via `quote_qualified_per_component=True`) and will hit this — either relax the project-id validation for the Databricks dialect or carry the catalog on a separate field. Surfaced while writing the skeleton stub tests; do NOT assume `TableRef` accepts arbitrary catalog identifiers.
+**`TableRef.project` now accepts Unity Catalog catalog names (RESOLVED by #224, DEC-005).** Historically `TableRef.project` validated strictly as a GCP project id (`validate_project_id`, 6–30 chars), so a Unity Catalog catalog like `main` (4 chars) **failed `TableRef` construction** — the #221 skeleton sidestepped with `project=None` (two-part `dataset.name`). #224's sampling path threads catalog names into `TableRef.project` (three-part `catalog.schema.table` via `quote_qualified_per_component=True`), so #224 added `_sql_safety.validate_catalog_or_project(field, value)` — accepts a value that passes **EITHER** `validate_identifier` **OR** `validate_project_id`, and routes `TableRef.__post_init__`'s `project` check through it. The combined admissible alphabet is `[A-Za-z0-9_-]`, so injection-shaped values (`;`, quotes, whitespace, backticks, newlines, control chars, leading hyphen) still raise `InvalidIdentifierError` — the relaxation only *adds* short / underscored identifiers (`main`, `db`), removing no rejection, and `validate_project_id` itself is untouched (BigQuery unchanged). It also fixes the latent short-Snowflake-DB case. **The general lesson: when a dialect-neutral value object's validator is too strict for a new vendor's identifier grammar, compose a union validator (`<strict> OR <other-strict>`) rather than weakening either constituent or adding a vendor-specific field** — the value stays identifier-shape-gated, so no injection vector opens.
+
+**Databricks sampling + connection-bound session (issue #224).** `DatabricksAdapter` graduates off the #221 skeleton with the full sampling surface — `sample_rows`, `get_row_count`, `materialise_sample`, `run_test_sql`, **and `column_stats`** (`estimate_query_bytes` / `run_stats_query` keep the ABC degrade pending #225; live validity is **#226**). It mirrors the Snowflake #122 conventions; the load-bearing deltas vs Snowflake are where Spark/Databricks diverges:
+
+- **Connection IS the session** (Snowflake #122 verbatim): `_active_session` stores the connection object; lazy `_get_connection()` + `connection=` injection seam; all ops on the one connection; fail-soft `__exit__` WARNING with **no manual cleanup command** (temp objects are session-local) and **no `auto-expire` countdown**; raw session id only in the failure WARNING, hashed id (`_hash_session_id`) in the success INFO; `_active_session is None` idempotency, `_connection` never nulled in `finally`.
+- **Inline-predicate sample shape — NO Snowflake `HASH(*)` restriction (DEC-002).** Databricks accepts `xxhash64(...)` / masked `MOD(...)` directly in `WHERE`/`ORDER BY`, so `DATABRICKS_DIALECT.sample_hash_in_projection=False` (BigQuery-style inline), **unlike** Snowflake's projection-subquery form (#139). `sample_rows` / `materialise_sample` delegate to the shared `render_sample_select` and read the hash from the dialect — never hard-coded.
+- **`get_row_count` via `SELECT COUNT(*)`, NOT `DESCRIBE DETAIL` (DEC-003).** `DESCRIBE DETAIL` has **no reliable top-level `numRows`** (it lives in the `statistics` map, only after `ANALYZE TABLE`, Delta-only) and `information_schema.tables` has no `row_count` — so `get_row_count` runs `COUNT(*)` (metadata-cheap on Delta), returning `None` on a `WarehouseError` so the shared fail-loud `_resolve_sample_bucket` sizing decides (`UnknownTableSizeError` / `bucket=1000` / `SamplingRequiresPartitionFilterError` / `max(n//,1)` — re-declares the 100M threshold, never imports a BQ-private one). **Lesson: a vendor whose cheap metadata row-count is unreliable should `COUNT(*)` (correct, often metadata-cheap) and route uncertainty through the shared sizing, not invent a flaky metadata read.**
+- **Fold-then-quote parity, applied to the partition-filter column too (DEC-008).** `_quote`/`_quote_identifier`/`_fold` lower-fold then backtick-quote per component, identical to the prune compiler (the #124 CREATE-vs-REFERENCE lesson) — and `_render_partition_filter` folds the PF column, which the **Snowflake adapter does NOT** (a latent Snowflake divergence the Databricks code avoids).
+- **`materialise_sample` = qualified `CREATE TEMPORARY TABLE` mirroring Snowflake (DEC-004).** `CREATE TEMPORARY TABLE <cat>.<sch>._sf_sample_<run_id> AS <sample body>`; `run_id` via the shared `_compute_run_id`; returns the qualified temp `TableRef`; `MaterialisationFailedError(cause=mapped)` on cursor error. Reusing the qualified-temp-ref shape lets the prune compiler's Dialect-quoting consume it with **zero TableRef/compiler change**.
+- **`run_test_sql` capture: `to_json(struct(*))` returns JSON STRINGS → `json.loads` per row (DEC-007).** Per-row `to_json(struct(*))` + `LIMIT k` (NOT `collect_list`, avoiding `ARRAY<STRING>` marshalling assumptions) — **distinct from Snowflake's single-VARIANT `OBJECT_CONSTRUCT` decode** (one outer `json.loads`); here each row's column is its own JSON string needing its own decode.
+- **`column_stats` shipped AHEAD of Snowflake (DEC-011).** A single aggregate (`COUNT`/`COUNT(DISTINCT)`/`COUNT_IF(... IS NULL)`/`MIN`/`MAX`/`MAX(typeof(col))` for `data_type`), no context-manager batching. Snowflake's parity is the open follow-up **[#258](https://github.com/wjduenow/SignalForge/issues/258)**. **Known divergence:** BigQuery skips `MIN`/`MAX` (→ `None`) for complex types (ARRAY/STRUCT/MAP/JSON/BINARY/GEOGRAPHY) per the `ColumnStats` contract; Databricks emits them unconditionally because `data_type` is derived inline (`typeof`) in the same aggregate, so the type isn't known before the query is built — the scalar-column path is the supported v0.x surface, complex-type handling is a #226 item.
+- **Full error taxonomy in the shim (DEC-009).** `map_databricks_exception` maps auth → `WarehouseAuthError`, table-not-found → `TableNotFoundError`, invalid-identifier → `ColumnNotFoundError`, residual programming → `QuerySyntaxError`, else passthrough — reusing existing typed errors (no new class), lazy SDK-error import confined to `_databricks_client.py` (the confinement test stays green).
+- **Close the cursor in `try/finally` after every query (PR #257 review).** `_execute` / `_execute_to_dicts` / `materialise_sample` open a cursor on the long-lived connection and MUST release it on both the success and failure paths, or repeated queries leak server-side cursor handles. The `_execute_to_dicts` close happens **after** `_rows_to_dicts` reads `cursor.description` (the description is needed to shape tuple rows into dicts) — so the cleanup is an outer `try/finally` wrapping an inner `try/except` that scopes the exception mapping to `execute`/`fetchall` only. Pinned by `tests/warehouse/test_databricks_adapter.py::test_execute_closes_cursor_on_{success,failure}` + the `_execute_to_dicts` / materialise variants (the fake's `_FakeDatabricksCursor.closed` + `FakeDatabricksConnection.cursors`). **Latent-leak note:** the Snowflake adapter only closes its cursor in `_execute_scalar` — its `_execute` / `_execute_to_dicts` / `materialise_sample` have the **same leak** and should get the same `try/finally` when next touched (out of #224 scope; the new convention is "every cursor-opening adapter method closes in `try/finally`").
+
+**Testing tiers + the #226 live-cert ledger.** Shape is certified now — hand-rolled `FakeDatabricksConnection` (behaviour) + an **ungated** `sqlglot` `databricks`-dialect parse-guard (`tests/warehouse/test_databricks_sql_parse.py`, default suite, with a planted-violation self-check + per-shape `len(statements)` floor) over every emitted statement (COUNT, sample, CTAS, capture, column_stats), plus the #116 `custom_sql {{ this }}` materialised-substitution test. **Snapshot/parse equality certifies SHAPE, not that a live Databricks SQL warehouse ACCEPTS the SQL** (the #121/#124/#171 lesson) — so the following are explicit **#226 live-cert items**, flagged in code comments + test docstrings (never claimed certified): (a) qualified `CREATE TEMPORARY TABLE <cat>.<sch>.<temp>` acceptance; (b) the `databricks-sql-connector` persisting the session across queries (so the temp table is reachable from a follow-up `run_test_sql`); (c) `to_json(struct(*))` per-row capture marshalling; (d) `column_stats` MIN/MAX on complex types; (e) the `str`-partition-filter escape's dependency on Spark's default `escapedStringLiterals=false`. **When a worker can't reach the live warehouse, certify shape (fakes + sqlglot) and enumerate the vendor-divergence risks as a named live-cert ledger — don't let snapshot-green read as live-green.**
 
 **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:
 
@@ -260,4 +274,4 @@ The gated `snowflake` marker now spans offline (`fakesnow`/`sqlglot`, run with n
 
 ## Reference
 
-`plans/super/124-snowflake-test-docs.md` — DEC-001 … DEC-011 (test harness + full taxonomy + two-strategy live e2e + ops docs). `plans/super/3-bigquery-adapter.md` — DEC-001 … DEC-028. `plans/super/22-temp-table-sample.md` — v0.2 materialised-sample additions (`materialise_sample` ABC, BigQuery session-state pattern, cleanup-WARNING shape). `plans/super/36-estimate-cost-preview.md` — `estimate_query_bytes` ABC addition. `plans/super/120-snowflake-profile.md` — DEC-001 … DEC-010 (unified `DbtProfileTarget` + per-type cross-field validator, `IncompleteProfileError`, `validate_snowflake_account`, auth scope, drift detector). `plans/super/122-snowflake-sampling.md` — DEC-001 … DEC-010 (Snowflake `sample_rows` / `materialise_sample` / `run_test_sql`, connection-bound session state, shared `_sample_id` hoist, INFORMATION_SCHEMA sizing, Snowflake-shaped cleanup WARNING). `plans/super/130-snowflake-estimate-explain.md` — DEC-001 … DEC-009 (Snowflake `estimate_query_bytes` via `EXPLAIN USING JSON`, `EstimateUnavailableError` typed degrade, planner-estimate accuracy caveat). `src/signalforge/warehouse/_sample_id.py` — shared deterministic-sample-id seam. `src/signalforge/warehouse/` — current implementation. `tests/warehouse/_fake.py` — `FakeBigQueryClient` + `expect_*` API. `docs/warehouse-adapter-ops.md` — operational reference.
+`plans/super/124-snowflake-test-docs.md` — DEC-001 … DEC-011 (test harness + full taxonomy + two-strategy live e2e + ops docs). `plans/super/3-bigquery-adapter.md` — DEC-001 … DEC-028. `plans/super/22-temp-table-sample.md` — v0.2 materialised-sample additions (`materialise_sample` ABC, BigQuery session-state pattern, cleanup-WARNING shape). `plans/super/36-estimate-cost-preview.md` — `estimate_query_bytes` ABC addition. `plans/super/120-snowflake-profile.md` — DEC-001 … DEC-010 (unified `DbtProfileTarget` + per-type cross-field validator, `IncompleteProfileError`, `validate_snowflake_account`, auth scope, drift detector). `plans/super/122-snowflake-sampling.md` — DEC-001 … DEC-010 (Snowflake `sample_rows` / `materialise_sample` / `run_test_sql`, connection-bound session state, shared `_sample_id` hoist, INFORMATION_SCHEMA sizing, Snowflake-shaped cleanup WARNING). `plans/super/130-snowflake-estimate-explain.md` — DEC-001 … DEC-009 (Snowflake `estimate_query_bytes` via `EXPLAIN USING JSON`, `EstimateUnavailableError` typed degrade, planner-estimate accuracy caveat). `plans/super/224-databricks-sampling.md` — DEC-001 … DEC-012 (Databricks `sample_rows` / `get_row_count` / `materialise_sample` / `run_test_sql` / `column_stats`, `validate_catalog_or_project` for Unity Catalog catalogs, inline-predicate sample shape, `COUNT(*)` sizing, `to_json` per-row capture, the #226 live-cert ledger; `tests/warehouse/{_fake_databricks.py,test_databricks_adapter.py,test_databricks_sql_parse.py}`). `src/signalforge/warehouse/_sample_id.py` — shared deterministic-sample-id seam. `src/signalforge/warehouse/` — current implementation. `tests/warehouse/_fake.py` — `FakeBigQueryClient` + `expect_*` API. `docs/warehouse-adapter-ops.md` — operational reference.