diff --git a/docs/adr/0035-advanced-mode-sql-ddl.md b/docs/adr/0035-advanced-mode-sql-ddl.md index c40d523..91e705a 100644 --- a/docs/adr/0035-advanced-mode-sql-ddl.md +++ b/docs/adr/0035-advanced-mode-sql-ddl.md @@ -3,10 +3,12 @@ ## Status Accepted. Design agreed with the user (2026-05-24); the approach is -**validated end-to-end by sub-phase 4a** (`CREATE TABLE`, implemented -2026-05-25 — plan `docs/plans/20260524-adr-0035-sql-ddl-4a.md`), so the -decision is accepted while the remaining sub-phases (**4a.2, 4b–4i**, -§13) continue. This is **Phase 4** of the ADR-0030 roadmap (the +**validated end-to-end by sub-phases 4a / 4a.2 / 4a.3** (`CREATE TABLE` +with column- and table-level constraints, implemented 2026-05-25 — +plans `docs/plans/20260524-adr-0035-sql-ddl-4a.md`, +`…-4a2.md`, `…-4a3.md`), so the decision is accepted while the remaining +sub-phases (**4b–4i**, §13) continue. This is **Phase 4** of the +ADR-0030 roadmap (the advanced-mode SQL surface), the peer of ADR-0031 (expression grammar), ADR-0032 (`SELECT`), and ADR-0033 (DML). It **clarifies ADR-0030 §4** on how DDL is represented and executed. @@ -201,10 +203,12 @@ is no simple-mode rename-table verb. It needs a genuinely new low-level operation (none exists today): within one transaction, rename the table in the database, rename its `data/.csv` file, and update every metadata row that names it — the column-metadata -rows, and **both ends of any relationship** in -`__rdbms_playground_relationships` that references the old name. Name -validation and `__rdbms_*` rejection apply to the target. This closes -the rename half of `C1` for the advanced surface. +rows, **both ends of any relationship** in +`__rdbms_playground_relationships` that references the old name, and +**the table-level CHECK rows** in `__rdbms_playground_table_checks` +(added in 4a.3; keyed by `table_name`). Name validation and +`__rdbms_*` rejection apply to the target. This closes the rename half +of `C1` for the advanced surface. ### 7. Column type conversion — one engine, mode-appropriate policy @@ -332,23 +336,34 @@ ADR-0033's structure: field, detected on read via the UNIQUE-constraint index (`PRAGMA index_list` origin `u`), round-tripped through YAML, with save/load/rebuild tests. -- **4a.3 — Table-level / multi-column `CHECK(…)`.** Split from 4a.2 - (2026-05-25, user-confirmed) because SQLite exposes **no PRAGMA for - CHECK constraints**, so a table-level CHECK cannot be read back from - the engine and needs a **new `__rdbms_*` metadata table** as its - source of truth (the ADR-0012/0013 pattern) — a distinct - architectural step. Until 4a.2/4a.3 land, 4a rejects these forms - "not yet supported". (The general rule: a DDL feature needs new - model/execution work only when it introduces a structure simple mode - could never produce, or an expression the structural helper cannot - consume — cf. the `UNIQUE`-index flag in 4d and the rename op in 4h.) +- **4a.3 — Table-level / multi-column `CHECK(…)`.** *(Implemented + 2026-05-25 — plan `docs/plans/20260525-adr-0035-sql-ddl-4a3.md`.)* + Split from 4a.2 (2026-05-25, user-confirmed) because SQLite exposes + **no PRAGMA for CHECK constraints**, so a table-level CHECK cannot be + read back from the engine and needs a **new `__rdbms_*` metadata + table** as its source of truth (the ADR-0012/0013 pattern) — a + distinct architectural step. Landed as + `__rdbms_playground_table_checks (table_name, seq, check_expr)`; the + builder distinguishes a table-level CHECK from a column-level one by + element position (no column-def open). Composite `UNIQUE` deliberately + stays PRAGMA-detected (engine-reportable, unlike CHECK). (The general + rule: a DDL feature needs new model/execution work only when it + introduces a structure simple mode could never produce, or an + expression the structural helper cannot consume — cf. the + `UNIQUE`-index flag in 4d and the rename op in 4h.) - **4b — Foreign keys in `CREATE TABLE`.** Inline `REFERENCES` + table-level `FOREIGN KEY` → relationship metadata, one undo step. - **4c — `DROP TABLE [IF EXISTS]`** → `SqlDropTable` (cascade parity; `IF EXISTS` no-op-with-note, §4). - **4d — `CREATE [UNIQUE] INDEX` / `DROP INDEX`** → `SqlCreateIndex` / `SqlDropIndex` (ADR-0025; the `UNIQUE` flag extension if needed). -- **4e — `ALTER TABLE` add/drop/rename column.** +- **4e — `ALTER TABLE` add/drop/rename column.** Drop/rename column + must guard against a **table-level CHECK that references the column** + (4a.3): today the rebuild rejects it cleanly via the engine (the + rebuilt table's `CHECK` names a missing column), leaving the table + intact — 4e adds the up-front detection (parsing column references + out of the raw CHECK text in `__rdbms_playground_table_checks`) so the + refusal is deliberate; the friendly wording itself is **H1** work. - **4f — `ALTER TABLE … ALTER COLUMN TYPE`** (the §7 conversion model + the lossy-with-note path). - **4g — `ALTER TABLE` add/drop constraint, add foreign key.** diff --git a/docs/adr/README.md b/docs/adr/README.md index 331125d..b5b9cbe 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -40,4 +40,4 @@ This directory contains the project's ADRs, recorded per - [ADR-0032 — The full SQL `SELECT` grammar](0032-sql-select-grammar.md) — **Accepted**, the Phase-2 grammar commissioned by ADR-0030 §3: full `SELECT` with `INNER`/`LEFT`/`RIGHT`/`FULL OUTER`/`CROSS` joins, `GROUP BY`/`HAVING`, all four set ops (`UNION`/`UNION ALL`/`INTERSECT`/`EXCEPT`), `WITH` and `WITH RECURSIVE` CTEs, `LIMIT … OFFSET`, `DISTINCT`, `t.*`, and bare-alias projection (lifting Phase-1 §4.2); additive extensions to ADR-0031's `sql_expr` for scalar subqueries, `IN (SELECT …)`, `[NOT] EXISTS`, and qualified column refs (redeeming ADR-0031 §7 OOS-1/OOS-2); grammar-recursion via `Subgrammar(&SQL_SELECT_COMPOUND)` reuses ADR-0026's `MAX_SUBGRAMMAR_DEPTH = 64` cap unchanged; **softens ADR-0030 §8's "ambient assistance comes for free" claim**: completion scope needs new `WalkContext` accumulators (a `from_scope_stack` of `ScopeFrame`s holding `from_scope` / `cte_bindings` / `projection_aliases`), a **new walker node variant `Node::ScopedSubgrammar(&Node)`** as the push/pop trigger (existing `Node::Subgrammar` unchanged so DSL `Expr` and `sql_expr` recursion are unaffected), qualified-prefix completion narrowing, body-projection-derived CTE column resolution (so `SELECT *` and explicit-projection CTE bodies both yield real column completion past `cte_alias.|`), and a **post-walk fixup pass** that re-resolves projection-list identifier highlighting/validity once `FROM` is parsed (the projection-before-FROM problem); classifies every Phase-2 validation case against ADR-0027's ERROR/WARNING guideline (§11): five new `diagnostic.*` keys for parse-time-detectable cases (unknown qualifier, ambiguous column, projection-alias misplaced, CTE/compound arity mismatch) plus eight `engine.*` translation keys; a MatchedPath-walking predicate-warnings variant that closes the Phase-1 gap where SQL `WHERE` expressions emitted no `LIKE`-on-numeric / `= NULL` / type-mismatch warnings (ADR-0027 Amendment 1 finally extends to the SQL surface); adds a worker-side post-prepare type-resolution pass via engine column-origin metadata so bare column refs recover their playground type (partially lifting Phase-1 §4.5, the bool→0/1 case) — `Cargo.toml` gains `column_metadata` to rusqlite features (verified against pinned 0.39.0); `__rdbms_*` rejection extended to every new table-source slot; Amendment 1 narrows §12's resolution rule from a grammar-side structural classification to "trust the engine's column-origin metadata verbatim" after an empirical probe showed origin metadata follows through non-recursive CTEs, scalar subqueries, derived tables, set ops, and joins — the one structural exception is recursive CTE result columns, which return None and stay typeless; Amendment 2 records that §10.6's "rewrite the highlight class" prescription is realised via the two-pass schema-existence diagnostic + the renderer's diagnostic-overlay path (no separate per-byte rewrite step needed; no new HighlightClass variant), and that the projection-before-FROM completion narrowing has been improved by an `src/completion.rs` look-ahead probe when the leading walk's `from_scope` is empty but the full input parses - [ADR-0033 — The full SQL DML grammar (`INSERT` / `UPDATE` / `DELETE`)](0033-sql-dml-grammar.md) — **Accepted** (implemented + verified through sub-phase 3k, 2026-05-23; phase-exit report `docs/handoff/20260523-phase-3-verification.md`), the Phase-3 grammar commissioned by ADR-0030 §3: single- and multi-row `INSERT` (incl. `INSERT … SELECT` recursing through ADR-0032's `SQL_SELECT_COMPOUND`), `UPDATE` with `SET` assignment list, `DELETE`, all three optionally followed by `RETURNING projection_list`, plus full `ON CONFLICT … DO NOTHING / DO UPDATE` UPSERT on INSERT; **fixes the DSL-vs-SQL dispatch architecture for shared entry words (`insert`/`update`/`delete`)**: SQL-first / DSL-fallback in Advanced mode via a `Choice(SQL_shape, DSL_shape)` per shape, gated by a new walker capability `Node::Guard(fn)` — a zero-byte-consumption gating node that fails the enclosing Seq with a `ValidationError`; carries `Command::SqlInsert` / `SqlUpdate` / `SqlDelete` variants and `do_sql_*` worker handlers each of which knows the target table (for re-persistence) and the `returning: bool` flag (for DataResult routing); `shortid` auto-fill mirrors the DSL `do_insert` mechanism via worker post-fill; SQL DELETE produces the same per-relationship cascade summary the DSL DELETE does (ADR-0014 parity); three new walker diagnostics (`insert_arity_mismatch` ERROR, `auto_column_overridden` WARNING, `not_null_missing` WARNING) with positive + negative tests each; OOS list explicitly carves out `DEFAULT VALUES` (the project's planned seed feature), SQLite-specific `OR REPLACE` / `OR IGNORE` / `OR ABORT` / `OR FAIL` / `OR ROLLBACK` prefixes, `UPDATE FROM` multi-table updates, and WITH-prefixed DML; the `excluded` keyword inside `ON CONFLICT DO UPDATE` is a deliberate carve-out from ADR-0030 §7's engine-neutral posture (no standard-SQL UPSERT spelling exists that SQLite and PostgreSQL share); eleven phased sub-phases each with explicit exit gates + written DA gate, opening with the dispatch mechanism before any DML grammar lands; initial DA review recorded seven critiques that were resolved before status moved to Proposed; **Amendment 1 supersedes §2's dispatch mechanism**: the originally-chosen `Node::Guard(fn)` + `Choice(SQL_shape, DSL_shape)` was found during 3a to be unworkable as framed (any guard-in-`Choice` mechanism forces a `walk_choice` change — `walk_choice` only falls through on `NoMatch`, so Simple-mode valid-DSL would wrongly surface "this is SQL", and `walk_seq` treats a `NoMatch` past `idx 0` as a hard `Failed`, breaking Advanced-mode DSL fall-through); replaced by **category-grouped, mode-aware dispatch** in `walker::walk` (each `REGISTRY` entry tagged `CommandCategory::{Simple, Advanced}`, generalising the existing whole-command `is_advanced_only` gate), shared entry words carrying a node in both groups, no `Node::Guard` and no `walk_choice`/`walk_seq` change, advanced-mode completion SQL-first with DSL as a full-line fallback; **Amendment 2 (sub-phase 3f) supersedes §7's cascade mechanism**: the WHERE-injected per-child pre-count rested on a premise that was factually wrong about the DSL handler (which detects cascades by before/after row-count diffing inside a transaction, not by `Expr`-derived pre-count subqueries) and would have broken the §2 parity promise by reporting `SET NULL` the DSL path doesn't; replaced by mirroring `do_delete`'s count-diff exactly (verbatim DELETE executes, child-count diff observes the cascade — `ON DELETE CASCADE` row removals only, SET NULL deferred for both paths to preserve parity), which shares the render-layer formatter for free via `CommandOutcome::Delete` and **withdraws risk R2** (no WHERE-byte extraction, no N+1 subquery); **Amendment 3 (sub-phase 3j) records the command-identity model and defers the execution-mode side-channel**: a command is the typed outcome of a *mode-rooted* grammar path and its identity is intrinsic (Advanced mode tries SQL first, falls back to the *Simple* DSL command when no SQL branch matches a token, e.g. `delete … --all-rows`; note `update … --all-rows` does *not* fall back — the SQL `SET` expression eats `--all-rows`, harmless since the engine treats it as a comment); **Simple mode commits the DSL candidate for shared words** so the *real* DSL error surfaces, and when that line would also run in advanced mode the rendering layer **combines** them — DSL error **plus** an `advanced_mode.also_valid_sql` pointer ("… (valid as SQL in advanced mode)") — keeping the actionable DSL fix while pointing at advanced mode; bare "this is SQL" is reserved for entry words with no DSL form (`select`/`with`); a fully-overlapping input (`insert … values …`) legitimately yields *two distinct commands* (`Command::Insert` typed-AST vs `Command::SqlInsert` validated-text) that do the same thing but execute differently (ADR-0030 §4), so each is tested in the mode that produces it; **corrects the plan's 3j exit-gate premise** that the DSL DML tests run in Simple mode (they call `parse_command`, which defaults to Advanced) — the real invariant is "Simple-mode behaviour unchanged, Advanced mode SQL-first, DSL grammar tested in Simple mode, both variants tested in their producing mode", with §6/§7 parity keeping the paths observably equivalent; and **defers to its own future ADR** the execution-time mode side-channel (three-way `Mode`: simple/advanced/advanced-one-shot threaded through `Action`→worker, for mode-dependent *output* like echoing generated SQL) — today only the *rendering* side-channel `OutputLine.mode_at_submission` exists, and the three-way distinction is not required for Phase 3 dispatch correctness - [ADR-0034 — `history.log` as a complete command journal; replay reads success-only](0034-history-journal-and-replay-filter.md) — **Accepted**, resolves a three-way tension in `history.log`'s roles found while implementing ADR-0033 3f: (1) the persistent log is success-only while the in-memory Up/Down recall ring records *every* submission (success or failure, "so users can recall and edit typo'd commands"), and the ring is re-seeded from the log on project open — so **failed commands are recallable within a session but silently lost across sessions**; (2) replay wants the state-building (successful) commands while recall wants everything typed, which one success-only file cannot serve; (3) `replay history.log` never actually worked — `run_replay` parses each whole line through the DSL parser with no understanding of the `||` record shape, so a real log fails on line 1, and **no test ever fed the pipe format to replay** (the `replay_history_log_records_subcommands_only` test only checks what replay *writes*, never replays the log as input). Decision: `history.log` becomes a **complete journal** — every submission recorded, tagged `ok`/`err` via the status field the format already reserved (ADR-0015 §5) — and **each consumer filters**: hydration reads all records (cross-session recall matches in-session), replay reads `ok` only (and learns the journal format, while still accepting bare-command `.commands` scripts; detection by the leading timestamp+status prefix so a `|` inside a bare command isn't misread). Successful commands stay journalled transactionally by the worker; failed commands are journalled `err` best-effort from the runtime/app error path (a parse failure never reaches the worker). Amends ADR-0006's "successfully executed" wording and ADR-0015 §5 ("status always `ok`") / §12 (hydration). Code deferred to two tracked test-first sub-tasks (journal-failures+filtering; replay-parses-journal-format); existing all-`ok` logs need no migration; **implemented 2026-05-24** (plan `docs/plans/20260524-adr-0034-history-journal.md`); **Amendment 1 (2026-05-24): replay filters out app-lifecycle commands** — a working `replay history.log` (the §3 fix) exposed that the journal also records `save as`/`load`/`new`/`export`/`import`/`rebuild`/`mode` (which would panic the worker dispatch or abort the replay), so replay now re-applies **only** schema/data write commands and **skips** every `Command::App` + nested `Command::Replay`; **all skips continue** (never abort — reversing the prior nested-`replay` refusal, so a journal containing a once-run `replay` needs no hand-editing, and the infinite-loop footgun is closed by construction), with a `[skip]` **warning** on `import` and nested-`replay` skips (their omission can leave replayed state incomplete) and silent skips for the rest; `replay.error_nested` removed, `replay.skipped_import`/`replay.skipped_replay` added, `ReplayCompleted` carries `warnings` -- [ADR-0035 — Advanced-mode SQL DDL](0035-advanced-mode-sql-ddl.md) — **Accepted** (design agreed 2026-05-24; validated end-to-end by sub-phase 4a `CREATE TABLE`, implemented 2026-05-25; 4a.2/4b–4i pending), **Phase 4** of the ADR-0030 roadmap (peer of 0031/0032/0033) and **clarifies ADR-0030 §4**. Advanced-mode `CREATE`/`DROP`/`ALTER TABLE` + `CREATE`/`DROP INDEX` get their **own per-statement commands** (`SqlCreateTable`/`SqlAlterTable`/`SqlDropTable`/`SqlCreateIndex`/`SqlDropIndex`), like DML's `Sql*` set — but unlike DML they **execute *structurally*, not verbatim** (raw execution would lose the playground's types, named relationships, and `STRICT`; "verbatim" was a DML convenience, not a rule). Handlers **reuse the low-level schema/metadata helpers** where the operation matches simple mode and **stand alone where the SQL surface is richer** (clarity over forced refactoring); simple mode is untouched (additive). Dispatch: `create`/`drop` reuse ADR-0033 Amendment 1's category-grouped mode-aware dispatch (SQL-first, simple fallback); `alter` is a new advanced-only entry word. Full surface (no pre-emptive cuts, `Q4`): `CREATE TABLE` with column + table constraints, single/compound `PRIMARY KEY`, inline + table-level `FOREIGN KEY` → **named relationships** (one statement = one command = **one undo step**, ADR-0006); `ALTER TABLE` add/drop/rename column, `ALTER COLUMN TYPE`, add/drop constraint, add FK, **`RENAME TO`** (advanced-only table rename — new low-level op renaming the table + its CSV + the relationship metadata, closing the rename half of `C1`); `CREATE [UNIQUE] INDEX` / `DROP INDEX`. Type slot accepts the ten playground keywords **and** standard-SQL aliases (`integer`→`int`, `varchar`→`text`, `timestamp`→`datetime`, …; length args accepted-and-ignored; no engine type names in/out — ADR-0030 §5). `CHECK`/`DEFAULT` reuse ADR-0031 `sql_expr`. **Pre-implementation `/runda` refinements (2026-05-24, user-confirmed):** `CREATE TABLE`/`DROP TABLE` **admit `IF [NOT] EXISTS`** (no-op-that-succeeds-with-a-note — a near-universal cross-vendor idiom, reclassified *into* scope, not engine-specific); `INTEGER PRIMARY KEY` maps to a **plain `int`** PK, *not* auto-increment (`serial` stays the sole auto-increment type). **Column-type-conversion is unified** (ADR-0017 engine, mode-appropriate policy): clean auto-converts and incompatible/own-type-static cases refuse in both modes, but a **lossy** change refuses-by-default in simple mode (`--force-conversion` opts in) while advanced mode **performs it with a loss note** and relies on **`undo` as the safety net** — no force flag, no dropping to simple mode (a payoff of shipping ADR-0006 first). OOS: views/triggers/txn-control/PRAGMA/etc. (ADR-0030 §3), the Postgres `USING` clause, and the DSL→SQL teaching echo (ADR-0030 Phase 5). Sub-phases 4a–4i, plus **4a.2** (per-column `CHECK`/`DEFAULT` via raw `sql_expr` text — `sql_expr` is validate-only, no `Expr` AST — + composite `UNIQUE(a,b)`; no new internal table) and **4a.3** (table-level/multi-column `CHECK`, split off because SQLite has no PRAGMA for CHECK so it needs a new `__rdbms_*` metadata table); 4a rejects all of these "not yet supported" until they land. Each sub-phase has exit + DA gates +- [ADR-0035 — Advanced-mode SQL DDL](0035-advanced-mode-sql-ddl.md) — **Accepted** (design agreed 2026-05-24; validated end-to-end by sub-phases 4a/4a.2/4a.3 `CREATE TABLE`, implemented 2026-05-25; 4b–4i pending), **Phase 4** of the ADR-0030 roadmap (peer of 0031/0032/0033) and **clarifies ADR-0030 §4**. Advanced-mode `CREATE`/`DROP`/`ALTER TABLE` + `CREATE`/`DROP INDEX` get their **own per-statement commands** (`SqlCreateTable`/`SqlAlterTable`/`SqlDropTable`/`SqlCreateIndex`/`SqlDropIndex`), like DML's `Sql*` set — but unlike DML they **execute *structurally*, not verbatim** (raw execution would lose the playground's types, named relationships, and `STRICT`; "verbatim" was a DML convenience, not a rule). Handlers **reuse the low-level schema/metadata helpers** where the operation matches simple mode and **stand alone where the SQL surface is richer** (clarity over forced refactoring); simple mode is untouched (additive). Dispatch: `create`/`drop` reuse ADR-0033 Amendment 1's category-grouped mode-aware dispatch (SQL-first, simple fallback); `alter` is a new advanced-only entry word. Full surface (no pre-emptive cuts, `Q4`): `CREATE TABLE` with column + table constraints, single/compound `PRIMARY KEY`, inline + table-level `FOREIGN KEY` → **named relationships** (one statement = one command = **one undo step**, ADR-0006); `ALTER TABLE` add/drop/rename column, `ALTER COLUMN TYPE`, add/drop constraint, add FK, **`RENAME TO`** (advanced-only table rename — new low-level op renaming the table + its CSV + the relationship and table-CHECK metadata, closing the rename half of `C1`); `CREATE [UNIQUE] INDEX` / `DROP INDEX`. Type slot accepts the ten playground keywords **and** standard-SQL aliases (`integer`→`int`, `varchar`→`text`, `timestamp`→`datetime`, …; length args accepted-and-ignored; no engine type names in/out — ADR-0030 §5). `CHECK`/`DEFAULT` reuse ADR-0031 `sql_expr`. **Pre-implementation `/runda` refinements (2026-05-24, user-confirmed):** `CREATE TABLE`/`DROP TABLE` **admit `IF [NOT] EXISTS`** (no-op-that-succeeds-with-a-note — a near-universal cross-vendor idiom, reclassified *into* scope, not engine-specific); `INTEGER PRIMARY KEY` maps to a **plain `int`** PK, *not* auto-increment (`serial` stays the sole auto-increment type). **Column-type-conversion is unified** (ADR-0017 engine, mode-appropriate policy): clean auto-converts and incompatible/own-type-static cases refuse in both modes, but a **lossy** change refuses-by-default in simple mode (`--force-conversion` opts in) while advanced mode **performs it with a loss note** and relies on **`undo` as the safety net** — no force flag, no dropping to simple mode (a payoff of shipping ADR-0006 first). OOS: views/triggers/txn-control/PRAGMA/etc. (ADR-0030 §3), the Postgres `USING` clause, and the DSL→SQL teaching echo (ADR-0030 Phase 5). Sub-phases 4a–4i, plus **4a.2** (per-column `CHECK`/`DEFAULT` via raw `sql_expr` text — `sql_expr` is validate-only, no `Expr` AST — + composite `UNIQUE(a,b)`; no new internal table) and **4a.3** (table-level/multi-column `CHECK`, landed via the new `__rdbms_playground_table_checks` metadata table because SQLite has no PRAGMA for CHECK; the builder tells a table-level CHECK from a column-level one by element position); FK and the remaining forms stay "not yet supported" until their sub-phases land. Each sub-phase has exit + DA gates diff --git a/docs/plans/20260525-adr-0035-sql-ddl-4a3.md b/docs/plans/20260525-adr-0035-sql-ddl-4a3.md new file mode 100644 index 0000000..bbfd4ba --- /dev/null +++ b/docs/plans/20260525-adr-0035-sql-ddl-4a3.md @@ -0,0 +1,256 @@ +# Plan: ADR-0035 Phase 4, sub-phase 4a.3 — table-level / multi-column `CHECK` + +The constraint slice's second (and final) half. Adds, to advanced-mode +SQL `CREATE TABLE`, the one constraint that needs a **new `__rdbms_*` +metadata table**: a **table-level `CHECK ()`** that can reference +several columns, e.g. `CREATE TABLE t (a int, b int, CHECK (a < b))`. +SQLite exposes **no PRAGMA for CHECK constraints**, so a table-level +CHECK cannot be read back from the engine and must live in metadata as +its source of truth (the ADR-0012/0013 pattern). Builds directly on the +4a/4a.2 `SqlCreateTable` command + grammar. + +## 1. Baseline + +- Tests: **1752 passing, 0 failing, 0 skipped, 1 ignored** (the + `friendly/mod.rs` ` ```ignore ` doctest); clippy clean + (`cargo clippy --all-targets -- -D warnings`). Branch `main`, last + commit `1991fb4` (handoff-37). 4a.3 starts here. + +## 2. Decisions locked with the user (do not re-litigate) + +1. **New metadata table — `__rdbms_playground_table_checks`** (user + confirmed 2026-05-25), focused/minimal, purpose-named like the + existing metadata tables: + + ```sql + __rdbms_playground_table_checks ( + table_name TEXT NOT NULL, + seq INT NOT NULL, -- declaration order + check_expr TEXT NOT NULL, + PRIMARY KEY (table_name, seq) + ) STRICT; + ``` + + It is the **source of truth** for table-level CHECKs; `read_schema` + reads them from here, not PRAGMA. Auto-filtered from `list_tables` + by the `__rdbms_` prefix. A constraint `name` column is **not** added + now — 4g's `ADD CONSTRAINT ` will add it when actually needed. +2. **Composite `UNIQUE` stays PRAGMA-detected** (user confirmed, + 2026-05-25): the PRAGMA/metadata split is principled — engine- + reportable (UNIQUE, PK, FK, indexes) → PRAGMA; not reportable (CHECK, + column + table) → metadata. No churn to shipped 4a.2 code. +3. **Stored as raw SQL text**, like 4a.2's column CHECK: `sql_expr` is + validate-only (no `Expr` AST), so the builder captures the inner + expression text by byte span via `capture_parenthesised_span`. +4. **One undo step**; structural execution reuses `do_create_table`, + which writes the metadata rows inside its existing transaction. +5. **FK stays rejected** (4b). Only the table-level CHECK shape is + lifted from the 4a "not yet supported" parse rejection. + +## 3. Phase 1 — Requirements checklist (4a.3) + +### Functional + +- [ ] Table element `CHECK ()` parses (advanced mode), in any + position among the elements, accepting the full `sql_expr` surface. +- [ ] The builder distinguishes a **table-level** CHECK (no column-def + open in the current element) from a **column-level** CHECK (after a + column's type — 4a.2, unchanged). Depth-aware element-boundary + detection (§4.2). +- [ ] Multiple table-level CHECKs in one statement, preserved in + declaration order (the `seq` column). +- [ ] A table-level CHECK is **enforced** by the engine (a violating + insert fails) and **survives a rebuild** (the part-D proof). +- [ ] The 4a/4a.2 "table-level CHECK not yet supported" parse-rejection + is **lifted**; **FK stays rejected** (4b). +- [ ] Engine-neutral errors; `STRICT` preserved; one undo step. + +### Cross-cutting / round-trip + +- [ ] A table with one or more table-level CHECKs survives save → load + → rebuild (DDL + enforcement). The new metadata table is the source + of truth on read; `schema_to_ddl` re-emits the clauses on rebuild. +- [ ] `project.yaml` round-trips the CHECKs + (`TableSchema.check_constraints`, YAML `#[serde(default)]`, optional + on read — mirrors `unique_constraints`). +- [ ] `history.log` / replay unchanged (part of the same `create` + write command). + +### Testing (ADR-0008 four tiers) + +- [ ] **Tier 1** (builder, `sql_create_table.rs`): table CHECK captured + verbatim, distinct from a column CHECK; multiple table CHECKs ordered; + table CHECK after a length-arg column + column CHECK (the depth probe); + table CHECK after a table-level PK/UNIQUE; nested parens balanced; + FK still rejected (update `table_level_check_and_fk_still_rejected`). +- [ ] **Tier 3** (`tests/sql_create_table.rs`): worker round-trip — + table CHECK enforced (violating insert fails), **survives rebuild**; + multiple CHECKs all enforced. +- [ ] **YAML round-trip unit test** for the metadata field. + +## 4. Architecture & design + +### 4.1 Grammar (`src/dsl/grammar/sql_create_table.rs`) + +Add a table-level CHECK element, mirroring `TABLE_UNIQUE`: + +```rust +static TABLE_CHECK_NODES: &[Node] = &[ + Node::Word(Word::keyword("check")), + Node::Punct('('), + Node::Subgrammar(&sql_expr::SQL_OR_EXPR), + Node::Punct(')'), +]; +const TABLE_CHECK: Node = Node::Seq(TABLE_CHECK_NODES); +``` + +Extend `ELEMENT_CHOICES`: +`&[TABLE_PK, TABLE_UNIQUE, TABLE_CHECK, COLUMN_DEF]`. Order note: a +column literally named `check` is already unavailable (it is a keyword +in the column-constraint set); `TABLE_CHECK` before `COLUMN_DEF` keeps +the table-level form winning at element start. (`COLUMN_DEF`'s own +`CHECK` lives inside `COL_CONSTRAINT_SUFFIX`, so the two never compete +for the same position.) + +### 4.2 Builder distinguisher (the load-bearing mechanism) + +`MatchedKind::Word` carries **no role or node provenance** (only +`Ident` carries `role`), so the table-level and column-level `check` +keywords are indistinguishable by kind. Distinguish by **element +position**, depth-aware: + +- Track `depth` over the top-level item stream: `+1` on each `Punct('(')` + that reaches the loop, `-1` on each `Punct(')')`. The column-list + interior is `depth == 1`. (Parens consumed inside the `check` / + `default` capture helpers and the table-`unique` sub-loop never reach + the loop, so they don't perturb `depth`; the outer list parens, type + length-args `(10, 2)`, and table-`PRIMARY KEY (a, b)` parens **do**, + and balance.) +- Track `column_open: bool` — set `true` when a `col_type` / `double` + finalises a column; reset `false` on a `Punct(',')` at `depth == 1` + (an element separator). +- On `MatchedKind::Word("check")`: capture the parenthesised span as + today; then **route by `column_open`** — `true` → column-level + (`columns.last_mut().check_sql`, 4a.2 behaviour); `false` → + table-level (push raw text onto `check_constraints`). + +This is **verified by probe tests**, not reasoning — in particular the +`a numeric(10,2) check (a>0)` case (a naive "reset on any comma" would +misclassify it because the length-arg comma is at `depth == 2`). + +The capture for a table-level CHECK reuses `capture_parenthesised_span` +(`src/dsl/grammar/ddl.rs`) unchanged — `CHECK ( … )` is paren-bounded. + +### 4.3 Command AST (`src/dsl/command.rs`) + +`Command::SqlCreateTable` gains `check_constraints: Vec` (raw +inner SQL texts, declaration order), peer to `unique_constraints`. + +### 4.4 Worker / DDL — both generators in lockstep (`src/db.rs`) + +The two DDL generators **must** emit the table CHECK clauses +identically (the §6.1 rule; the 4a `serial` bug is the cautionary tale): + +- **`do_create_table`** gains a `check_constraints: &[String]` param; + emits `, CHECK (expr)` table clauses (after composite UNIQUE, before + `STRICT`), and **writes the `__rdbms_playground_table_checks` rows** + (one per CHECK, `seq` = index) inside its existing transaction. +- **`schema_to_ddl`** emits the same clauses from + `ReadSchema.check_constraints`. +- **`configure_connection`** creates the new metadata table alongside + the existing `__rdbms_*` tables. +- **`read_schema`** + **`read_schema_snapshot`** read the CHECKs from + the metadata table (ordered by `seq`) into `ReadSchema` / + `SchemaSnapshot` (→ `TableSchema.check_constraints`). +- **`Request::SqlCreateTable`** dispatch passes the new field through to + `do_create_table`; `snapshot_then` wrapping unchanged (one undo step). +- A table drop / rebuild must clear/repopulate the metadata rows — verify + the existing drop path clears `__rdbms_*` rows for the table (it does + for columns/relationships); extend it to the new table. + +### 4.5 Persistence round-trip + +- `TableSchema.check_constraints: Vec` (`src/persistence/mod.rs`). +- `RawTable.check_constraints` with `#[serde(default)]`; + `write_table` emits only when non-empty; `parse_schema` maps it — + all mirroring `unique_constraints` exactly. + +### 4.6 Friendly catalog / keys + +Update the `ddl.sql_create_table` help body and +`parse.usage.sql_create_table` usage skeleton to show the table-level +`CHECK (…)` form. No new keys expected (the parse error for a still- +rejected shape reuses existing keys); if any new diagnostic key is +added, keep `keys.rs` ↔ `en-US.yaml` in lockstep (the validator test) +and engine-neutral (the vocab audit). + +## 5. Out of 4a.3 scope + +- FK (4b); `DROP` (4c); indexes (4d); `ALTER` (4e–4h). +- `CONSTRAINT CHECK (…)` (named constraints) → 4g (adds a `name` + column to the metadata table then). + +## 6. Open items / implementer calls + +1. **Builder distinguisher** (§4.2) — depth-aware; settle by the probe + tests in step 2 before relying on it. +2. **Drop / rebuild cleanup of the new metadata rows** — confirm by + test that dropping (and rebuilding) a table leaves no orphan CHECK + rows and repopulates correctly. +3. **CHECK column-validation at create time** — table CHECKs reference + columns being defined (not yet in the schema cache); confirm by test + they raise no spurious unknown-column `[ERR]` (mirror the 4a.2 + column-CHECK finding; it was fine there). + +## 7. Devil's Advocate review of this plan + +- **Why a new table at all — is CHECK really unreportable?** Yes; + SQLite has no PRAGMA for CHECK (column or table). 4a.2's *column* + CHECK only round-tripped because it was already stored in + `__rdbms_playground_columns.check_expr` for the same reason. A + table-level CHECK has no column to hang on, hence the new table. ✓ +- **Two DDL generators in sync?** The plan emits the CHECK clauses in + **both** `do_create_table` and `schema_to_ddl` and adds a + survives-rebuild test — the exact safety net the 4a `serial` drift + proved necessary. ✓ +- **Distinguisher robust?** The naive comma reset is explicitly + rejected (length-arg / table-PK inner commas); depth-aware detection + is probe-tested, including the `numeric(10,2) check(...)` trap. ✓ +- **Silent scope creep?** FK stays rejected (4b); named CHECK is 4g; + composite UNIQUE deliberately stays on PRAGMA (user-confirmed). ✓ +- **Round-trip detectable on read?** The metadata table is read by both + `read_schema` and `read_schema_snapshot`; YAML mirrors + `unique_constraints`. ✓ +- **Tests first?** §8 orders failing tests before code at every step. ✓ + +## 8. Implementation sequence (test-first) + +1. **Builder probe + Tier-1** — write the distinguisher tests (table + CHECK captured + ordered; the `numeric(10,2) check(...)` depth trap; + table CHECK after table PK/UNIQUE; nested parens; column CHECK still + column-level; update `table_level_check_and_fk_still_rejected` so + table CHECK is accepted and **FK stays rejected**) → red → add + `TABLE_CHECK` grammar + the depth-aware builder branch + + `Command.check_constraints` → green. *(Compiles once the worker + dispatch threads the new field; steps 1–2 land together.)* +2. **Worker + metadata table** — write Tier-3 (`tests/sql_create_table.rs`): + table CHECK enforced (violating insert fails); multiple CHECKs; the + metadata rows present → red → add `__rdbms_playground_table_checks` + in `configure_connection`, the `do_create_table` emission + metadata + writes, the `read_schema` / `read_schema_snapshot` reads, and the + drop-path cleanup → green. +3. **Round-trip** — extend `TableSchema` + YAML + `schema_to_ddl` → + Tier-3 survives-rebuild test + a YAML round-trip unit test → green. +4. **Catalog** — update help/usage bodies; run + `keys_validate_against_catalog` + the vocab audit → green. +5. **Full sweep** — `cargo test` (no regressions from 1752) + + `cargo clippy --all-targets -- -D warnings`. +6. **Docs** — ADR §13 already records 4a.3; update `requirements.md` + Q1 note; flip nothing (ADR already Accepted). Propose the commit + message; wait for approval. + +## 9. Exit gate + +- All §3 checklist items satisfied; four tiers green, zero skips; no + regression from the 1752 baseline; written DA pass on the delivered + slice; clippy clean. diff --git a/docs/requirements.md b/docs/requirements.md index d423eff..ccfdaac 100644 --- a/docs/requirements.md +++ b/docs/requirements.md @@ -218,9 +218,11 @@ handoff-14 cleanup; 449 after B2/C2.) `CREATE TABLE` (ADR-0035, 2026-05-25 — executed structurally: columns + types + `NOT NULL`/`UNIQUE`/`PRIMARY KEY` + `IF NOT EXISTS` (4a), then per-column `DEFAULT`/`CHECK` (raw `sql_expr` text) and composite - `UNIQUE(a,b)` (4a.2)). Remaining DDL — table-level/multi-column - `CHECK` (4a.3), FK (4b), `DROP TABLE` (4c), indexes (4d), - `ALTER TABLE` (4e–4h) — is phased per ADR-0035 §13.)* + `UNIQUE(a,b)` (4a.2), then table-level/multi-column `CHECK` (4a.3 — + round-trips via the new `__rdbms_playground_table_checks` metadata + table, since the engine reports no CHECK constraints)). Remaining DDL + — FK (4b), `DROP TABLE` (4c), indexes (4d), `ALTER TABLE` (4e–4h) — + is phased per ADR-0035 §13.)* - [ ] **Q2** Non-standard syntax rejected with a clear message pointing at the supported subset. *(Design done — ADR-0030 §8: out-of-subset statements are diff --git a/src/db.rs b/src/db.rs index 2708604..eba95e7 100644 --- a/src/db.rs +++ b/src/db.rs @@ -466,6 +466,7 @@ enum Request { columns: Vec, primary_key: Vec, unique_constraints: Vec>, + check_constraints: Vec, if_not_exists: bool, source: Option, reply: oneshot::Sender>, @@ -829,12 +830,14 @@ impl Database { /// Advanced-mode SQL `CREATE TABLE` (ADR-0035 §1, 4a). Executes /// structurally; returns whether the table was created or skipped /// (the `IF NOT EXISTS` no-op, ADR-0035 §4). + #[allow(clippy::too_many_arguments)] pub async fn sql_create_table( &self, name: String, columns: Vec, primary_key: Vec, unique_constraints: Vec>, + check_constraints: Vec, if_not_exists: bool, source: Option, ) -> Result { @@ -844,6 +847,7 @@ impl Database { columns, primary_key, unique_constraints, + check_constraints, if_not_exists, source, reply, @@ -1411,6 +1415,12 @@ const REL_TABLE: &str = "__rdbms_playground_relationships"; /// `created_at`. Created on first connect and only ever /// written by us; the user never touches it directly. const META_PROJECT_TABLE: &str = "__rdbms_playground_meta"; +/// Table-level `CHECK ()` constraints (ADR-0035 §4a.3). The +/// engine exposes no PRAGMA for CHECK constraints, so — unlike UNIQUE / +/// PK / FK, which are read back from PRAGMA — a table-level CHECK has no +/// engine-readable home and this table is its source of truth. One row +/// per CHECK, ordered by `seq` (declaration order). +const CHECK_TABLE: &str = "__rdbms_playground_table_checks"; fn configure_connection(conn: &Connection) -> Result<(), rusqlite::Error> { conn.execute_batch(&format!( @@ -1432,6 +1442,12 @@ fn configure_connection(conn: &Connection) -> Result<(), rusqlite::Error> { on_update TEXT NOT NULL,\n\ PRIMARY KEY (child_table, child_column)\n\ ) STRICT;\n\ + CREATE TABLE IF NOT EXISTS {CHECK_TABLE} (\n\ + table_name TEXT NOT NULL,\n\ + seq INTEGER NOT NULL,\n\ + check_expr TEXT NOT NULL,\n\ + PRIMARY KEY (table_name, seq)\n\ + ) STRICT;\n\ CREATE TABLE IF NOT EXISTS {META_PROJECT_TABLE} (\n\ key TEXT NOT NULL PRIMARY KEY,\n\ value TEXT NOT NULL\n\ @@ -1691,6 +1707,7 @@ fn handle_request( &columns, &primary_key, &[], + &[], )); } Request::SqlCreateTable { @@ -1698,6 +1715,7 @@ fn handle_request( columns, primary_key, unique_constraints, + check_constraints, if_not_exists, source, reply, @@ -1726,6 +1744,7 @@ fn handle_request( &columns, &primary_key, &unique_constraints, + &check_constraints, ) .map(CreateOutcome::Created) }); @@ -2324,6 +2343,7 @@ fn read_schema_snapshot(conn: &Connection) -> Result { primary_key: read.primary_key.clone(), columns, unique_constraints: read.unique_constraints.clone(), + check_constraints: read.check_constraints.clone(), }); } @@ -2569,6 +2589,7 @@ fn read_schema_for_specs(columns: &[ColumnSpec], primary_key: &[String]) -> Read primary_key: primary_key.to_vec(), foreign_keys: Vec::new(), unique_constraints: Vec::new(), + check_constraints: Vec::new(), } } @@ -2606,6 +2627,7 @@ pub enum CreateOutcome { Skipped(TableDescription), } +#[allow(clippy::too_many_arguments)] fn do_create_table( conn: &Connection, persistence: Option<&Persistence>, @@ -2614,6 +2636,7 @@ fn do_create_table( columns: &[ColumnSpec], primary_key: &[String], unique_constraints: &[Vec], + check_constraints: &[String], ) -> Result { if columns.is_empty() { // SQLite requires at least one column. The DSL grammar @@ -2702,6 +2725,17 @@ fn do_create_table( ddl.push(')'); } + // Table-level CHECK constraints (ADR-0035 §4a.3), emitted verbatim + // from the captured raw SQL text. Must stay identical to the + // `schema_to_ddl` rebuild path (the §6.1 two-generators rule). + // The engine has no PRAGMA to report these back, so they are also + // recorded in `CHECK_TABLE` (below) as their source of truth. + for expr in check_constraints { + ddl.push_str(", CHECK ("); + ddl.push_str(expr); + ddl.push(')'); + } + ddl.push_str(") STRICT;"); debug!(ddl = %ddl, "create_table"); @@ -2715,6 +2749,18 @@ fn do_create_table( for (col, check_sql) in columns.iter().zip(&check_sqls) { insert_column_metadata(&tx, name, &col.name, col.ty, check_sql.as_deref())?; } + // Record table-level CHECKs in their metadata table (the engine + // reports no CHECK constraints, ADR-0035 §4a.3). `seq` preserves + // declaration order so read-back / rebuild re-emit them in order. + for (seq, expr) in check_constraints.iter().enumerate() { + tx.execute( + &format!( + "INSERT INTO {CHECK_TABLE} (table_name, seq, check_expr) VALUES (?1, ?2, ?3);" + ), + rusqlite::params![name, seq as i64, expr], + ) + .map_err(DbError::from_rusqlite)?; + } let description = do_describe_table(conn, name)?; let changes = Changes { schema_dirty: true, @@ -2764,6 +2810,12 @@ fn do_drop_table( [name], ) .map_err(DbError::from_rusqlite)?; + // Table-level CHECK metadata goes with the table (ADR-0035 §4a.3). + tx.execute( + &format!("DELETE FROM {CHECK_TABLE} WHERE table_name = ?1;"), + [name], + ) + .map_err(DbError::from_rusqlite)?; let changes = Changes { schema_dirty: true, rewritten_tables: Vec::new(), @@ -4722,6 +4774,11 @@ struct ReadSchema { /// read from the UNIQUE-constraint indexes (`origin = 'u'`). /// Single-column UNIQUE rides on `ReadColumn::unique` instead. unique_constraints: Vec>, + /// Table-level CHECK constraints as raw SQL text, in declaration + /// order (ADR-0035 §4a.3). The engine reports no CHECK constraints, + /// so these are read from `__rdbms_playground_table_checks` rather + /// than PRAGMA, and echoed verbatim by `schema_to_ddl` on rebuild. + check_constraints: Vec, } #[derive(Debug, Clone)] @@ -4842,14 +4899,40 @@ fn read_schema(conn: &Connection, table: &str) -> Result { foreign_keys.push(row.map_err(DbError::from_rusqlite)?); } + // Table-level CHECK constraints (ADR-0035 §4a.3) come from their + // metadata table, not PRAGMA — the engine reports no CHECKs. + let check_constraints = read_table_checks(conn, table)?; + Ok(ReadSchema { columns, primary_key, foreign_keys, unique_constraints, + check_constraints, }) } +/// Read a table's table-level CHECK constraints (ADR-0035 §4a.3) from +/// `CHECK_TABLE`, in declaration order (`seq`). The engine exposes no +/// PRAGMA for CHECK constraints, so this metadata table is their only +/// source of truth. +fn read_table_checks(conn: &Connection, table: &str) -> Result, DbError> { + let mut stmt = conn + .prepare(&format!( + "SELECT check_expr FROM {CHECK_TABLE} \ + WHERE table_name = ?1 ORDER BY seq;" + )) + .map_err(DbError::from_rusqlite)?; + let rows = stmt + .query_map([table], |row| row.get::<_, String>(0)) + .map_err(DbError::from_rusqlite)?; + let mut out = Vec::new(); + for row in rows { + out.push(row.map_err(DbError::from_rusqlite)?); + } + Ok(out) +} + /// Read the user-created indexes on `table` (ADR-0025). /// /// `pragma_index_list` reports every index; we keep only those @@ -5042,6 +5125,13 @@ fn schema_to_ddl(table: &str, schema: &ReadSchema) -> String { clauses.push(format!("UNIQUE ({})", idents.join(", "))); } + // Table-level CHECK constraints (ADR-0035 §4a.3) — echoed verbatim + // from the raw SQL stored in the metadata table, emitted identically + // to `do_create_table` (the §6.1 two-generators rule). + for expr in &schema.check_constraints { + clauses.push(format!("CHECK ({expr})")); + } + for fk in &schema.foreign_keys { clauses.push(format!( "FOREIGN KEY ({child}) REFERENCES {parent_table}({parent_col}) \ @@ -7504,6 +7594,7 @@ fn build_read_schema(table: &TableSchema, relationships: &[RelationshipSchema]) primary_key: table.primary_key.clone(), foreign_keys, unique_constraints: table.unique_constraints.clone(), + check_constraints: table.check_constraints.clone(), } } @@ -11206,6 +11297,7 @@ mod tests { primary_key: vec!["id".to_string()], foreign_keys: vec![], unique_constraints: Vec::new(), + check_constraints: Vec::new(), }; let ddl = schema_to_ddl("T", &schema); assert!( diff --git a/src/dsl/command.rs b/src/dsl/command.rs index 6b571fd..0a9510d 100644 --- a/src/dsl/command.rs +++ b/src/dsl/command.rs @@ -148,6 +148,11 @@ pub enum Command { /// (ADR-0035 §4a.2). Single-column table-level `UNIQUE` is /// folded into the column's `unique` flag instead. unique_constraints: Vec>, + /// Table-level `CHECK ()` constraints, in declaration + /// order, as raw SQL text (ADR-0035 §4a.3). A multi-column + /// CHECK has no column to hang on and the engine reports no + /// CHECKs, so it round-trips via a dedicated metadata table. + check_constraints: Vec, if_not_exists: bool, }, /// Add a column to an existing table. The column carries diff --git a/src/dsl/grammar/ddl.rs b/src/dsl/grammar/ddl.rs index 033ea9d..659082b 100644 --- a/src/dsl/grammar/ddl.rs +++ b/src/dsl/grammar/ddl.rs @@ -1320,7 +1320,20 @@ fn build_sql_create_table(path: &MatchedPath, source: &str) -> Result = Vec::new(); let mut primary_key: Vec = Vec::new(); let mut unique_constraints: Vec> = Vec::new(); + let mut check_constraints: Vec = Vec::new(); let mut pending_name: Option = None; + // Distinguish a table-level `CHECK (…)` from a column-level one + // (ADR-0035 §4a.3): both are spelled `check (`, and `Word` matches + // carry no role, so position is the only signal. `column_open` is + // `true` while a column definition is accepting constraints in the + // current element; a `check` seen while it is `false` is table-level. + // `depth` tracks the parens that reach this loop (the outer column + // list, type length-args `(10, 2)`, and table-`PRIMARY KEY (a, b)` — + // the `check`/`default`/table-`unique` arms consume their own parens + // internally, so they never perturb it). An element separator is a + // comma at the column-list interior, `depth == 1`. + let mut column_open = false; + let mut depth = 0usize; let mut items = path.items.iter().peekable(); while let Some(item) = items.next() { match &item.kind { @@ -1336,6 +1349,7 @@ fn build_sql_create_table(path: &MatchedPath, source: &str) -> Result Result { @@ -1432,14 +1447,31 @@ fn build_sql_create_table(path: &MatchedPath, source: &str) -> Result )` — capture the inner expression text - // (without the wrapping parens) by matching paren depth. + // (without the wrapping parens) by matching paren depth, then + // route by element position: a CHECK inside an open column + // definition is column-level (4a.2); one seen at element + // start (no column open) is a table-level CHECK (4a.3). MatchedKind::Word("check") => { - if let Some((s, e)) = capture_parenthesised_span(&mut items) - && let Some(last) = columns.last_mut() - { - last.check_sql = Some(source[s..e].trim().to_string()); + if let Some((s, e)) = capture_parenthesised_span(&mut items) { + let text = source[s..e].trim().to_string(); + if column_open { + if let Some(last) = columns.last_mut() { + last.check_sql = Some(text); + } + } else { + check_constraints.push(text); + } } } + // Track paren depth for element-boundary detection. The + // column/`check`/`default`/table-`unique` arms consume their + // own parens, so the only parens reaching here are the outer + // column list, type length-args, and table-`PRIMARY KEY (…)`. + MatchedKind::Punct('(') => depth += 1, + MatchedKind::Punct(')') => depth = depth.saturating_sub(1), + // A comma at the column-list interior ends the current + // element — the next element starts fresh (no column open). + MatchedKind::Punct(',') if depth == 1 => column_open = false, _ => {} } } @@ -1461,6 +1493,7 @@ fn build_sql_create_table(path: &MatchedPath, source: &str) -> Result )` (ADR-0035 §4a.3) — a multi-column +// CHECK referencing several columns. Same paren-bounded shape as the +// column-level CHECK; the builder tells them apart by position (a +// CHECK at element start, with no column definition open, is +// table-level). The engine reports no CHECK constraints, so a +// table-level CHECK round-trips via a dedicated metadata table. +static TABLE_CHECK_NODES: &[Node] = &[ + Node::Word(Word::keyword("check")), + Node::Punct('('), + Node::Subgrammar(&sql_expr::SQL_OR_EXPR), + Node::Punct(')'), +]; +const TABLE_CHECK: Node = Node::Seq(TABLE_CHECK_NODES); + +// One element of the column list: a table-level `PRIMARY KEY (…)` / +// `UNIQUE (…)` / `CHECK (…)`, or a column definition. The table-level +// forms are tried first — each starts with a keyword (`primary` / +// `unique` / `check`) that disambiguates it from a column name. (A +// column literally named `primary`/`unique`/`check` is therefore +// unavailable, the same trade real SQL makes with its reserved words.) +static ELEMENT_CHOICES: &[Node] = &[TABLE_PK, TABLE_UNIQUE, TABLE_CHECK, COLUMN_DEF]; const ELEMENT: Node = Node::Choice(ELEMENT_CHOICES); static COLUMN_LIST_NODES: &[Node] = &[ @@ -445,10 +460,20 @@ mod tests { } #[test] - fn table_level_check_and_fk_still_rejected() { - // Table-level (multi-column) CHECK is 4a.3 (needs a metadata - // table); FK is 4b. Neither shape exists here yet. - bad("table t (a int, b int, check (a < b))"); + fn table_level_check_accepted() { + // 4a.3: a table-level (multi-column) CHECK is now admitted, in + // any position among the elements and alongside other forms. + good("table t (a int, b int, check (a < b))"); + good("table t (a int, b int, c int, check (a < b), check (b < c))"); + good("table t (a int, b int, primary key (a), check (a < b))"); + good("table t (a int, b int, unique (a, b), check (a <> b))"); + good("table t (price real check (price >= 0), total real, check (total >= price))"); + } + + #[test] + fn foreign_key_still_rejected() { + // FK in CREATE TABLE is 4b — neither inline `REFERENCES` nor a + // table-level `FOREIGN KEY` shape exists in the grammar yet. bad("table t (id int, ref int references other(id))"); bad("table t (id int, foreign key (id) references other(id))"); } @@ -696,4 +721,91 @@ mod builder_tests { assert!(col(&cols, "b").unique, "it folds into the column's flag"); assert!(!col(&cols, "a").unique); } + + // --- 4a.3: table-level / multi-column CHECK --- + + /// Parse and return the columns + the table-level CHECK constraints. + fn parse_sct_checks(input: &str) -> (Vec, Vec) { + match parse_command(input).expect("should parse") { + Command::SqlCreateTable { + columns, + check_constraints, + .. + } => (columns, check_constraints), + other => panic!("expected SqlCreateTable, got {other:?}"), + } + } + + #[test] + fn table_level_check_captured_as_raw_text() { + let (cols, checks) = parse_sct_checks("create table t (a int, b int, check (a < b))"); + assert_eq!(checks, vec!["a < b".to_string()]); + // The CHECK belongs to no column — it stays table-level. + assert!(col(&cols, "a").check_sql.is_none() && col(&cols, "b").check_sql.is_none()); + } + + #[test] + fn multiple_table_checks_preserve_declaration_order() { + let (_, checks) = + parse_sct_checks("create table t (a int, b int, c int, check (a < b), check (b < c))"); + assert_eq!(checks, vec!["a < b".to_string(), "b < c".to_string()]); + } + + #[test] + fn column_check_and_table_check_route_separately() { + // A column-level CHECK (after a column's type) and a table-level + // CHECK (its own element) in the same statement must not be + // conflated — the load-bearing distinction of 4a.3. + let (cols, checks) = parse_sct_checks( + "create table t (price real check (price >= 0), total real, check (total >= price))", + ); + assert_eq!(col(&cols, "price").check_sql.as_deref(), Some("price >= 0")); + assert!(col(&cols, "total").check_sql.is_none()); + assert_eq!(checks, vec!["total >= price".to_string()]); + } + + #[test] + fn column_check_after_length_arg_stays_column_level() { + // The depth trap: the `,` inside `numeric(10, 2)` is at paren + // depth 2, not an element boundary, so the following `check` + // is still column-level. A naive "reset on any comma" would + // misclassify it as table-level (the §4.2 probe). + let (cols, checks) = + parse_sct_checks("create table t (n numeric(10, 2) check (n > 0))"); + assert_eq!(col(&cols, "n").check_sql.as_deref(), Some("n > 0")); + assert!(checks.is_empty(), "no table-level CHECK was produced"); + } + + #[test] + fn table_check_after_table_primary_key() { + // A table-PK `(a, b)` injects its own parens/comma into the + // item stream; the following table CHECK must still be detected. + let (_, checks) = + parse_sct_checks("create table t (a int, b int, primary key (a, b), check (a < b))"); + assert_eq!(checks, vec!["a < b".to_string()]); + } + + #[test] + fn table_check_after_table_unique() { + let (_, checks) = + parse_sct_checks("create table t (a int, b int, unique (a, b), check (a <> b))"); + assert_eq!(checks, vec!["a <> b".to_string()]); + } + + #[test] + fn table_check_captures_balanced_nested_parens() { + let (_, checks) = + parse_sct_checks("create table t (a int, b int, check ((a + b) > (a - b)))"); + assert_eq!(checks, vec!["(a + b) > (a - b)".to_string()]); + } + + #[test] + fn table_check_before_a_later_column_is_table_level() { + // A CHECK element that appears between columns (not after a + // column's type) is table-level even though more columns follow. + let (cols, checks) = + parse_sct_checks("create table t (a int, check (a > 0), b int)"); + assert_eq!(checks, vec!["a > 0".to_string()]); + assert!(col(&cols, "a").check_sql.is_none() && col(&cols, "b").check_sql.is_none()); + } } diff --git a/src/persistence/mod.rs b/src/persistence/mod.rs index 51ee37e..a62f119 100644 --- a/src/persistence/mod.rs +++ b/src/persistence/mod.rs @@ -146,6 +146,13 @@ pub struct TableSchema { /// written before composite UNIQUE existed — the YAML field is /// optional on read. pub unique_constraints: Vec>, + /// Table-level `CHECK ()` constraints, in declaration + /// order, as raw SQL text (ADR-0035 §4a.3). The engine reports + /// no CHECK constraints, so these are the source of truth (held + /// in `__rdbms_playground_table_checks`) and echoed verbatim + /// into the rebuilt DDL. Empty for project files written before + /// table-level CHECK existed — the YAML field is optional on read. + pub check_constraints: Vec, } #[derive(Debug, Clone, PartialEq, Eq)] diff --git a/src/persistence/yaml.rs b/src/persistence/yaml.rs index 845613b..3a87e43 100644 --- a/src/persistence/yaml.rs +++ b/src/persistence/yaml.rs @@ -105,6 +105,15 @@ fn write_table(out: &mut String, table: &TableSchema) { let _ = writeln!(out, "]"); } } + // Table-level CHECK constraints as raw SQL text (ADR-0035 §4a.3) — + // double-quoted (an expression like `a < b` is not a bare scalar) + // and emitted only when present. + if !table.check_constraints.is_empty() { + let _ = writeln!(out, " check_constraints:"); + for expr in &table.check_constraints { + let _ = writeln!(out, " - {}", yaml_string(expr)); + } + } } /// Always render `s` as a double-quoted YAML string — used @@ -265,6 +274,7 @@ pub(crate) fn parse_schema(body: &str) -> Result { primary_key: t.primary_key, columns, unique_constraints: t.unique_constraints, + check_constraints: t.check_constraints, }); } let mut relationships: Vec = Vec::with_capacity(raw.relationships.len()); @@ -377,6 +387,10 @@ struct RawTable { /// Optional on read — older project files omit it. #[serde(default)] unique_constraints: Vec>, + /// Table-level CHECK constraints as raw SQL text (ADR-0035 §4a.3). + /// Optional on read — older project files omit it. + #[serde(default)] + check_constraints: Vec, } #[derive(Deserialize)] @@ -439,6 +453,7 @@ mod tests { ColumnSchema { name: "Name".to_string(), user_type: Type::Text, unique: false, not_null: false, default: None, check: None }, ], unique_constraints: Vec::new(), + check_constraints: Vec::new(), }, TableSchema { name: "Orders".to_string(), @@ -448,6 +463,7 @@ mod tests { ColumnSchema { name: "CustId".to_string(), user_type: Type::Int, unique: false, not_null: false, default: None, check: None }, ], unique_constraints: Vec::new(), + check_constraints: Vec::new(), }, ], relationships: vec![RelationshipSchema { @@ -517,6 +533,7 @@ mod tests { check: None, }], unique_constraints: Vec::new(), + check_constraints: Vec::new(), }], relationships: vec![], indexes: vec![], @@ -575,6 +592,7 @@ mod tests { }, ], unique_constraints: Vec::new(), + check_constraints: Vec::new(), }], relationships: vec![], indexes: vec![], @@ -584,6 +602,54 @@ mod tests { assert_eq!(parsed, snap, "constraints survive the yaml round-trip"); } + #[test] + fn table_level_constraints_round_trip_through_yaml() { + // Composite UNIQUE and table-level CHECK (raw SQL text) survive + // a serialize → parse cycle in declaration order (ADR-0035 + // §4a.2 / §4a.3). + let snap = SchemaSnapshot { + created_at: "2026-05-25T00:00:00Z".to_string(), + tables: vec![TableSchema { + name: "T".to_string(), + primary_key: vec![], + columns: vec![ + ColumnSchema { name: "a".to_string(), user_type: Type::Int, unique: false, not_null: false, default: None, check: None }, + ColumnSchema { name: "b".to_string(), user_type: Type::Int, unique: false, not_null: false, default: None, check: None }, + ColumnSchema { name: "c".to_string(), user_type: Type::Int, unique: false, not_null: false, default: None, check: None }, + ], + unique_constraints: vec![vec!["a".to_string(), "b".to_string()]], + check_constraints: vec!["a < b".to_string(), "b < c".to_string()], + }], + relationships: vec![], + indexes: vec![], + }; + let body = serialize_schema(&snap); + let parsed = parse_schema(&body).expect("parse schema"); + assert_eq!( + parsed, snap, + "table-level UNIQUE + CHECK survive the yaml round-trip in order" + ); + } + + #[test] + fn check_constraints_optional_on_read() { + // A project file written before table-level CHECK existed (no + // `check_constraints:` key) parses with an empty list. + let body = "\ +version: 1 +project: + created_at: 2026-05-25T00:00:00Z +tables: + - name: T + primary_key: [id] + columns: + - { name: id, type: int } +relationships: [] +"; + let parsed = parse_schema(body).expect("parse"); + assert!(parsed.tables[0].check_constraints.is_empty()); + } + #[test] fn parses_minimal_yaml_with_no_tables() { let body = "\ @@ -662,6 +728,7 @@ relationships: ColumnSchema { name: "b".to_string(), user_type: Type::Int, unique: false, not_null: false, default: None, check: None }, ], unique_constraints: Vec::new(), + check_constraints: Vec::new(), }], relationships: vec![], indexes: vec![], diff --git a/src/runtime.rs b/src/runtime.rs index 909c140..57e3410 100644 --- a/src/runtime.rs +++ b/src/runtime.rs @@ -1925,9 +1925,18 @@ async fn execute_command_typed( columns, primary_key, unique_constraints, + check_constraints, if_not_exists, } => database - .sql_create_table(name, columns, primary_key, unique_constraints, if_not_exists, src) + .sql_create_table( + name, + columns, + primary_key, + unique_constraints, + check_constraints, + if_not_exists, + src, + ) .await .map(|outcome| match outcome { CreateOutcome::Created(d) => CommandOutcome::Schema(Some(d)), diff --git a/tests/sql_create_table.rs b/tests/sql_create_table.rs index e0fca04..e9652d5 100644 --- a/tests/sql_create_table.rs +++ b/tests/sql_create_table.rs @@ -52,6 +52,7 @@ fn created_table_appears_with_playground_types() { ], vec!["id".to_string()], vec![], // no composite UNIQUE + vec![], // no table CHECK false, Some("create table Widget (id int primary key, name text)".to_string()), )) @@ -89,6 +90,7 @@ fn integer_primary_key_is_plain_int() { vec![ColumnSpec::new("id", Type::Int)], vec!["id".to_string()], vec![], // no composite UNIQUE + vec![], // no table CHECK false, Some("create table T (id integer primary key)".to_string()), )) @@ -114,6 +116,7 @@ fn serial_pk_autoincrements_in_multi_column_table() { ], vec!["id".to_string()], vec![], // no composite UNIQUE + vec![], // no table CHECK false, Some("create table T (id serial primary key, name text)".to_string()), )) @@ -157,6 +160,7 @@ fn if_not_exists_is_a_noop_when_table_exists() { specs(), vec!["id".to_string()], vec![], // no composite UNIQUE + vec![], // no table CHECK false, Some("create table T (id int)".to_string()), )) @@ -168,6 +172,7 @@ fn if_not_exists_is_a_noop_when_table_exists() { specs(), vec!["id".to_string()], vec![], // no composite UNIQUE + vec![], // no table CHECK true, // IF NOT EXISTS Some("create table if not exists T (id int)".to_string()), )) @@ -194,6 +199,7 @@ fn table_without_primary_key_is_allowed() { vec![ColumnSpec::new("body", Type::Text)], vec![], // no primary key vec![], // no composite UNIQUE + vec![], // no table CHECK false, Some("create table Notes (body text)".to_string()), )) @@ -236,6 +242,7 @@ fn check_constraint_is_enforced() { vec![ColumnSpec::new("id", Type::Serial), col_check("price", Type::Real, "price >= 0")], vec!["id".to_string()], vec![], + vec![], // no table CHECK false, Some("create table T (id serial primary key, price real check (price >= 0))".to_string()), )) @@ -270,6 +277,7 @@ fn default_is_applied_when_column_omitted() { ], vec!["id".to_string()], vec![], + vec![], // no table CHECK false, Some("create table T (id serial primary key, label text, n int default 7)".to_string()), )) @@ -298,6 +306,7 @@ fn composite_unique_is_enforced() { vec![ColumnSpec::new("a", Type::Int), ColumnSpec::new("b", Type::Int)], vec![], vec![vec!["a".to_string(), "b".to_string()]], + vec![], // no table CHECK false, Some("create table T (a int, b int, unique (a, b))".to_string()), )) @@ -332,6 +341,7 @@ fn check_default_and_composite_unique_survive_rebuild() { ], vec![], vec![vec!["a".to_string(), "b".to_string()]], + vec![], // no table CHECK false, Some( "create table T (a int, b int, price real check (price >= 0), \ @@ -369,6 +379,197 @@ fn check_default_and_composite_unique_survive_rebuild() { assert!(r.block_on(ins("1", "1", "5")).is_err(), "composite UNIQUE survived rebuild"); } +#[test] +fn table_level_check_is_enforced() { + // ADR-0035 §4a.3: a multi-column CHECK has no column to hang on and + // the engine reports no CHECKs, so it round-trips via a metadata + // table. Here we prove the engine actually enforces it. + let (_p, db, _d) = open(false); + let r = rt(); + r.block_on(db.sql_create_table( + "T".to_string(), + vec![ColumnSpec::new("a", Type::Int), ColumnSpec::new("b", Type::Int)], + vec![], + vec![], // no composite UNIQUE + vec!["a < b".to_string()], // table-level CHECK + false, + Some("create table T (a int, b int, check (a < b))".to_string()), + )) + .expect("create"); + let ins = |a: &str, b: &str| { + db.insert( + "T".to_string(), + None, + vec![Value::Number(a.to_string()), Value::Number(b.to_string())], + Some("insert".to_string()), + ) + }; + r.block_on(ins("1", "2")).expect("(1,2) satisfies a < b"); + assert!(r.block_on(ins("2", "1")).is_err(), "CHECK (a < b) rejects (2,1)"); + assert!(r.block_on(ins("3", "3")).is_err(), "CHECK (a < b) rejects (3,3)"); +} + +#[test] +fn multiple_table_level_checks_all_enforced() { + let (_p, db, _d) = open(false); + let r = rt(); + r.block_on(db.sql_create_table( + "T".to_string(), + vec![ + ColumnSpec::new("a", Type::Int), + ColumnSpec::new("b", Type::Int), + ColumnSpec::new("c", Type::Int), + ], + vec![], + vec![], // no composite UNIQUE + vec!["a < b".to_string(), "b < c".to_string()], + false, + Some("create table T (a int, b int, c int, check (a < b), check (b < c))".to_string()), + )) + .expect("create"); + let ins = |a: &str, b: &str, c: &str| { + db.insert( + "T".to_string(), + None, + vec![ + Value::Number(a.to_string()), + Value::Number(b.to_string()), + Value::Number(c.to_string()), + ], + Some("insert".to_string()), + ) + }; + r.block_on(ins("1", "2", "3")).expect("(1,2,3) satisfies both checks"); + assert!(r.block_on(ins("2", "1", "3")).is_err(), "first CHECK (a < b) enforced"); + assert!(r.block_on(ins("1", "3", "2")).is_err(), "second CHECK (b < c) enforced"); +} + +#[test] +fn dropping_a_table_clears_its_table_check_metadata() { + // The CHECK metadata table is keyed by (table_name, seq). If a drop + // left orphan rows behind, re-creating the same table with a CHECK + // would collide on that primary key and fail. A clean create→drop→ + // create round-trip proves the drop path clears the metadata. + let (_p, db, _d) = open(false); + let r = rt(); + let make = || { + db.sql_create_table( + "T".to_string(), + vec![ColumnSpec::new("a", Type::Int), ColumnSpec::new("b", Type::Int)], + vec![], + vec![], // no composite UNIQUE + vec!["a < b".to_string()], + false, + Some("create table T (a int, b int, check (a < b))".to_string()), + ) + }; + r.block_on(make()).expect("first create"); + r.block_on(db.drop_table("T".to_string(), Some("drop table T".to_string()))) + .expect("drop"); + r.block_on(make()).expect("re-create must not collide on orphaned CHECK metadata"); + // The re-created CHECK is enforced (and there is exactly one of it). + let ins = |a: &str, b: &str| { + db.insert( + "T".to_string(), + None, + vec![Value::Number(a.to_string()), Value::Number(b.to_string())], + Some("insert".to_string()), + ) + }; + r.block_on(ins("1", "2")).expect("(1,2) valid"); + assert!(r.block_on(ins("2", "1")).is_err(), "CHECK enforced after re-create"); +} + +#[test] +fn table_level_check_survives_a_rebuild_triggering_column_add() { + // Cross-cutting probe (ADR-0013 rebuild primitive × 4a.3 metadata): + // adding a constrained column to a table that carries a table-level + // CHECK rebuilds the table via `schema_to_ddl`. The CHECK must + // survive both in the engine (enforced) AND in the metadata table + // (so a *later* rebuild_from_text still re-emits it) — otherwise the + // constraint is silently lost the next time the table is rebuilt. + let (p, db, _d) = open(false); + let r = rt(); + r.block_on(db.sql_create_table( + "T".to_string(), + vec![ColumnSpec::new("a", Type::Int), ColumnSpec::new("b", Type::Int)], + vec![], + vec![], // no composite UNIQUE + vec!["a < b".to_string()], + false, + Some("create table T (a int, b int, check (a < b))".to_string()), + )) + .expect("create"); + + // A UNIQUE column forces the rebuild path (ADR-0029 §6). + let mut c = ColumnSpec::new("c", Type::Int); + c.unique = true; + r.block_on(db.add_column("T".to_string(), c, Some("add column T: c(int) unique".to_string()))) + .expect("add column via rebuild"); + + let ins = |a: &str, b: &str, c: &str| { + db.insert( + "T".to_string(), + Some(vec!["a".to_string(), "b".to_string(), "c".to_string()]), + vec![ + Value::Number(a.to_string()), + Value::Number(b.to_string()), + Value::Number(c.to_string()), + ], + Some("insert".to_string()), + ) + }; + // Engine still enforces the CHECK right after the rebuild. + r.block_on(ins("1", "2", "10")).expect("(1,2) valid after column add"); + assert!(r.block_on(ins("2", "1", "20")).is_err(), "CHECK survived the column-add rebuild"); + + // And the metadata survived too: a fresh rebuild from project.yaml + // re-emits the CHECK (it would be lost if the rebuild primitive had + // dropped the table_checks rows without repopulating them). + r.block_on(db.rebuild_from_text(p.path().to_path_buf(), None)) + .expect("rebuild"); + assert!( + r.block_on(ins("9", "8", "30")).is_err(), + "CHECK still present after a later rebuild_from_text — metadata was preserved" + ); +} + +#[test] +fn table_level_check_survives_rebuild() { + // The part-D proof for 4a.3: the engine reports no CHECK, so the + // constraint can only be reconstructed from the metadata table via + // project.yaml. After a rebuild it must still be enforced. + let (p, db, _d) = open(false); + let r = rt(); + r.block_on(db.sql_create_table( + "T".to_string(), + vec![ColumnSpec::new("a", Type::Int), ColumnSpec::new("b", Type::Int)], + vec![], + vec![], // no composite UNIQUE + vec!["a < b".to_string()], + false, + Some("create table T (a int, b int, check (a < b))".to_string()), + )) + .expect("create"); + + r.block_on(db.rebuild_from_text(p.path().to_path_buf(), None)) + .expect("rebuild"); + + let ins = |a: &str, b: &str| { + db.insert( + "T".to_string(), + None, + vec![Value::Number(a.to_string()), Value::Number(b.to_string())], + Some("insert".to_string()), + ) + }; + r.block_on(ins("1", "2")).expect("(1,2) still valid after rebuild"); + assert!( + r.block_on(ins("5", "4")).is_err(), + "table-level CHECK survived rebuild via the metadata table" + ); +} + #[test] fn if_not_exists_noop_is_journalled() { // A successful no-op is still a submission and belongs in the @@ -381,6 +582,7 @@ fn if_not_exists_noop_is_journalled() { vec![ColumnSpec::new("id", Type::Int)], vec!["id".to_string()], vec![], // no composite UNIQUE + vec![], // no table CHECK false, Some("create table T (id int)".to_string()), )) @@ -392,6 +594,7 @@ fn if_not_exists_noop_is_journalled() { vec![ColumnSpec::new("id", Type::Int)], vec!["id".to_string()], vec![], // no composite UNIQUE + vec![], // no table CHECK true, Some(noop.to_string()), )) @@ -411,6 +614,7 @@ fn plain_create_errors_when_table_exists() { specs(), vec!["id".to_string()], vec![], // no composite UNIQUE + vec![], // no table CHECK false, Some("create table T (id int)".to_string()), )) @@ -421,6 +625,7 @@ fn plain_create_errors_when_table_exists() { specs(), vec!["id".to_string()], vec![], // no composite UNIQUE + vec![], // no table CHECK false, // no IF NOT EXISTS Some("create table T (id int)".to_string()), )); @@ -436,6 +641,7 @@ fn sql_create_table_is_one_undo_step() { vec![ColumnSpec::new("id", Type::Int)], vec!["id".to_string()], vec![], // no composite UNIQUE + vec![], // no table CHECK false, Some("create table T (id int)".to_string()), )) @@ -487,6 +693,7 @@ fn serial_pk_first_column_autoincrements_after_rebuild() { ], vec!["id".to_string()], vec![], // no composite UNIQUE + vec![], // no table CHECK false, Some("create table T (id serial primary key, name text)".to_string()), )) @@ -519,6 +726,7 @@ fn serial_pk_non_first_column_autoincrements_after_rebuild() { ], vec!["id".to_string()], vec![], // no composite UNIQUE + vec![], // no table CHECK false, Some("create table T (name text, id serial primary key)".to_string()), )) @@ -535,3 +743,57 @@ fn serial_pk_non_first_column_autoincrements_after_rebuild() { "serial keeps autoincrement after a rebuild even as a non-first column" ); } + +#[test] +fn dropping_a_column_a_table_check_references_fails_cleanly() { + // Cross-cutting safety probe: a simple-mode `drop column` of a column + // that a table-level CHECK references rebuilds the table via + // `schema_to_ddl`, which re-emits `CHECK (a < b)` for a temp table + // that no longer has `a` — the engine rejects it. This must fail + // *cleanly* (the rebuild transaction rolls back), leaving the table + // fully intact, never half-migrated. Up-front detection (parsing the + // referenced columns out of the raw CHECK text so the refusal is + // deliberate) is 4e work; the friendly wording itself is H1. Today's + // clean engine-level rejection is the safe interim (user-confirmed). + let (_p, db, _d) = open(false); + let r = rt(); + r.block_on(db.sql_create_table( + "T".to_string(), + vec![ColumnSpec::new("a", Type::Int), ColumnSpec::new("b", Type::Int)], + vec![], + vec![], // no composite UNIQUE + vec!["a < b".to_string()], + false, + Some("create table T (a int, b int, check (a < b))".to_string()), + )) + .expect("create"); + + let dropped = r.block_on(db.drop_column( + "T".to_string(), + "a".to_string(), + false, + Some("drop column T: a".to_string()), + )); + assert!(dropped.is_err(), "dropping a column a CHECK references is rejected"); + + // The table is intact: both columns survive (rollback) ... + let desc = r + .block_on(db.describe_table("T".to_string(), None)) + .expect("describe still works"); + assert_eq!( + desc.columns.iter().map(|c| c.name.clone()).collect::>(), + vec!["a".to_string(), "b".to_string()], + "the failed drop rolled back — no half-migrated table" + ); + // ... and the CHECK is still enforced. + let ins = |a: &str, b: &str| { + db.insert( + "T".to_string(), + Some(vec!["a".to_string(), "b".to_string()]), + vec![Value::Number(a.to_string()), Value::Number(b.to_string())], + Some("insert".to_string()), + ) + }; + r.block_on(ins("1", "2")).expect("(1,2) valid — table survived intact"); + assert!(r.block_on(ins("2", "1")).is_err(), "CHECK still enforced after the failed drop"); +}