Skip to content
Markdown

ADR 0019 — Custom-SQL checks ride kind='expectation' via GX UnexpectedRowsExpectation

  • Status: Accepted — amended 2026-08-08 (see the amendment section below: the Unity Catalog half of the gating decision had never worked and needed a runner branch)
  • Date: 2026-06-14
  • Deciders: @TheurgicDuke771
  • Related: ADR 0003 (GX-only v1), 0012 (the check.kind seam — and why custom-SQL is not a new kind), 0016 (binary fallback when there's no bandable metric), 0010 (least-privilege connection roles)

Context

The Week-4 plan calls for a Monaco custom-SQL check editor, and the progress ledger flagged it backend-blocked: "need a custom-SQL check kind." That assumption predates a close look at GX Core.

GX Core ships UnexpectedRowsExpectation: a Batch Expectation that runs a user-supplied SQL query and fails if the query returns one or more rows (success = zero rows). The query references the run target with a {batch} placeholder. Crucially, it rides the machinery we already have:

  • Check already has expectation_type: str (free-form, title-cased to a GX class at run time) + config: JSONB (free-form GX kwargs). Check CRUD validates neither — they are pass-through.
  • gx_runner._to_gx_expectation is generic: getattr(gxe, PascalCase(expectation_type))(**config). unexpected_rows_expectation → gxe.UnexpectedRowsExpectation(unexpected_rows_query=...) needs no runner change.
  • The run / result / severity / dry-run path is kind='expectation'-shaped and already exercised.

A de-risk run through the real path (an UnexpectedRowsExpectation against the dev Postgres) confirmed it end-to-end: a 0-row query → success=True, observed_value=0; a row-returning query → success=False, observed_value=<unexpected row count>; {batch} substitution and to_suite_outcome mapping both work, with zero changes to gx_runner.

A distinct kind='custom_sql' (the original assumption) would mean a migration to add the kind and relax expectation_type NOT NULL, plus kind-based runner dispatch — all to re-implement what UnexpectedRowsExpectation already gives under kind='expectation'. The check.kind seam (ADR 0012) exists for auto-monitors (freshness / volume / …) that are not expectations and produce a measured scalar instead of a row-level assertion. Custom-SQL is a row-level assertion expressed in SQL — it belongs under expectation, not as a sibling kind.

Decision

A custom-SQL check is a GX UnexpectedRowsExpectation, persisted as a normal kind='expectation' check: expectation_type='unexpected_rows_expectation', config={"unexpected_rows_query": "<SQL with {batch}>"}. No new kind, no migration, no gx_runner change. The progress-ledger "needs a new kind" note is hereby revised.

What v1 does add — guardrails, because this is the first path that executes user-authored SQL against a live warehouse:

  1. Read-only, single-statement validation (app layer). The query must be a single statement and read-only: SELECT/WITH only; reject DML/DDL/DCL (INSERT/UPDATE/DELETE/MERGE/TRUNCATE/DROP/ALTER/CREATE/GRANT/REVOKE/…) and statement-chaining (a stray ; with a trailing statement). Enforced in check_service create/update and suite import, so it can't be smuggled in through any authoring path.
  2. Datasource gating. Custom-SQL is offered only for SQL-queryable datasources — Snowflake and Unity Catalog. Flat-file stores (ADLS / S3) are GX DataFrame assets, not SQL, so a custom-SQL check there can never run; reject it at author time against the suite's connection type.
  3. Defense-in-depth, not app-layer-only. App-layer parsing is best-effort (it is not a SQL firewall); the real boundary is the connection's least-privilege role (ADR 0010). The ADR + connection docs state that a connection used for custom-SQL should authenticate as a read-only role, and warehouse statement timeouts bound runaway queries. Where the lexer can't be sure it fails closed: an unterminated string/comment (the rest of the query was swallowed as literal text) is rejected, not accepted; and backtick is not treated as a string quote (Snowflake/UC don't quote with it, so a `-span would be a hiding spot — backtick content is scanned as code). The guardrail runs on every path that authors or executes the query — check create/update, suite import, and dry-run (the path that actually runs the SQL before save).
  4. Binary pass/fail in v1. UnexpectedRowsExpectation emits an unexpected row count (observed_value), not an unexpected-percent, so severity.extract_metric finds no bandable metric and the check resolves binary pass/fail (ADR 0016 fallback) — exactly right for "this query should return no rows." Banding severity on the row count (populating metric_value from the count) is a deferred enhancement, not a v1 need.

Consequences

Positive - Smallest possible surface: no migration, no new kind, no runner branch. The custom-SQL check flows through the existing run/result/severity/dry-run path unchanged — the GX-only-v1 architecture (ADR 0003) pays off again. - Security is concentrated in one validation module reused by every authoring path, with the connection role as the real backstop. - Forward-compatible: if count-based severity is wanted later, it's an additive change in severity.extract_metric (read observed_value for this expectation_type), no schema change.

Negative / watch - expectation_type stays free-form (no server allowlist), so the SQL guardrail — not a type allowlist — is the control for this path. A general expectation_type allowlist is noted as optional later hardening (it would also catch typo'd expectation types). - App-layer read-only parsing can be fooled by exotic SQL; we accept that and lean on the least-privilege role. We do not claim the parser is a security boundary. - {batch} is a GX-owned placeholder; the editor/docs must teach it, and a query that forgets it (a bare table name) runs against whatever the author typed, not the suite's run target — a correctness footgun the dry-run preview helps surface.

Amendment — 2026-08-08: the Unity Catalog half needed a runner branch after all

Status: Accepted. The decision above stands for Snowflake exactly as written. Two of its claims were wrong for Unity Catalog, and stayed wrong from this ADR's date until fixed — every UC custom-SQL check errored, in a run and in a dry-run, for the whole of that period. Nobody noticed because no test paired the two: the round-trip test (test_custom_sql_gx.py) stands Postgres in "for the warehouse (which has no live connect in CI)", and every UC runner test used ordinary expectations, which work fine on a DataFrame.

What was wrong.

  1. §Decision says "no gx_runner change" and §Consequences says "no runner branch". True of gx_runner — it is still untouched — but not of the runners. It holds for Snowflake because SnowflakeCheckRunner already builds a SQL batch (add_snowflake → add_table_asset). UnityCatalogCheckRunner builds a pandas DataFrame batch (the DQX swap-in shape, CLAUDE.md §5), and UnexpectedRowsExpectation depends on unexpected_rows_query.table / .row_count, both declared @metric_value(engine=SqlAlchemyExecutionEngine) with no pandas provider. So GX raised No provider found for unexpected_rows_query.table using PandasExecutionEngine before any query ran.

  2. §Decision 2 (datasource gating) reasons that custom SQL is offered on "SQL-queryable datasources — Snowflake and Unity Catalog", because "flat-file stores are GX DataFrame assets, not SQL". The premise was applied to the connection when the thing that decides is the batch the runner builds. By that test UC was, at the time, in the same position as ADLS/S3.

What changed. UnityCatalogCheckRunner.run_checks now partitions: custom-SQL checks run against a GX Databricks-SQL batch over the same table; every other expectation keeps the DataFrame batch; outcomes merge back in submission order. Deliberately GX's own SQL datasource rather than a hand-rolled COUNT/LIMIT, so the semantics are identical to the Snowflake path by construction — and so no new SQL-string interpolation is introduced, leaving the guardrails in §Decision 1–3 inherited unchanged rather than re-implemented.

What did not change. The persisted shape (kind='expectation', expectation_type='unexpected_rows_expectation', config={"unexpected_rows_query": …}), the guardrail module and every authoring path that calls it, gx_runner, and §Decision 4's binary pass/fail for a no-threshold check (count-based severity for a check WITH thresholds landed separately — see the 2026-08-08 amendment below).

One new constraint, UC only. GX's DatabricksDsn requires catalog and schema on the connection URL, and an unqualified name would resolve against the session default — a wrong table read rather than an error. So a UC suite target with no schema makes its custom-SQL checks error (its other checks are unaffected). Snowflake is unchanged: its schema comes from the connection config.

The lesson, which is the reason this amendment exists rather than a silent fix: the original validation section below records a de-risk run "against the dev Postgres" and calls it confirmation of the decision. It confirmed the decision for a SQL backend. The gating list it justified named two datasources, and only one of them had ever been executed — a capability declared from reasoning about a connection rather than from running the code path, a pattern that has recurred in this project. See docs/feature-matrix.md footnote ᶜ for the live numbers that now back the UC tick.

Amendment — 2026-08-08: count-based severity landed

Status: Accepted. §Decision 4's binary pass/fail is unchanged for a check with no thresholds configured (still exactly ADR 0005's binary fallback) — this only fills in the deferred half: metric_value is now populated for unexpected_rows_expectation, additively in severity.extract_metric, exactly as §Consequences predicted ("no schema change").

The bandable quantity is the raw unexpected row COUNT, not a percentage of the batch. A percentage was the other option on the table (row count isn't comparable across differently-sized tables in the abstract), but it was rejected for two reasons: it would need a second aggregate query issued by the runner for the batch's total row count, which severity.py — deliberately pure, no DB/GX access — cannot issue itself and which this ADR's "additive... no schema change" framing never anticipated; and, more fundamentally, the cross-table comparability a percentage buys isn't needed by either consumer. Both the severity thresholds and the anomaly baseline evaluate a check's metric against that same check's own history on the same query/table — never against a different check's metric — so a raw count trends and baselines exactly as a percentage would, at zero extra query cost.

No schema change, no backfill. The results.metric_value column already existed (Week-3 threshold migration). Existing result rows keep metric_value = NULL — nothing backfills a value GX never measured for them.

Unchanged, on purpose: a custom-SQL check with no thresholds configured still resolves as a plain pass/fail — derive_status's no-thresholds branch is checked before any metric comparison, so populating metric_value cannot make an unbanded check start banding.

Validation

De-risk script confirmed the GX round-trip (pass/fail + observed_value + {batch}) through the unchanged gx_runner against the dev Postgres. PR 1 lands the validation module + datasource gating + an adversarial SQL battery (DML / multi-statement / comment-smuggled / empty) and the GX round-trip test; the dry-run path (PR 2) lets the editor preview before save; the Monaco editor (PR 3) authors unexpected_rows_query and mirrors the read-only validation client-side.