From 76d60591bfed82a0f68c090381085aad2009f979 Mon Sep 17 00:00:00 2001 From: "claude@clouddev1" Date: Mon, 25 May 2026 15:35:48 +0000 Subject: [PATCH] =?UTF-8?q?feat:=20ADR-0035=204b=20=E2=80=94=20foreign=20k?= =?UTF-8?q?eys=20in=20CREATE=20TABLE?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add foreign keys to advanced-mode SQL CREATE TABLE — the SQL spelling of an ADR-0013 named relationship, created in the same transaction as the table (one undo step). - Grammar: inline ` … REFERENCES [()] [ON DELETE/UPDATE …]` (a new column constraint) and table-level `[CONSTRAINT ] FOREIGN KEY () REFERENCES …` (two new element branches — both start on a concrete keyword, never a leading Optional, which would abort the element Choice). Referential clauses reuse shared::REFERENTIAL_CLAUSES. - Builder: greedy FK-clause consumption (parens consumed internally so they don't perturb the 4a.3 element-boundary depth tracker); inline FK auto-named, table FK takes an optional CONSTRAINT name. - Worker: do_create_table resolves + validates each FK before building the DDL (self-ref validates against the in-statement columns/PK; bare REFERENCES resolves to the parent's single-column PK, composite -> error; PK-target + Type::fk_target_type compatibility), emits the FOREIGN KEY clause identically to schema_to_ddl, and writes the relationship metadata in the create transaction. - Reuse: name/uniqueness/metadata-insert/type-compat factored into shared helpers; do_add_relationship refactored to use them. - FKs round-trip via the existing relationship plumbing (no new persistence structures); describe surfaces the relationship. Self-references and bare `REFERENCES ` supported (user-confirmed). Self-ref pre-submit indicator wrinkle deferred to 4i (tracked in ADR §13, a code comment, and the plan). DA/runda round added cross-cutting probes (FK survives the add-column rebuild + a later rebuild_from_text; referential actions survive rebuild; drop-child clears the relationship; drop-parent refused; bare self-ref resolves to own PK) — all green, no fixes needed. 27 new tests (grammar/builder + Tier-3). Docs: ADR-0035 Status/§13, README, requirements.md Q1. Tests: 1795 passing, 0 failing, 1 ignored. Clippy clean. --- docs/adr/0035-advanced-mode-sql-ddl.md | 42 +- docs/adr/README.md | 2 +- docs/plans/20260525-adr-0035-sql-ddl-4b.md | 266 ++++++++++ docs/requirements.md | 9 +- src/db.rs | 332 ++++++++++-- src/dsl/command.rs | 32 ++ src/dsl/grammar/ddl.rs | 127 ++++- src/dsl/grammar/sql_create_table.rs | 289 ++++++++++- src/dsl/mod.rs | 2 +- src/runtime.rs | 2 + tests/sql_create_table.rs | 566 ++++++++++++++++++++- 11 files changed, 1588 insertions(+), 81 deletions(-) create mode 100644 docs/plans/20260525-adr-0035-sql-ddl-4b.md diff --git a/docs/adr/0035-advanced-mode-sql-ddl.md b/docs/adr/0035-advanced-mode-sql-ddl.md index 91e705a..560dcdb 100644 --- a/docs/adr/0035-advanced-mode-sql-ddl.md +++ b/docs/adr/0035-advanced-mode-sql-ddl.md @@ -3,12 +3,13 @@ ## Status Accepted. Design agreed with the user (2026-05-24); the approach is -**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 +**validated end-to-end by sub-phases 4a / 4a.2 / 4a.3 / 4b** +(`CREATE TABLE` with column- and table-level constraints and foreign +keys, implemented 2026-05-25 — plans +`docs/plans/20260524-adr-0035-sql-ddl-4a.md`, `…-4a2.md`, `…-4a3.md`, +`docs/plans/20260525-adr-0035-sql-ddl-4b.md`), so the decision is +accepted while the remaining sub-phases (**4c–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. @@ -351,8 +352,21 @@ ADR-0033's structure: 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. +- **4b — Foreign keys in `CREATE TABLE`.** *(Implemented 2026-05-25 — + plan `docs/plans/20260525-adr-0035-sql-ddl-4b.md`.)* Inline + `REFERENCES [()] [ON DELETE/UPDATE …]` + table-level + `[CONSTRAINT ] FOREIGN KEY () REFERENCES …` → ADR-0013 + relationship metadata, written in the create transaction (one undo + step). Reuses the relationship name/uniqueness/metadata helpers + shared with `add relationship`; `do_create_table` emits the + `FOREIGN KEY` clause identically to `schema_to_ddl`. **Self-references** + (parent = the table being created, validated against the in-statement + columns/PK) and the **bare `REFERENCES `** form (resolves to + the parent's single-column PK; composite → error) are both supported + (user-confirmed). Inline FKs are auto-named; only the table-level form + takes `CONSTRAINT `. PK-target only (UNIQUE-target deferred with + `add relationship`); `Type::fk_target_type` (ADR-0011) governs type + compatibility. - **4c — `DROP TABLE [IF EXISTS]`** → `SqlDropTable` (cascade parity; `IF EXISTS` no-op-with-note, §4). - **4d — `CREATE [UNIQUE] INDEX` / `DROP INDEX`** → `SqlCreateIndex` @@ -370,7 +384,17 @@ ADR-0033's structure: - **4h — `ALTER TABLE … RENAME TO`** (the §6 new low-level op). - **4i — Verification sweep.** Typing-surface + matrix coverage, engine-neutral error pass, undo-parity check (one step per - statement), `help`/usage for the new forms. + statement), `help`/usage for the new forms. **Carried in from earlier + slices:** (a) refresh the `CREATE TABLE` help/usage skeleton for the + 4a.2 `DEFAULT`/`CHECK`/composite-`UNIQUE`, 4a.3 table-`CHECK`, and 4b + FK forms (deferred from each); (b) `describe` display of table-level + constraints (composite `UNIQUE` + table `CHECK`); (c) **4b self-ref + FK indicator** — a `CREATE TABLE` with a self-referencing FK + (`references `) parses + executes correctly, but the pre-submit + schema-existence diagnostic falsely flags the not-yet-created self + table as unknown (the FK parent slot is `IdentSource::Tables`). Make + the diagnostic treat a FK parent equal to the `CREATE TABLE` target as + valid, so the indicator stops lying for self-references. ## Consequences diff --git a/docs/adr/README.md b/docs/adr/README.md index b5b9cbe..6644696 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-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 +- [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/4b `CREATE TABLE` (incl. foreign keys), implemented 2026-05-25; 4c–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) and **4b** (foreign keys — inline `REFERENCES` + table-level `FOREIGN KEY` → ADR-0013 named relationships in the create transaction, one undo step; self-references + bare `REFERENCES ` supported, user-confirmed); the remaining DDL 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-4b.md b/docs/plans/20260525-adr-0035-sql-ddl-4b.md new file mode 100644 index 0000000..ecf8861 --- /dev/null +++ b/docs/plans/20260525-adr-0035-sql-ddl-4b.md @@ -0,0 +1,266 @@ +# Plan: ADR-0035 Phase 4, sub-phase 4b — foreign keys in `CREATE TABLE` + +Add foreign keys to advanced-mode SQL `CREATE TABLE`: inline +` REFERENCES [()] [ON DELETE …] [ON UPDATE …]` +and table-level `[CONSTRAINT ] FOREIGN KEY () REFERENCES +[()] [ON DELETE …] [ON UPDATE …]`. Each FK is the SQL +spelling of an ADR-0013 **named relationship**: one `CREATE TABLE` +creates the table *and* its relationship metadata in one transaction = +**one undo step** (ADR-0035 §5). Builds on 4a/4a.2/4a.3. + +## 1. Baseline + +- Tests: **1769 passing, 0 failing, 1 ignored**; clippy clean. Branch + `main`, last commit `60111f6` (4a.3). 4b starts here. + +## 2. Decisions locked with the user (do not re-litigate) + +1. **Self-referencing FK supported** (2026-05-25): `CREATE TABLE emp + (id int primary key, mgr int REFERENCES emp(id))`. When the FK's + parent table equals the table being created, the referenced column is + validated against the columns/PK **being defined** (not `read_schema`, + which would fail — the table doesn't exist yet). Metadata row carries + `parent_table = child_table`. +2. **Bare `REFERENCES ` supported** (2026-05-25, standard SQL + + the §7 "trust like SQL" posture): resolves to the parent's + **single-column PK**; a parent with a composite PK is a clear error + ("name the referenced column"). The explicit `REFERENCES parent(col)` + form is the primary one (ADR §4). +3. **Inline `REFERENCES` is always auto-named; only the table-level + `FOREIGN KEY` form takes `CONSTRAINT `** (ADR §4/§5). Auto-name: + `__to__` (ADR-0013). +4. **PK-target only** (ADR-0013/0011): the referenced column must be a + primary key; UNIQUE-target FKs stay deferred (as for `add + relationship`). Type compatibility via `Type::fk_target_type` + (ADR-0011). +5. **One statement = one command = one transaction = one undo step** + (ADR-0035 §5/§10), including a multi-FK `CREATE TABLE`. + +## 3. Phase 1 — Requirements checklist (4b) + +### Functional + +- [ ] Inline column FK `REFERENCES [()] [ON DELETE ] + [ON UPDATE ]` parses (advanced mode); child column = the column it + is attached to. +- [ ] Table-level `[CONSTRAINT ] FOREIGN KEY () REFERENCES + [()] [ON DELETE ] [ON UPDATE ]` parses; `` + is the child column. +- [ ] `ON DELETE` / `ON UPDATE` each optional, either order, default + `no action`; actions `cascade` / `restrict` / `set null` / `no action` + (reuse `shared::REFERENTIAL_CLAUSES`). +- [ ] Multiple FKs in one statement → multiple relationships, all in one + transaction / one undo step. +- [ ] Self-referencing FK (parent = table being created) validated + against the columns/PK being defined. +- [ ] Bare `REFERENCES ` resolves to the parent's single-column + PK; composite-PK parent → friendly "name the column" error. +- [ ] Validation (reused with `do_add_relationship`): parent column is a + PK; child column type == `parent_type.fk_target_type()`; relationship + name unique. Engine-neutral errors. +- [ ] Structural execution: `do_create_table` emits the `FOREIGN KEY` + clause(s) **identically to `schema_to_ddl`** (the §6.1 rule) and writes + `__rdbms_playground_relationships` rows in its transaction. +- [ ] FK still-rejected test from 4a.3 (`foreign_key_still_rejected`) + flipped to accepted. + +### Cross-cutting / round-trip + +- [ ] A table created with FK(s) round-trips: relationships appear in + `project.yaml`, survive `rebuild_from_text`, and the FK is enforced + (a violating insert fails) before and after rebuild. +- [ ] `describe` shows the relationship (outbound on the child, inbound + on the parent) — FKs are first-class (unlike table CHECK, FK metadata + *is* surfaced by `do_describe_table`). +- [ ] One undo step removes the table **and** its relationship rows; + redo recreates both. +- [ ] `history.log` / replay: the literal SQL line replays as a write. + +### Testing (ADR-0008 four tiers) + +- [ ] **Tier 1** (builder): inline + table-level FK captured (child/ + parent/columns/actions); `CONSTRAINT `; bare `REFERENCES` + (parent_column = None); multiple FKs; ON DELETE/UPDATE in both orders; + self-ref shape; FK alongside CHECK/PK/UNIQUE (depth/element coexistence). +- [ ] **Tier 3** (`tests/sql_create_table.rs`): FK enforced (violating + insert fails); cascade behaviour; auto-name + explicit name in + `REL_TABLE`; type-mismatch + non-PK-target + composite-bare-ref errors; + self-ref enforced; **survives rebuild**; `describe` shows the relation. +- [ ] **Undo** Tier-3: one step; undo drops table + relationship rows. + +## 4. Architecture & design + +### 4.1 Grammar (`src/dsl/grammar/sql_create_table.rs`) + +- **Action clauses**: reuse `shared::REFERENTIAL_CLAUSES` (the + `on ` repeat, 0..2) — same keywords as SQL. +- **Inline FK** — new `COL_CONSTRAINT_CHOICES` branch + `REFERENCES [ '(' ')' ] REFERENTIAL_CLAUSES`. + Idents: `fk_parent_table`, `fk_parent_column` roles + (`IdentSource::Schema` for the parent so it completes from the cache; + parent-column also schema-ish — confirm the right source; the table + being created is `NewName`). +- **Table-level FK** — new `ELEMENT_CHOICES` branch + `[CONSTRAINT ] FOREIGN KEY '(' ')' REFERENCES + [ '(' ')' ] REFERENTIAL_CLAUSES`. Idents: + `fk_name` (NewName), `fk_child_column`, `fk_parent_table`, + `fk_parent_column`. +- The bare `REFERENCES parent` form: make the `( pcol )` group + `Optional`. + +### 4.2 Builder (`build_sql_create_table`, `ddl.rs`) + +Greedy-consume each FK clause when its keyword is hit (parens consumed +**inside** the handler, so they don't perturb the §4a.3 `depth` tracker): + +- On `Word("references")` (inline; `column_open == true`): child column + = `columns.last()`; read the following `fk_parent_table` ident, + optional `( fk_parent_column )`, then drain referential clauses + (`while peek == on { … }`, mapping `delete`/`update` → actions via the + existing `parse_action` logic). Push an `FkSpec { name: None, … }`. +- On `Word("constraint")`: stash `pending_fk_name = next fk_name ident`. +- On `Word("foreign")` (table-level; `column_open == false`): consume + `key ( fk_child_column ) references fk_parent_table [ ( fk_parent_column ) ]` + + referential clauses; push `FkSpec { name: pending_fk_name.take(), … }`. + +A small `FkSpec { name: Option, child_column: String, +parent_table: String, parent_column: Option, on_delete, +on_update }` accumulates into `Command::SqlCreateTable.foreign_keys`. + +### 4.3 Command AST (`src/dsl/command.rs`) + +`SqlCreateTable` gains `foreign_keys: Vec` where +`SqlForeignKey { name: Option, child_column: String, +parent_table: String, parent_column: Option, on_delete: +ReferentialAction, on_update: ReferentialAction }`. (`parent_column` +optional = the bare-ref form, resolved at execution.) + +### 4.4 Worker — `do_create_table` (the only executor) + +Threaded a `foreign_keys: &[SqlForeignKey]` param (and through +`Request`/method/dispatch). Inside, **before** building the DDL: + +For each FK, resolve + validate (reusing helpers, §4.5): +1. **Resolve `parent_column`** (bare form): self-ref → the statement's + own single-column `primary_key` (composite → error); else → + `read_schema(parent).primary_key` single column (composite → error). +2. **Parent is a PK**: self-ref → `parent_column ∈ primary_key` being + defined; else → `read_schema(parent)` column with `primary_key`. + Obtain `parent_user_type`. +3. **Child type compat**: child column must be among the `columns` being + defined; its type must equal `parent_user_type.fk_target_type()` + (shared helper; friendly mismatch error). +4. **Name**: `resolve_relationship_name(name, parent, pcol, child, ccol)` + then uniqueness check (shared helpers). + +Then emit `FOREIGN KEY () REFERENCES () ON +DELETE ON UPDATE ` **after the CHECK clauses**, exactly as +`schema_to_ddl` does (always explicit pcol + both actions, so create-DDL +== rebuild-DDL — no drift). Write each `REL_TABLE` row in the existing +transaction (before `finalize_persistence`, so the snapshot/yaml sees it). + +`do_describe_table(name)` already surfaces the new relationships. + +### 4.5 Shared helpers (refactor `do_add_relationship`, don't duplicate) + +Factor out of `do_add_relationship`, used by both it and +`do_create_table`: + +- `fk_type_mismatch_error(...)` / a `check_fk_type_compat(parent_type, + child_type, …) -> Result<()>`. +- `resolve_relationship_name(name, parent, pcol, child, ccol) -> String`. +- `ensure_relationship_name_unique(conn, name) -> Result<()>`. +- `insert_relationship_metadata(tx, name, parent, pcol, child, ccol, + on_delete, on_update) -> Result<()>`. + +`do_add_relationship` keeps its `read_schema`-based validation + rebuild; +`do_create_table` validates against the in-statement specs. Only the +naming/uniqueness/metadata-write + type-compat message are shared. + +### 4.6 No new persistence/round-trip structures + +FKs already round-trip: `read_schema` reads them via +`pragma_foreign_key_list`; `RelationshipSchema` → `project.yaml`; +`build_read_schema` re-emits them inline on rebuild via `schema_to_ddl`. +4b adds **no** new `TableSchema`/YAML field — it reuses the existing +relationship plumbing entirely. (The litmus test: an FK is a structure +the model already persists.) + +### 4.7 Friendly catalog / keys + +New diagnostics may be needed (non-PK target, type mismatch, +composite-bare-ref, unknown parent table) — but most reuse +`do_add_relationship`'s existing engine-neutral errors. Any new +`help_id`/diagnostic key gets a `keys.rs` entry **and** an `en-US.yaml` +body (lockstep test) with engine-neutral wording. Help/usage skeleton +refresh stays batched into **4i** (consistent with 4a.2/4a.3). + +## 5. Out of 4b scope + +- `DROP TABLE` (4c), indexes (4d), `ALTER TABLE` (4e–4h). +- UNIQUE-target FKs (deferred with `add relationship`). +- `MATCH` / `DEFERRABLE` and other engine-specific FK extras (ADR §12 — + parse error). + +## 6. Open items / implementer calls + +1. **`IdentSource` for the parent table/column slots** — RESOLVED: + `IdentSource::Tables`/`Columns` (matching the `add relationship` + endpoints; completion + existence hint). **Known wrinkle deferred to + 4i (user-confirmed 2026-05-25):** a self-referencing FK + (`references `) parses + executes correctly, but the pre-submit + schema-existence indicator falsely flags the not-yet-created self + table as unknown. Tracked in **three places** so it is not lost: + ADR-0035 §13 4i bullet, a `NOTE (4i)` code comment at `FK_PARENT_TABLE` + (`src/dsl/grammar/sql_create_table.rs`), and here. The 4i fix teaches + the diagnostic about the `CREATE TABLE` target. +2. **Two-DDL-generators drift** — assert by a create→rebuild test that + the FK survives and is enforced after rebuild (the §6.1 net). +3. **Self-ref column-order / forward ref** — `REFERENCES emp(id)` where + `id` is defined in the same statement; SQLite accepts it. Confirm by + test, incl. the bare self-ref `REFERENCES emp`. + +## 7. Devil's Advocate review of this plan + +- **Reuse vs fork?** `do_create_table` stays the single create executor; + FK validation/naming/metadata-write are shared helpers with + `do_add_relationship` — no second relationship path. ✓ +- **DDL drift?** `do_create_table` emits the FK clause byte-identical to + `schema_to_ddl` (explicit resolved pcol + both actions), guarded by a + survives-rebuild test — the 4a `serial` lesson applied. ✓ +- **Bare-ref ambiguity?** Resolved to the single-column PK with an + explicit composite-PK error — not silently guessed. ✓ +- **Self-ref soundness?** Validated against the in-statement specs; + SQLite accepts the self-FK DDL; proven by an enforced + rebuild test. ✓ +- **One undo step?** All FK rows + the table land in the one + `snapshot_then`-wrapped create transaction. A dedicated undo test. ✓ +- **Silent scope drop?** UNIQUE-target + FK extras explicitly out (parse + error / deferred), consistent with `add relationship`. ✓ + +## 8. Implementation sequence (test-first) + +1. **Command + grammar (inline + table FK) + builder** — Tier-1 builder + tests (inline, table-level, CONSTRAINT name, bare ref, multi-FK, + action orders, self-ref shape, FK+CHECK coexistence) → red → add + `SqlForeignKey`, the grammar branches (reusing + `shared::REFERENTIAL_CLAUSES`), the builder handlers, thread the field + through `Request`/method/dispatch/runtime → green. Flip + `foreign_key_still_rejected` → accepted. +2. **Worker execution + shared helpers** — Tier-3 (FK enforced, cascade, + auto/explicit name in `REL_TABLE`, errors: non-PK, type mismatch, + composite-bare-ref; self-ref enforced) → red → factor the shared + helpers out of `do_add_relationship`, add FK resolve/validate + DDL + emission + metadata writes to `do_create_table` → green. +3. **Round-trip + describe + undo** — Tier-3 (survives rebuild + still + enforced; `describe` shows the relation; one undo step drops table + + rels) → green. +4. **Catalog/keys** — any new diagnostic keys in lockstep + vocab audit. +5. **Full sweep** — `cargo test` (no regression from 1769) + clippy. +6. **Docs** — ADR-0035 Status/§13 4b note; README; `requirements.md` Q1. + Propose commit; wait for approval. + +## 9. Exit gate + +- All §3 items satisfied; four tiers green, zero skips; no regression + from the 1769 baseline; written DA pass on the delivered slice; clippy + clean. diff --git a/docs/requirements.md b/docs/requirements.md index ccfdaac..9e394b7 100644 --- a/docs/requirements.md +++ b/docs/requirements.md @@ -220,9 +220,12 @@ handoff-14 cleanup; 449 after B2/C2.) then per-column `DEFAULT`/`CHECK` (raw `sql_expr` text) and composite `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.)* + table, since the engine reports no CHECK constraints), then foreign + keys (4b — inline `REFERENCES` + table-level `FOREIGN KEY` → ADR-0013 + named relationships in the create transaction; self-references and + bare `REFERENCES ` supported)). Remaining DDL — `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 eba95e7..0609eb5 100644 --- a/src/db.rs +++ b/src/db.rs @@ -33,7 +33,7 @@ use tracing::{debug, info, warn}; use crate::dsl::action::ReferentialAction; use crate::dsl::command::{ ChangeColumnMode, Command, CompareOp, Constraint, ConstraintKind, Expr, IndexSelector, - Operand, Predicate, RelationshipSelector, RowFilter, + Operand, Predicate, RelationshipSelector, RowFilter, SqlForeignKey, }; use crate::dsl::ColumnSpec; use crate::dsl::shortid; @@ -467,6 +467,7 @@ enum Request { primary_key: Vec, unique_constraints: Vec>, check_constraints: Vec, + foreign_keys: Vec, if_not_exists: bool, source: Option, reply: oneshot::Sender>, @@ -838,6 +839,7 @@ impl Database { primary_key: Vec, unique_constraints: Vec>, check_constraints: Vec, + foreign_keys: Vec, if_not_exists: bool, source: Option, ) -> Result { @@ -848,6 +850,7 @@ impl Database { primary_key, unique_constraints, check_constraints, + foreign_keys, if_not_exists, source, reply, @@ -1708,6 +1711,7 @@ fn handle_request( &primary_key, &[], &[], + &[], )); } Request::SqlCreateTable { @@ -1716,6 +1720,7 @@ fn handle_request( primary_key, unique_constraints, check_constraints, + foreign_keys, if_not_exists, source, reply, @@ -1745,6 +1750,7 @@ fn handle_request( &primary_key, &unique_constraints, &check_constraints, + &foreign_keys, ) .map(CreateOutcome::Created) }); @@ -2637,6 +2643,7 @@ fn do_create_table( primary_key: &[String], unique_constraints: &[Vec], check_constraints: &[String], + foreign_keys: &[SqlForeignKey], ) -> Result { if columns.is_empty() { // SQLite requires at least one column. The DSL grammar @@ -2647,6 +2654,11 @@ fn do_create_table( "tables need at least one column".to_string(), )); } + // Resolve + validate any foreign keys before building the DDL, so + // an invalid reference aborts before the table is created (ADR-0035 + // §5, sub-phase 4b). Self-references validate against the columns + // being defined; other parents must already exist. + let resolved_fks = resolve_create_table_fks(conn, name, columns, primary_key, foreign_keys)?; // Inline `PRIMARY KEY` on the column when the table has a single // primary-key column and it is the **first** column — the exact @@ -2736,6 +2748,21 @@ fn do_create_table( ddl.push(')'); } + // Foreign keys (ADR-0035 §5, sub-phase 4b) — emitted identically to + // `schema_to_ddl` (the §6.1 two-generators rule): always the + // explicit resolved parent column + both actions, so the create DDL + // and the rebuild DDL match byte-for-byte. + for fk in &resolved_fks { + ddl.push_str(&format!( + ", FOREIGN KEY ({child}) REFERENCES {parent}({pcol}) ON DELETE {od} ON UPDATE {ou}", + child = quote_ident(&fk.child_column), + parent = quote_ident(&fk.parent_table), + pcol = quote_ident(&fk.parent_column), + od = fk.on_delete.sql_clause(), + ou = fk.on_update.sql_clause(), + )); + } + ddl.push_str(") STRICT;"); debug!(ddl = %ddl, "create_table"); @@ -2761,6 +2788,21 @@ fn do_create_table( ) .map_err(DbError::from_rusqlite)?; } + // Foreign-key relationships (ADR-0035 §5): one metadata row per FK, + // in the same transaction as the table — so the whole statement is + // one undo step. + for fk in &resolved_fks { + insert_relationship_metadata( + &tx, + &fk.name, + &fk.parent_table, + &fk.parent_column, + name, + &fk.child_column, + fk.on_delete, + fk.on_update, + )?; + } let description = do_describe_table(conn, name)?; let changes = Changes { schema_dirty: true, @@ -5314,6 +5356,225 @@ where ) } +/// Auto-name a relationship per ADR-0013 +/// (`__to__`, reading in the declared +/// direction) when `name` is `None`; otherwise use the supplied name. +/// Shared by the DSL `add relationship` path and advanced-mode SQL +/// `CREATE TABLE` foreign keys (ADR-0035 §5). +fn resolve_relationship_name( + name: Option<&str>, + parent_table: &str, + parent_column: &str, + child_table: &str, + child_column: &str, +) -> String { + name.map_or_else( + || format!("{parent_table}_{parent_column}_to_{child_table}_{child_column}"), + ToString::to_string, + ) +} + +/// Reject a relationship name that collides with an existing one (the +/// `name` column is UNIQUE, ADR-0013). Engine-neutral message. +fn ensure_relationship_name_unique(conn: &Connection, name: &str) -> Result<(), DbError> { + let collision: i64 = conn + .query_row( + &format!("SELECT COUNT(*) FROM {REL_TABLE} WHERE name = ?1;"), + [name], + |row| row.get(0), + ) + .map_err(DbError::from_rusqlite)?; + if collision > 0 { + return Err(DbError::Unsupported(format!( + "a relationship named `{name}` already exists. \ + Pick a different name or drop the existing one first." + ))); + } + Ok(()) +} + +/// Insert one relationship metadata row into `REL_TABLE` (ADR-0013). +/// Shared by `add relationship` (inside its rebuild transaction) and +/// SQL `CREATE TABLE` foreign keys (inside the create transaction); +/// `conn` may be a `&Transaction` (deref-coerces to `&Connection`). +#[allow(clippy::too_many_arguments)] +fn insert_relationship_metadata( + conn: &Connection, + name: &str, + parent_table: &str, + parent_column: &str, + child_table: &str, + child_column: &str, + on_delete: ReferentialAction, + on_update: ReferentialAction, +) -> Result<(), DbError> { + conn.execute( + &format!( + "INSERT INTO {REL_TABLE} \ + (name, parent_table, parent_column, child_table, child_column, on_delete, on_update) \ + VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7);" + ), + [ + name, + parent_table, + parent_column, + child_table, + child_column, + on_delete.keyword(), + on_update.keyword(), + ], + ) + .map_err(DbError::from_rusqlite)?; + Ok(()) +} + +/// Validate that an FK child column's type is compatible with the +/// referenced parent column's type — it must equal the parent type's +/// `fk_target_type()` (ADR-0011). Engine-neutral mismatch error. +fn check_fk_type_compat( + parent_table: &str, + parent_column: &str, + parent_type: Type, + child_table: &str, + child_column: &str, + child_type: Type, +) -> Result<(), DbError> { + let expected = parent_type.fk_target_type(); + if child_type != expected { + return Err(DbError::Unsupported(format!( + "type mismatch: `{child_table}.{child_column}` is `{child_type}` but \ + a foreign key referencing `{parent_table}.{parent_column}` \ + (`{parent_type}`) requires `{expected}`. \ + Either change the column type, or pick a different FK column." + ))); + } + Ok(()) +} + +/// A `CREATE TABLE` foreign key after resolution + validation +/// (ADR-0035 §5, sub-phase 4b): the bare-`REFERENCES` parent column is +/// resolved, the relationship name is decided, and PK-target / +/// type-compat are checked. +struct ResolvedFk { + name: String, + child_column: String, + parent_table: String, + parent_column: String, + on_delete: ReferentialAction, + on_update: ReferentialAction, +} + +/// Resolve + validate every foreign key declared in a `CREATE TABLE` +/// (ADR-0035 §5, sub-phase 4b) **before** the table is built, so an +/// invalid reference aborts cleanly. A self-referencing FK (parent is +/// the table being created) is validated against the columns/PK being +/// defined; any other parent must already exist. The bare +/// `REFERENCES ` form resolves to the parent's single-column +/// PK (composite → error). Reuses the relationship validation/naming +/// helpers shared with `do_add_relationship`. +fn resolve_create_table_fks( + conn: &Connection, + table_name: &str, + columns: &[ColumnSpec], + primary_key: &[String], + foreign_keys: &[SqlForeignKey], +) -> Result, DbError> { + let mut out = Vec::with_capacity(foreign_keys.len()); + for fk in foreign_keys { + // The parent's PK column list + a (name -> user type) lookup. + // A self-reference reads the in-statement specs (the table does + // not exist yet); any other parent must already exist. + let (parent_pk, parent_cols): (Vec, Vec<(String, Option)>) = + if fk.parent_table == table_name { + ( + primary_key.to_vec(), + columns.iter().map(|c| (c.name.clone(), Some(c.ty))).collect(), + ) + } else { + let ps = read_schema(conn, &fk.parent_table)?; + ( + ps.primary_key.clone(), + ps.columns + .iter() + .map(|c| (c.name.clone(), c.user_type)) + .collect(), + ) + }; + + // Explicit referenced column, or the parent's single-column PK + // for the bare `REFERENCES ` form. + let parent_column = match &fk.parent_column { + Some(c) => c.clone(), + None => { + if parent_pk.len() == 1 { + parent_pk[0].clone() + } else { + return Err(DbError::Unsupported(format!( + "`{parent}` has a composite primary key, so a bare \ + reference is ambiguous — name the referenced column, \ + e.g. `REFERENCES {parent}()`.", + parent = fk.parent_table, + ))); + } + } + }; + + // The referenced column must be a primary key (ADR-0011/0013). + if !parent_pk.contains(&parent_column) { + return Err(DbError::Unsupported(format!( + "column `{}.{}` is not a primary key. Foreign keys must \ + reference a primary key (UNIQUE-target FKs land in a later \ + iteration).", + fk.parent_table, parent_column + ))); + } + let parent_type = parent_cols + .iter() + .find(|(n, _)| n == &parent_column) + .and_then(|(_, t)| *t) + .ok_or_else(|| DbError::Sqlite { + message: format!("no such column: {}.{}", fk.parent_table, parent_column), + kind: SqliteErrorKind::NoSuchColumn, + })?; + + // The child column must be one of the columns being defined. + let child = columns + .iter() + .find(|c| c.name == fk.child_column) + .ok_or_else(|| DbError::Sqlite { + message: format!("no such column: {}.{}", table_name, fk.child_column), + kind: SqliteErrorKind::NoSuchColumn, + })?; + check_fk_type_compat( + &fk.parent_table, + &parent_column, + parent_type, + table_name, + &fk.child_column, + child.ty, + )?; + + let resolved_name = resolve_relationship_name( + fk.name.as_deref(), + &fk.parent_table, + &parent_column, + table_name, + &fk.child_column, + ); + ensure_relationship_name_unique(conn, &resolved_name)?; + + out.push(ResolvedFk { + name: resolved_name, + child_column: fk.child_column.clone(), + parent_table: fk.parent_table.clone(), + parent_column, + on_delete: fk.on_delete, + on_update: fk.on_update, + }); + } + Ok(out) +} + #[allow(clippy::too_many_arguments)] fn do_add_relationship( conn: &Connection, @@ -5388,38 +5649,21 @@ fn do_add_relationship( let actual = child_col.user_type.ok_or_else(|| DbError::Unsupported( "child column has no user type metadata".to_string(), ))?; - if actual != expected_child_type { - return Err(DbError::Unsupported(format!( - "type mismatch: `{child_table}.{child_column}` is `{actual}` but \ - a foreign key referencing `{parent_table}.{parent_column}` \ - (`{parent_user_type}`) requires `{expected_child_type}`. \ - Either change the column type, or pick a different FK column." - ))); - } + check_fk_type_compat( + parent_table, + parent_column, + parent_user_type, + child_table, + child_column, + actual, + )?; } // 4. Determine relationship name (auto-gen or supplied) and // check uniqueness against the metadata table. - let resolved_name = name.map_or_else( - // Auto-name follows the user-typed `from . - // to .` direction so the name reads as the - // grammar reads — see ADR-0013. - || format!("{parent_table}_{parent_column}_to_{child_table}_{child_column}"), - ToString::to_string, - ); - let collision: i64 = conn - .query_row( - &format!("SELECT COUNT(*) FROM {REL_TABLE} WHERE name = ?1;"), - [&resolved_name], - |row| row.get(0), - ) - .map_err(DbError::from_rusqlite)?; - if collision > 0 { - return Err(DbError::Unsupported(format!( - "a relationship named `{resolved_name}` already exists. \ - Pick a different name or drop the existing one first." - ))); - } + let resolved_name = + resolve_relationship_name(name, parent_table, parent_column, child_table, child_column); + ensure_relationship_name_unique(conn, &resolved_name)?; // 5. Build the new schema with the FK appended. let mut new_schema = child_schema.clone(); @@ -5432,10 +5676,7 @@ fn do_add_relationship( }); // 6. Rebuild, with metadata updates inside the transaction. - let on_delete_kw = on_delete.keyword(); - let on_update_kw = on_update.keyword(); let column_user_type_kw = expected_child_type.keyword(); - let resolved_name_for_meta = resolved_name.as_str(); rebuild_table(conn, child_table, &child_schema, &new_schema, |tx| { if needs_create_column { tx.execute( @@ -5447,23 +5688,16 @@ fn do_add_relationship( ) .map_err(DbError::from_rusqlite)?; } - tx.execute( - &format!( - "INSERT INTO {REL_TABLE} \ - (name, parent_table, parent_column, child_table, child_column, on_delete, on_update) \ - VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7);" - ), - [ - resolved_name_for_meta, - parent_table, - parent_column, - child_table, - child_column, - on_delete_kw, - on_update_kw, - ], - ) - .map_err(DbError::from_rusqlite)?; + insert_relationship_metadata( + tx, + &resolved_name, + parent_table, + parent_column, + child_table, + child_column, + on_delete, + on_update, + )?; // Persistence runs inside the same tx so a write // failure rolls back both the schema and the metadata // (commit-db-last per ADR-0015 §6). diff --git a/src/dsl/command.rs b/src/dsl/command.rs index 0a9510d..ddd5dbc 100644 --- a/src/dsl/command.rs +++ b/src/dsl/command.rs @@ -15,6 +15,33 @@ use crate::dsl::action::ReferentialAction; use crate::dsl::types::Type; use crate::dsl::value::Value; +/// A foreign key declared in an advanced-mode SQL `CREATE TABLE`. +/// +/// The SQL spelling of an ADR-0013 named relationship (ADR-0035 §5, +/// sub-phase 4b). Produced by both the inline +/// ` … REFERENCES [()] …` form (always auto-named) +/// and the table-level `[CONSTRAINT ] FOREIGN KEY () +/// REFERENCES [()] …` form. The relationship is created +/// together with the table, in one transaction = one undo step. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct SqlForeignKey { + /// `CONSTRAINT ` on a table-level FK; `None` for an inline + /// FK or an unnamed table FK (auto-named at execution per + /// ADR-0013). + pub name: Option, + /// The column in the table being created that holds the FK. + pub child_column: String, + /// The referenced (parent) table — may be the table being created + /// (a self-referencing FK). + pub parent_table: String, + /// The referenced parent column. `None` for the bare + /// `REFERENCES ` form, resolved at execution to the + /// parent's single-column primary key (ADR-0035 §4b, user-confirmed). + pub parent_column: Option, + pub on_delete: ReferentialAction, + pub on_update: ReferentialAction, +} + /// A column at table-creation time: a name, a user-facing /// type, and its column-level constraints (ADR-0029). /// @@ -153,6 +180,11 @@ pub enum Command { /// 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, + /// Foreign keys (ADR-0035 §5, sub-phase 4b) — inline + /// `REFERENCES` and table-level `FOREIGN KEY`, each created as + /// an ADR-0013 named relationship in the same transaction as + /// the table (one undo step). + foreign_keys: 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 659082b..4a8129e 100644 --- a/src/dsl/grammar/ddl.rs +++ b/src/dsl/grammar/ddl.rs @@ -14,7 +14,7 @@ use crate::dsl::action::ReferentialAction; use crate::dsl::command::{ ChangeColumnMode, ColumnSpec, Command, Constraint, ConstraintKind, Expr, IndexSelector, - RelationshipSelector, + RelationshipSelector, SqlForeignKey, }; use crate::dsl::value::Value; use crate::dsl::grammar::{ @@ -1321,6 +1321,7 @@ fn build_sql_create_table(path: &MatchedPath, source: &str) -> Result = Vec::new(); let mut unique_constraints: Vec> = Vec::new(); let mut check_constraints: Vec = Vec::new(); + let mut foreign_keys: 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 @@ -1334,6 +1335,9 @@ fn build_sql_create_table(path: &MatchedPath, source: &str) -> Result` prefix stashes the name until the following + // table-level `FOREIGN KEY` consumes it (ADR-0035 §5, 4b). + let mut pending_fk_name: Option = None; let mut items = path.items.iter().peekable(); while let Some(item) = items.next() { match &item.kind { @@ -1463,10 +1467,48 @@ fn build_sql_create_table(path: &MatchedPath, source: &str) -> Result` — stash the name for the table-level + // `foreign key` that follows (ADR-0035 §5, 4b). + MatchedKind::Word("constraint") => { + if let Some(it) = items.next() { + pending_fk_name = Some(it.text.clone()); + } + } + // Inline `references [()] [on …]` — a + // column-level FK on the current column (ADR-0035 §5, 4b). + // Auto-named at execution; the FK clause's own parens are + // consumed in `consume_fk_reference`, so they don't perturb + // the element-boundary `depth` tracker. + MatchedKind::Word("references") => { + let child_column = columns.last().map_or_else(String::new, |c| c.name.clone()); + foreign_keys.push(consume_fk_reference(&mut items, None, child_column)); + } + // Table-level `[constraint ] foreign key () + // references [()] [on …]` (ADR-0035 §5, 4b). + MatchedKind::Word("foreign") => { + if matches!(items.peek().map(|i| &i.kind), Some(MatchedKind::Word("key"))) { + items.next(); // `key` + } + // `( )` + if matches!(items.peek().map(|i| &i.kind), Some(MatchedKind::Punct('('))) { + items.next(); + } + let child_column = items.next().map_or_else(String::new, |it| it.text.clone()); + if matches!(items.peek().map(|i| &i.kind), Some(MatchedKind::Punct(')'))) { + items.next(); + } + // `references …` + if matches!(items.peek().map(|i| &i.kind), Some(MatchedKind::Word("references"))) { + items.next(); + } + let fk = consume_fk_reference(&mut items, pending_fk_name.take(), child_column); + foreign_keys.push(fk); + } // 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 (…)`. + // column/`check`/`default`/table-`unique`/FK 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 @@ -1494,6 +1536,7 @@ fn build_sql_create_table(path: &MatchedPath, source: &str) -> Result )`, and any `on ` +/// clauses. The next item must be the parent-table ident (the +/// `references` keyword was already consumed by the caller). The +/// reference's own parens are consumed here, so they never reach the +/// builder's element-boundary `depth` tracker. +fn consume_fk_reference<'a, I>( + items: &mut std::iter::Peekable, + name: Option, + child_column: String, +) -> SqlForeignKey +where + I: Iterator, +{ + let parent_table = items.next().map_or_else(String::new, |it| it.text.clone()); + // Optional `( )`. + let mut parent_column = None; + if matches!(items.peek().map(|i| &i.kind), Some(MatchedKind::Punct('('))) { + items.next(); // `(` + if let Some(it) = items.next() { + parent_column = Some(it.text.clone()); + } + if matches!(items.peek().map(|i| &i.kind), Some(MatchedKind::Punct(')'))) { + items.next(); // `)` + } + } + // `on ` clauses, in either order, 0..2. + let mut on_delete = ReferentialAction::default_action(); + let mut on_update = ReferentialAction::default_action(); + while matches!(items.peek().map(|i| &i.kind), Some(MatchedKind::Word("on"))) { + items.next(); // `on` + let target = items.next().map(|it| it.kind.clone()); + let action = consume_referential_action(items); + match target { + Some(MatchedKind::Word("delete")) => on_delete = action, + Some(MatchedKind::Word("update")) => on_update = action, + _ => {} + } + } + SqlForeignKey { + name, + child_column, + parent_table, + parent_column, + on_delete, + on_update, + } +} + +/// Read a single referential action (`cascade` / `restrict` / +/// `set null` / `no action`) from the matched-item stream — the +/// two-word forms (`set null`, `no action`) consume their second word. +fn consume_referential_action<'a, I>(items: &mut std::iter::Peekable) -> ReferentialAction +where + I: Iterator, +{ + match items.next().map(|it| it.kind.clone()) { + Some(MatchedKind::Word("cascade")) => ReferentialAction::Cascade, + Some(MatchedKind::Word("restrict")) => ReferentialAction::Restrict, + Some(MatchedKind::Word("set")) => { + if matches!(items.peek().map(|i| &i.kind), Some(MatchedKind::Word("null"))) { + items.next(); + } + ReferentialAction::SetNull + } + Some(MatchedKind::Word("no")) => { + if matches!(items.peek().map(|i| &i.kind), Some(MatchedKind::Word("action"))) { + items.next(); + } + ReferentialAction::NoAction + } + _ => ReferentialAction::default_action(), + } +} + pub static SQL_CREATE_TABLE: CommandNode = CommandNode { entry: Word::keyword("create"), shape: Node::Subgrammar(&super::sql_create_table::SQL_CREATE_TABLE_SHAPE), diff --git a/src/dsl/grammar/sql_create_table.rs b/src/dsl/grammar/sql_create_table.rs index 9cd3c6d..5b4582e 100644 --- a/src/dsl/grammar/sql_create_table.rs +++ b/src/dsl/grammar/sql_create_table.rs @@ -24,7 +24,7 @@ //! `sql_insert::SQL_INSERT_SHAPE`, which starts at `INTO`). use crate::dsl::grammar::sql_select::reject_internal_table; -use crate::dsl::grammar::{IdentSource, Node, ValidationError, Word, sql_expr}; +use crate::dsl::grammar::{IdentSource, Node, ValidationError, Word, shared, sql_expr}; use crate::dsl::types::Type; static COMMA: Node = Node::Punct(','); @@ -139,6 +139,63 @@ static CHECK_NODES: &[Node] = &[ Node::Subgrammar(&sql_expr::SQL_OR_EXPR), Node::Punct(')'), ]; + +// --- Foreign keys (ADR-0035 §5, sub-phase 4b) --------------------- +// +// Inline `REFERENCES [()] [ON DELETE/UPDATE …]` and +// table-level `[CONSTRAINT ] FOREIGN KEY () REFERENCES …`. +// Each is the SQL spelling of an ADR-0013 named relationship. The +// referenced parent table/column use the `Tables`/`Columns` sources +// (completion + existence hints), matching the `add relationship` +// endpoints; the `( )` is optional (the bare `REFERENCES +// ` form resolves to the parent's PK at execution). + +// NOTE (4i): `IdentSource::Tables` existence-checks the parent — good +// for the common case (a typo'd parent shows a pre-submit hint), but a +// self-referencing FK (`references ` while creating ``) +// false-flags the not-yet-created table as unknown. Parse + execution +// are correct (the self-ref is validated against the in-statement +// columns); only the live typing indicator is briefly wrong. ADR-0035 +// §13 4i: teach the schema-existence diagnostic about the CREATE TABLE +// target so the self-ref indicator stops lying. +const FK_PARENT_TABLE: Node = Node::Ident { + source: IdentSource::Tables, + role: "fk_parent_table", + validator: None, + highlight_override: None, + writes_table: false, + writes_column: false, + writes_user_listed_column: false, + writes_table_alias: false, + writes_cte_name: false, + writes_projection_alias: false, +}; +const FK_PARENT_COLUMN: Node = Node::Ident { + source: IdentSource::Columns, + role: "fk_parent_column", + validator: None, + highlight_override: None, + writes_table: false, + writes_column: false, + writes_user_listed_column: false, + writes_table_alias: false, + writes_cte_name: false, + writes_projection_alias: false, +}; +static FK_PARENT_COL_NODES: &[Node] = &[Node::Punct('('), FK_PARENT_COLUMN, Node::Punct(')')]; +const FK_PARENT_COL_OPT: Node = Node::Optional(&Node::Seq(FK_PARENT_COL_NODES)); + +// `REFERENCES [ ( ) ] [on delete/update …]` — the inline +// column-FK constraint. The referential clauses reuse the shared +// `on ` grammar (the DSL `add relationship` +// keywords are the SQL keywords). +static REFERENCES_NODES: &[Node] = &[ + Node::Word(Word::keyword("references")), + FK_PARENT_TABLE, + FK_PARENT_COL_OPT, + shared::REFERENTIAL_CLAUSES, +]; +const REFERENCES_CLAUSE: Node = Node::Seq(REFERENCES_NODES); // `NOT NULL` | `UNIQUE` | `PRIMARY KEY` | `DEFAULT ` | // `CHECK ()`. Each branch starts on a distinct keyword, so the // `Choice` never ambiguously commits. @@ -148,6 +205,7 @@ static COL_CONSTRAINT_CHOICES: &[Node] = &[ Node::Seq(PRIMARY_KEY_NODES), Node::Seq(DEFAULT_NODES), Node::Seq(CHECK_NODES), + REFERENCES_CLAUSE, ]; const COL_CONSTRAINT: Node = Node::Choice(COL_CONSTRAINT_CHOICES); /// Zero-or-more column constraints after the type (`min: 0`). @@ -250,13 +308,80 @@ static TABLE_CHECK_NODES: &[Node] = &[ ]; const TABLE_CHECK: Node = Node::Seq(TABLE_CHECK_NODES); +// Table-level foreign key (ADR-0035 §5, sub-phase 4b): +// `[CONSTRAINT ] FOREIGN KEY ( ) REFERENCES +// [ ( ) ] [on delete/update …]`. The child column is +// being defined in this statement (`NewName`); the optional +// `CONSTRAINT ` names the relationship (an inline `REFERENCES` +// is always auto-named instead). +const FK_CHILD_COLUMN: Node = Node::Ident { + source: IdentSource::NewName, + role: "fk_child_column", + validator: None, + highlight_override: None, + writes_table: false, + writes_column: false, + writes_user_listed_column: false, + writes_table_alias: false, + writes_cte_name: false, + writes_projection_alias: false, +}; +const FK_NAME: Node = Node::Ident { + source: IdentSource::NewName, + role: "fk_name", + validator: None, + highlight_override: None, + writes_table: false, + writes_column: false, + writes_user_listed_column: false, + writes_table_alias: false, + writes_cte_name: false, + writes_projection_alias: false, +}; +// The `FOREIGN KEY (col) REFERENCES …` body, shared by the named and +// unnamed table-FK element branches. Each branch starts with a concrete +// keyword (`foreign` / `constraint`) — never a leading `Optional`, +// which would advance the Seq index and turn a later mismatch into a +// hard failure that aborts the enclosing element `Choice`. +static FOREIGN_KEY_BODY_NODES: &[Node] = &[ + Node::Word(Word::keyword("foreign")), + Node::Word(Word::keyword("key")), + Node::Punct('('), + FK_CHILD_COLUMN, + Node::Punct(')'), + Node::Word(Word::keyword("references")), + FK_PARENT_TABLE, + FK_PARENT_COL_OPT, + shared::REFERENTIAL_CLAUSES, +]; +const FOREIGN_KEY_BODY: Node = Node::Seq(FOREIGN_KEY_BODY_NODES); +// `FOREIGN KEY (…) …` — the unnamed table-level FK (auto-named). +const TABLE_FK: Node = FOREIGN_KEY_BODY; +// `CONSTRAINT FOREIGN KEY (…) …` — the named table-level FK. +static TABLE_FK_NAMED_NODES: &[Node] = &[ + Node::Word(Word::keyword("constraint")), + FK_NAME, + Node::Word(Word::keyword("foreign")), + Node::Word(Word::keyword("key")), + Node::Punct('('), + FK_CHILD_COLUMN, + Node::Punct(')'), + Node::Word(Word::keyword("references")), + FK_PARENT_TABLE, + FK_PARENT_COL_OPT, + shared::REFERENTIAL_CLAUSES, +]; +const TABLE_FK_NAMED: Node = Node::Seq(TABLE_FK_NAMED_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]; +// `UNIQUE (…)` / `CHECK (…)` / `[CONSTRAINT ] FOREIGN KEY (…)`, +// or a column definition. The table-level forms are tried first — each +// starts with a keyword (`primary` / `unique` / `check` / `constraint` +// / `foreign`) that disambiguates it from a column name. (A column +// literally named with one of those keywords is therefore unavailable, +// the same trade real SQL makes with its reserved words.) +static ELEMENT_CHOICES: &[Node] = + &[TABLE_PK, TABLE_UNIQUE, TABLE_CHECK, TABLE_FK_NAMED, TABLE_FK, COLUMN_DEF]; const ELEMENT: Node = Node::Choice(ELEMENT_CHOICES); static COLUMN_LIST_NODES: &[Node] = &[ @@ -471,11 +596,23 @@ mod tests { } #[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))"); + fn foreign_keys_accepted() { + // 4b: inline `REFERENCES` and table-level `FOREIGN KEY`, with + // optional `(col)`, `ON DELETE`/`ON UPDATE`, and `CONSTRAINT`. + good("table t (id int, ref int references other(id))"); + good("table t (id int, ref int references other)"); // bare ref + good("table t (id int, ref int references other(id) on delete cascade)"); + good("table t (id int, ref int references other(id) on update set null on delete restrict)"); + good("table t (id int, ref int, foreign key (ref) references other(id))"); + good("table t (id int, ref int, constraint fk_x foreign key (ref) references other(id))"); + good( + "table t (id int, a int, b int, foreign key (a) references p(id), \ + foreign key (b) references q(id))", + ); + // FK alongside the other table elements (coexistence). + good("table t (id int primary key, ref int references other(id), check (id > 0))"); + // self-reference (parent is the table being created). + good("table emp (id int primary key, mgr int references emp(id))"); } } @@ -487,7 +624,8 @@ mod tests { #[cfg(test)] mod builder_tests { - use crate::dsl::command::{ColumnSpec, Command}; + use crate::dsl::action::ReferentialAction; + use crate::dsl::command::{ColumnSpec, Command, SqlForeignKey}; use crate::dsl::parser::{parse_command, parse_command_in_mode}; use crate::dsl::types::Type; use crate::mode::Mode; @@ -808,4 +946,129 @@ mod builder_tests { assert_eq!(checks, vec!["a > 0".to_string()]); assert!(col(&cols, "a").check_sql.is_none() && col(&cols, "b").check_sql.is_none()); } + + // --- 4b: foreign keys (inline + table-level) --- + + /// Parse and return the foreign keys. + fn parse_sct_fks(input: &str) -> Vec { + match parse_command(input).expect("should parse") { + Command::SqlCreateTable { foreign_keys, .. } => foreign_keys, + other => panic!("expected SqlCreateTable, got {other:?}"), + } + } + + #[test] + fn inline_reference_captured() { + let fks = parse_sct_fks("create table t (id int, pid int references parent(id))"); + assert_eq!(fks.len(), 1); + let fk = &fks[0]; + assert_eq!(fk.name, None, "inline FK is auto-named at execution"); + assert_eq!(fk.child_column, "pid"); + assert_eq!(fk.parent_table, "parent"); + assert_eq!(fk.parent_column.as_deref(), Some("id")); + assert_eq!(fk.on_delete, ReferentialAction::NoAction); + assert_eq!(fk.on_update, ReferentialAction::NoAction); + } + + #[test] + fn bare_inline_reference_has_no_parent_column() { + let fks = parse_sct_fks("create table t (id int, pid int references parent)"); + assert_eq!(fks[0].parent_column, None, "bare REFERENCES — resolved at execution"); + assert_eq!(fks[0].parent_table, "parent"); + assert_eq!(fks[0].child_column, "pid"); + } + + #[test] + fn inline_reference_with_referential_actions() { + let fks = parse_sct_fks( + "create table t (id int, pid int references parent(id) \ + on delete cascade on update set null)", + ); + assert_eq!(fks[0].on_delete, ReferentialAction::Cascade); + assert_eq!(fks[0].on_update, ReferentialAction::SetNull); + } + + #[test] + fn referential_action_order_is_flexible() { + // `on update` before `on delete` — either order is accepted. + let fks = parse_sct_fks( + "create table t (id int, pid int references parent(id) \ + on update restrict on delete no action)", + ); + assert_eq!(fks[0].on_update, ReferentialAction::Restrict); + assert_eq!(fks[0].on_delete, ReferentialAction::NoAction); + } + + #[test] + fn table_level_foreign_key_captured() { + let fks = + parse_sct_fks("create table t (id int, pid int, foreign key (pid) references parent(id))"); + assert_eq!(fks.len(), 1); + assert_eq!(fks[0].name, None); + assert_eq!(fks[0].child_column, "pid"); + assert_eq!(fks[0].parent_table, "parent"); + assert_eq!(fks[0].parent_column.as_deref(), Some("id")); + } + + #[test] + fn table_level_foreign_key_with_constraint_name() { + let fks = parse_sct_fks( + "create table t (id int, pid int, \ + constraint fk_parent foreign key (pid) references parent(id))", + ); + assert_eq!(fks[0].name.as_deref(), Some("fk_parent")); + assert_eq!(fks[0].child_column, "pid"); + } + + #[test] + fn multiple_foreign_keys_collected_in_order() { + let fks = parse_sct_fks( + "create table t (id int, a int, b int, \ + foreign key (a) references p(id), foreign key (b) references q(id))", + ); + assert_eq!(fks.len(), 2); + assert_eq!((fks[0].child_column.as_str(), fks[0].parent_table.as_str()), ("a", "p")); + assert_eq!((fks[1].child_column.as_str(), fks[1].parent_table.as_str()), ("b", "q")); + } + + #[test] + fn self_referencing_foreign_key_captured() { + let fks = + parse_sct_fks("create table emp (id int primary key, mgr int references emp(id))"); + assert_eq!(fks[0].parent_table, "emp", "self-reference"); + assert_eq!(fks[0].child_column, "mgr"); + assert_eq!(fks[0].parent_column.as_deref(), Some("id")); + } + + #[test] + fn inline_fk_coexists_with_check_and_pk() { + // FK clause must not be confused with the column CHECK that + // follows, nor disturb the table-level PK / CHECK detection. + match parse_command( + "create table t (id int primary key, pid int references parent(id) check (pid > 0), \ + check (id <> pid))", + ) + .expect("parses") + { + Command::SqlCreateTable { + primary_key, + foreign_keys, + check_constraints, + columns, + .. + } => { + assert_eq!(primary_key, vec!["id".to_string()]); + assert_eq!(foreign_keys.len(), 1); + assert_eq!(foreign_keys[0].child_column, "pid"); + // the column-level CHECK still attaches to `pid` + assert_eq!( + columns.iter().find(|c| c.name == "pid").unwrap().check_sql.as_deref(), + Some("pid > 0") + ); + // the table-level CHECK is captured separately + assert_eq!(check_constraints, vec!["id <> pid".to_string()]); + } + other => panic!("expected SqlCreateTable, got {other:?}"), + } + } } diff --git a/src/dsl/mod.rs b/src/dsl/mod.rs index 5812b4a..d2e9895 100644 --- a/src/dsl/mod.rs +++ b/src/dsl/mod.rs @@ -21,7 +21,7 @@ pub mod walker; pub use action::ReferentialAction; pub use command::{ AppCommand, ChangeColumnMode, ColumnSpec, Command, CompareOp, Expr, IndexSelector, - MessagesValue, ModeValue, Operand, Predicate, RelationshipSelector, RowFilter, + MessagesValue, ModeValue, Operand, Predicate, RelationshipSelector, RowFilter, SqlForeignKey, }; pub use parser::{ParseError, parse_command}; pub use types::Type; diff --git a/src/runtime.rs b/src/runtime.rs index 57e3410..8faae5d 100644 --- a/src/runtime.rs +++ b/src/runtime.rs @@ -1926,6 +1926,7 @@ async fn execute_command_typed( primary_key, unique_constraints, check_constraints, + foreign_keys, if_not_exists, } => database .sql_create_table( @@ -1934,6 +1935,7 @@ async fn execute_command_typed( primary_key, unique_constraints, check_constraints, + foreign_keys, if_not_exists, src, ) diff --git a/tests/sql_create_table.rs b/tests/sql_create_table.rs index e9652d5..a4d48d0 100644 --- a/tests/sql_create_table.rs +++ b/tests/sql_create_table.rs @@ -18,7 +18,7 @@ //! tests drive the worker directly, mirroring `tests/sql_insert.rs`. use rdbms_playground::db::{CreateOutcome, Database}; -use rdbms_playground::dsl::{ColumnSpec, Type, Value}; +use rdbms_playground::dsl::{ColumnSpec, ReferentialAction, RowFilter, SqlForeignKey, Type, Value}; use rdbms_playground::persistence::Persistence; use rdbms_playground::project; @@ -53,6 +53,7 @@ fn created_table_appears_with_playground_types() { vec!["id".to_string()], vec![], // no composite UNIQUE vec![], // no table CHECK + vec![], // no FK false, Some("create table Widget (id int primary key, name text)".to_string()), )) @@ -91,6 +92,7 @@ fn integer_primary_key_is_plain_int() { vec!["id".to_string()], vec![], // no composite UNIQUE vec![], // no table CHECK + vec![], // no FK false, Some("create table T (id integer primary key)".to_string()), )) @@ -117,6 +119,7 @@ fn serial_pk_autoincrements_in_multi_column_table() { vec!["id".to_string()], vec![], // no composite UNIQUE vec![], // no table CHECK + vec![], // no FK false, Some("create table T (id serial primary key, name text)".to_string()), )) @@ -161,6 +164,7 @@ fn if_not_exists_is_a_noop_when_table_exists() { vec!["id".to_string()], vec![], // no composite UNIQUE vec![], // no table CHECK + vec![], // no FK false, Some("create table T (id int)".to_string()), )) @@ -173,6 +177,7 @@ fn if_not_exists_is_a_noop_when_table_exists() { vec!["id".to_string()], vec![], // no composite UNIQUE vec![], // no table CHECK + vec![], // no FK true, // IF NOT EXISTS Some("create table if not exists T (id int)".to_string()), )) @@ -200,6 +205,7 @@ fn table_without_primary_key_is_allowed() { vec![], // no primary key vec![], // no composite UNIQUE vec![], // no table CHECK + vec![], // no FK false, Some("create table Notes (body text)".to_string()), )) @@ -243,6 +249,7 @@ fn check_constraint_is_enforced() { vec!["id".to_string()], vec![], vec![], // no table CHECK + vec![], // no FK false, Some("create table T (id serial primary key, price real check (price >= 0))".to_string()), )) @@ -278,6 +285,7 @@ fn default_is_applied_when_column_omitted() { vec!["id".to_string()], vec![], vec![], // no table CHECK + vec![], // no FK false, Some("create table T (id serial primary key, label text, n int default 7)".to_string()), )) @@ -307,6 +315,7 @@ fn composite_unique_is_enforced() { vec![], vec![vec!["a".to_string(), "b".to_string()]], vec![], // no table CHECK + vec![], // no FK false, Some("create table T (a int, b int, unique (a, b))".to_string()), )) @@ -342,6 +351,7 @@ fn check_default_and_composite_unique_survive_rebuild() { vec![], vec![vec!["a".to_string(), "b".to_string()]], vec![], // no table CHECK + vec![], // no FK false, Some( "create table T (a int, b int, price real check (price >= 0), \ @@ -392,6 +402,7 @@ fn table_level_check_is_enforced() { vec![], vec![], // no composite UNIQUE vec!["a < b".to_string()], // table-level CHECK + vec![], // no FK false, Some("create table T (a int, b int, check (a < b))".to_string()), )) @@ -423,6 +434,7 @@ fn multiple_table_level_checks_all_enforced() { vec![], vec![], // no composite UNIQUE vec!["a < b".to_string(), "b < c".to_string()], + vec![], // no FK false, Some("create table T (a int, b int, c int, check (a < b), check (b < c))".to_string()), )) @@ -459,6 +471,7 @@ fn dropping_a_table_clears_its_table_check_metadata() { vec![], vec![], // no composite UNIQUE vec!["a < b".to_string()], + vec![], // no FK false, Some("create table T (a int, b int, check (a < b))".to_string()), ) @@ -496,6 +509,7 @@ fn table_level_check_survives_a_rebuild_triggering_column_add() { vec![], vec![], // no composite UNIQUE vec!["a < b".to_string()], + vec![], // no FK false, Some("create table T (a int, b int, check (a < b))".to_string()), )) @@ -547,6 +561,7 @@ fn table_level_check_survives_rebuild() { vec![], vec![], // no composite UNIQUE vec!["a < b".to_string()], + vec![], // no FK false, Some("create table T (a int, b int, check (a < b))".to_string()), )) @@ -583,6 +598,7 @@ fn if_not_exists_noop_is_journalled() { vec!["id".to_string()], vec![], // no composite UNIQUE vec![], // no table CHECK + vec![], // no FK false, Some("create table T (id int)".to_string()), )) @@ -595,6 +611,7 @@ fn if_not_exists_noop_is_journalled() { vec!["id".to_string()], vec![], // no composite UNIQUE vec![], // no table CHECK + vec![], // no FK true, Some(noop.to_string()), )) @@ -615,6 +632,7 @@ fn plain_create_errors_when_table_exists() { vec!["id".to_string()], vec![], // no composite UNIQUE vec![], // no table CHECK + vec![], // no FK false, Some("create table T (id int)".to_string()), )) @@ -626,6 +644,7 @@ fn plain_create_errors_when_table_exists() { vec!["id".to_string()], vec![], // no composite UNIQUE vec![], // no table CHECK + vec![], // no FK false, // no IF NOT EXISTS Some("create table T (id int)".to_string()), )); @@ -642,6 +661,7 @@ fn sql_create_table_is_one_undo_step() { vec!["id".to_string()], vec![], // no composite UNIQUE vec![], // no table CHECK + vec![], // no FK false, Some("create table T (id int)".to_string()), )) @@ -694,6 +714,7 @@ fn serial_pk_first_column_autoincrements_after_rebuild() { vec!["id".to_string()], vec![], // no composite UNIQUE vec![], // no table CHECK + vec![], // no FK false, Some("create table T (id serial primary key, name text)".to_string()), )) @@ -727,6 +748,7 @@ fn serial_pk_non_first_column_autoincrements_after_rebuild() { vec!["id".to_string()], vec![], // no composite UNIQUE vec![], // no table CHECK + vec![], // no FK false, Some("create table T (name text, id serial primary key)".to_string()), )) @@ -763,6 +785,7 @@ fn dropping_a_column_a_table_check_references_fails_cleanly() { vec![], vec![], // no composite UNIQUE vec!["a < b".to_string()], + vec![], // no FK false, Some("create table T (a int, b int, check (a < b))".to_string()), )) @@ -797,3 +820,544 @@ fn dropping_a_column_a_table_check_references_fails_cleanly() { 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"); } + +// ================================================================= +// Sub-phase 4b — foreign keys in CREATE TABLE (ADR-0035 §5). +// +// These drive the worker directly with `SqlForeignKey` specs; the +// grammar (text -> Command) is covered by `builder_tests`. An FK is +// the SQL spelling of an ADR-0013 named relationship, created in the +// same transaction as the table (one undo step). +// ================================================================= + +/// A simple FK spec with default (no action) referential actions. +fn fk(child_column: &str, parent_table: &str, parent_column: Option<&str>) -> SqlForeignKey { + SqlForeignKey { + name: None, + child_column: child_column.to_string(), + parent_table: parent_table.to_string(), + parent_column: parent_column.map(str::to_string), + on_delete: ReferentialAction::NoAction, + on_update: ReferentialAction::NoAction, + } +} + +/// Create `parent (id serial primary key, label text)` — the extra +/// column lets a row be inserted (a sole auto-fill serial has nothing +/// to bind). `id` auto-fills 1, 2, … on each label insert. +fn make_parent(db: &Database, r: &tokio::runtime::Runtime) { + r.block_on(db.sql_create_table( + "parent".to_string(), + vec![ColumnSpec::new("id", Type::Serial), ColumnSpec::new("label", Type::Text)], + vec!["id".to_string()], + vec![], + vec![], + vec![], // no FK + false, + Some("create table parent (id serial primary key, label text)".to_string()), + )) + .expect("create parent"); +} + +/// Insert a parent row (label only); its serial `id` auto-fills. +fn insert_parent_row(db: &Database, r: &tokio::runtime::Runtime) { + r.block_on(db.insert( + "parent".to_string(), + Some(vec!["label".to_string()]), + vec![Value::Text("x".to_string())], + Some("insert".to_string()), + )) + .expect("parent row"); +} + +#[test] +fn foreign_key_is_enforced() { + let (_p, db, _d) = open(false); + let r = rt(); + make_parent(&db, &r); + r.block_on(db.sql_create_table( + "child".to_string(), + vec![ColumnSpec::new("id", Type::Serial), ColumnSpec::new("pid", Type::Int)], + vec!["id".to_string()], + vec![], + vec![], + vec![fk("pid", "parent", Some("id"))], + false, + Some("create table child (id serial primary key, pid int references parent(id))".to_string()), + )) + .expect("create child with FK"); + + // A parent row, then a valid child referencing it. + insert_parent_row(&db, &r); // id=1 + r.block_on(db.insert( + "child".to_string(), + Some(vec!["pid".to_string()]), + vec![Value::Number("1".to_string())], + Some("insert".to_string()), + )) + .expect("child pid=1 references an existing parent"); + // A child referencing a non-existent parent is rejected. + let bad = r.block_on(db.insert( + "child".to_string(), + Some(vec!["pid".to_string()]), + vec![Value::Number("999".to_string())], + Some("insert".to_string()), + )); + assert!(bad.is_err(), "FK rejects pid=999 (no such parent)"); +} + +#[test] +fn foreign_key_creates_named_relationship_visible_in_describe() { + let (_p, db, _d) = open(false); + let r = rt(); + make_parent(&db, &r); + r.block_on(db.sql_create_table( + "child".to_string(), + vec![ColumnSpec::new("id", Type::Serial), ColumnSpec::new("pid", Type::Int)], + vec!["id".to_string()], + vec![], + vec![], + vec![fk("pid", "parent", Some("id"))], + false, + Some("create table child (id serial primary key, pid int references parent(id))".to_string()), + )) + .expect("create child with FK"); + + // The child has an outbound relationship; the parent an inbound one. + let child = r.block_on(db.describe_table("child".to_string(), None)).expect("describe child"); + assert_eq!(child.outbound_relationships.len(), 1, "child references parent"); + let rel = &child.outbound_relationships[0]; + assert_eq!(rel.name, "parent_id_to_child_pid", "auto-named per ADR-0013"); + assert_eq!(rel.other_table, "parent"); + assert_eq!(rel.local_column, "pid"); + + let parent = r.block_on(db.describe_table("parent".to_string(), None)).expect("describe parent"); + assert_eq!(parent.inbound_relationships.len(), 1, "parent is referenced by child"); +} + +#[test] +fn explicit_constraint_name_is_used() { + let (_p, db, _d) = open(false); + let r = rt(); + make_parent(&db, &r); + let mut spec = fk("pid", "parent", Some("id")); + spec.name = Some("child_to_parent".to_string()); + r.block_on(db.sql_create_table( + "child".to_string(), + vec![ColumnSpec::new("id", Type::Serial), ColumnSpec::new("pid", Type::Int)], + vec!["id".to_string()], + vec![], + vec![], + vec![spec], + false, + Some("create table child (id serial primary key, pid int, constraint child_to_parent foreign key (pid) references parent(id))".to_string()), + )) + .expect("create child with named FK"); + let child = r.block_on(db.describe_table("child".to_string(), None)).expect("describe"); + assert_eq!(child.outbound_relationships[0].name, "child_to_parent"); +} + +#[test] +fn bare_references_resolves_to_parent_single_column_pk() { + let (_p, db, _d) = open(false); + let r = rt(); + make_parent(&db, &r); + r.block_on(db.sql_create_table( + "child".to_string(), + vec![ColumnSpec::new("id", Type::Serial), ColumnSpec::new("pid", Type::Int)], + vec!["id".to_string()], + vec![], + vec![], + vec![fk("pid", "parent", None)], // bare REFERENCES parent + false, + Some("create table child (id serial primary key, pid int references parent)".to_string()), + )) + .expect("create child with bare REFERENCES"); + let child = r.block_on(db.describe_table("child".to_string(), None)).expect("describe"); + assert_eq!(child.outbound_relationships[0].other_column, "id", "resolved to parent PK"); +} + +#[test] +fn self_referencing_foreign_key_is_enforced() { + let (_p, db, _d) = open(false); + let r = rt(); + r.block_on(db.sql_create_table( + "emp".to_string(), + vec![ColumnSpec::new("id", Type::Int), ColumnSpec::new("mgr", Type::Int)], + vec!["id".to_string()], + vec![], + vec![], + vec![fk("mgr", "emp", Some("id"))], // self-reference + false, + Some("create table emp (id int primary key, mgr int references emp(id))".to_string()), + )) + .expect("create self-referencing emp"); + let ins = |id: &str, mgr: Option<&str>| { + let (cols, vals) = mgr.map_or_else( + || (vec!["id".to_string()], vec![Value::Number(id.to_string())]), + |m| { + ( + vec!["id".to_string(), "mgr".to_string()], + vec![Value::Number(id.to_string()), Value::Number(m.to_string())], + ) + }, + ); + db.insert("emp".to_string(), Some(cols), vals, Some("insert".to_string())) + }; + r.block_on(ins("1", None)).expect("root (mgr NULL)"); + r.block_on(ins("2", Some("1"))).expect("emp 2 reports to 1"); + assert!(r.block_on(ins("3", Some("99"))).is_err(), "self-FK rejects mgr=99"); +} + +#[test] +fn foreign_key_type_mismatch_is_rejected() { + let (_p, db, _d) = open(false); + let r = rt(); + make_parent(&db, &r); // parent.id is serial -> fk_target_type int + // child.pid declared text -> incompatible with int. + let res = r.block_on(db.sql_create_table( + "child".to_string(), + vec![ColumnSpec::new("id", Type::Serial), ColumnSpec::new("pid", Type::Text)], + vec!["id".to_string()], + vec![], + vec![], + vec![fk("pid", "parent", Some("id"))], + false, + Some("create table child (id serial primary key, pid text references parent(id))".to_string()), + )); + assert!(res.is_err(), "FK column type must match the parent's fk_target_type"); +} + +#[test] +fn foreign_key_to_non_pk_column_is_rejected() { + let (_p, db, _d) = open(false); + let r = rt(); + r.block_on(db.sql_create_table( + "parent".to_string(), + vec![ColumnSpec::new("id", Type::Serial), ColumnSpec::new("label", Type::Text)], + vec!["id".to_string()], + vec![], + vec![], + vec![], + false, + Some("create table parent (id serial primary key, label text)".to_string()), + )) + .expect("create parent"); + let res = r.block_on(db.sql_create_table( + "child".to_string(), + vec![ColumnSpec::new("id", Type::Serial), ColumnSpec::new("plabel", Type::Text)], + vec!["id".to_string()], + vec![], + vec![], + vec![fk("plabel", "parent", Some("label"))], // label is not a PK + false, + Some("create table child (id serial primary key, plabel text references parent(label))".to_string()), + )); + assert!(res.is_err(), "FK must target a primary key"); +} + +#[test] +fn foreign_key_survives_rebuild() { + let (p, db, _d) = open(false); + let r = rt(); + make_parent(&db, &r); + r.block_on(db.sql_create_table( + "child".to_string(), + vec![ColumnSpec::new("id", Type::Serial), ColumnSpec::new("pid", Type::Int)], + vec!["id".to_string()], + vec![], + vec![], + vec![fk("pid", "parent", Some("id"))], + false, + Some("create table child (id serial primary key, pid int references parent(id))".to_string()), + )) + .expect("create child with FK"); + r.block_on(db.rebuild_from_text(p.path().to_path_buf(), None)).expect("rebuild"); + + insert_parent_row(&db, &r); + assert!( + r.block_on(db.insert( + "child".to_string(), + Some(vec!["pid".to_string()]), + vec![Value::Number("999".to_string())], + Some("insert".to_string()), + )) + .is_err(), + "FK still enforced after rebuild" + ); +} + +#[test] +fn create_table_with_fk_is_one_undo_step() { + let (_p, db, _d) = open(true); // undo enabled + let r = rt(); + make_parent(&db, &r); + r.block_on(db.sql_create_table( + "child".to_string(), + vec![ColumnSpec::new("id", Type::Serial), ColumnSpec::new("pid", Type::Int)], + vec!["id".to_string()], + vec![], + vec![], + vec![fk("pid", "parent", Some("id"))], + false, + Some("create table child (id serial primary key, pid int references parent(id))".to_string()), + )) + .expect("create child with FK"); + // One undo removes the child table AND its relationship row, so the + // parent (now un-referenced) can be described without a dangling rel. + r.block_on(db.undo()).expect("undo").expect("a step was undone"); + assert!(!r.block_on(db.list_tables()).unwrap().contains(&"child".to_string())); + let parent = r.block_on(db.describe_table("parent".to_string(), None)).expect("describe parent"); + assert!(parent.inbound_relationships.is_empty(), "the relationship was undone with the table"); +} + +#[test] +fn foreign_key_on_delete_cascade_takes_effect() { + // Proves the referential action reaches the engine DDL (not just + // the metadata): deleting a parent row cascades to the children. + let (_p, db, _d) = open(false); + let r = rt(); + make_parent(&db, &r); + let mut spec = fk("pid", "parent", Some("id")); + spec.on_delete = ReferentialAction::Cascade; + r.block_on(db.sql_create_table( + "child".to_string(), + vec![ColumnSpec::new("id", Type::Serial), ColumnSpec::new("pid", Type::Int)], + vec!["id".to_string()], + vec![], + vec![], + vec![spec], + false, + Some( + "create table child (id serial primary key, pid int references parent(id) \ + on delete cascade)" + .to_string(), + ), + )) + .expect("create child with ON DELETE CASCADE"); + insert_parent_row(&db, &r); // id=1 + r.block_on(db.insert( + "child".to_string(), + Some(vec!["pid".to_string()]), + vec![Value::Number("1".to_string())], + Some("insert".to_string()), + )) + .expect("child referencing parent 1"); + // Delete the parent row; the child should cascade away. + r.block_on(db.delete( + "parent".to_string(), + RowFilter::eq("id", Value::Number("1".to_string())), + Some("delete".to_string()), + )) + .expect("delete parent"); + let child_rows = r + .block_on(db.query_data("child".to_string(), None, None, None)) + .expect("query child"); + assert!(child_rows.rows.is_empty(), "ON DELETE CASCADE removed the child row"); +} + +#[test] +fn foreign_key_to_unknown_parent_is_rejected() { + let (_p, db, _d) = open(false); + let r = rt(); + let res = r.block_on(db.sql_create_table( + "child".to_string(), + vec![ColumnSpec::new("id", Type::Serial), ColumnSpec::new("pid", Type::Int)], + vec!["id".to_string()], + vec![], + vec![], + vec![fk("pid", "ghost", Some("id"))], // no such table + false, + Some("create table child (id serial primary key, pid int references ghost(id))".to_string()), + )); + assert!(res.is_err(), "a FK to a non-existent parent table is rejected"); + // And the failed create left nothing behind. + assert!(!r.block_on(db.list_tables()).unwrap().contains(&"child".to_string())); +} + +#[test] +fn composite_pk_bare_reference_is_rejected() { + let (_p, db, _d) = open(false); + let r = rt(); + r.block_on(db.sql_create_table( + "cp".to_string(), + vec![ColumnSpec::new("a", Type::Int), ColumnSpec::new("b", Type::Int)], + vec!["a".to_string(), "b".to_string()], // composite PK + vec![], + vec![], + vec![], + false, + Some("create table cp (a int, b int, primary key (a, b))".to_string()), + )) + .expect("create composite-PK parent"); + // A bare `REFERENCES cp` cannot disambiguate which PK column. + let res = r.block_on(db.sql_create_table( + "child".to_string(), + vec![ColumnSpec::new("id", Type::Serial), ColumnSpec::new("ref", Type::Int)], + vec!["id".to_string()], + vec![], + vec![], + vec![fk("ref", "cp", None)], // bare reference to a composite-PK parent + false, + Some("create table child (id serial primary key, ref int references cp)".to_string()), + )); + assert!(res.is_err(), "bare REFERENCES to a composite-PK parent must be rejected"); +} + +#[test] +fn fk_survives_a_rebuild_triggering_column_add() { + // Cross-cutting (ADR-0013 rebuild primitive × 4b): adding a + // constrained column to a child that has an FK rebuilds the table + // via schema_to_ddl. The FK must survive in the engine AND the + // relationship metadata (so a later rebuild_from_text re-emits it). + let (p, db, _d) = open(false); + let r = rt(); + make_parent(&db, &r); + r.block_on(db.sql_create_table( + "child".to_string(), + vec![ColumnSpec::new("id", Type::Serial), ColumnSpec::new("pid", Type::Int)], + vec!["id".to_string()], + vec![], + vec![], + vec![fk("pid", "parent", Some("id"))], + false, + Some("create table child (id serial primary key, pid int references parent(id))".to_string()), + )) + .expect("create child with FK"); + // A UNIQUE column forces the rebuild path. + let mut c = ColumnSpec::new("code", Type::Int); + c.unique = true; + r.block_on(db.add_column("child".to_string(), c, Some("add column child: code(int) unique".to_string()))) + .expect("add column via rebuild"); + + // The relationship still exists after the rebuild. + let child = r.block_on(db.describe_table("child".to_string(), None)).expect("describe"); + assert_eq!(child.outbound_relationships.len(), 1, "FK survived the column-add rebuild"); + // And the engine still enforces it (now and after a fresh rebuild). + insert_parent_row(&db, &r); + r.block_on(db.rebuild_from_text(p.path().to_path_buf(), None)).expect("rebuild"); + assert!( + r.block_on(db.insert( + "child".to_string(), + Some(vec!["pid".to_string()]), + vec![Value::Number("999".to_string())], + Some("insert".to_string()), + )) + .is_err(), + "FK still enforced after the column-add rebuild and a later rebuild_from_text" + ); +} + +#[test] +fn fk_referential_actions_survive_rebuild() { + // The actions (not just the FK's existence) must round-trip through + // pragma -> REL_TABLE -> project.yaml -> rebuild. + let (p, db, _d) = open(false); + let r = rt(); + make_parent(&db, &r); + let mut spec = fk("pid", "parent", Some("id")); + spec.on_delete = ReferentialAction::Cascade; + spec.on_update = ReferentialAction::SetNull; + r.block_on(db.sql_create_table( + "child".to_string(), + vec![ColumnSpec::new("id", Type::Serial), ColumnSpec::new("pid", Type::Int)], + vec!["id".to_string()], + vec![], + vec![], + vec![spec], + false, + Some( + "create table child (id serial primary key, pid int references parent(id) \ + on delete cascade on update set null)" + .to_string(), + ), + )) + .expect("create"); + r.block_on(db.rebuild_from_text(p.path().to_path_buf(), None)).expect("rebuild"); + let child = r.block_on(db.describe_table("child".to_string(), None)).expect("describe"); + let rel = &child.outbound_relationships[0]; + assert_eq!(rel.on_delete, ReferentialAction::Cascade, "ON DELETE survived rebuild"); + assert_eq!(rel.on_update, ReferentialAction::SetNull, "ON UPDATE survived rebuild"); +} + +#[test] +fn dropping_the_child_clears_the_fk_relationship() { + let (_p, db, _d) = open(false); + let r = rt(); + make_parent(&db, &r); + r.block_on(db.sql_create_table( + "child".to_string(), + vec![ColumnSpec::new("id", Type::Serial), ColumnSpec::new("pid", Type::Int)], + vec!["id".to_string()], + vec![], + vec![], + vec![fk("pid", "parent", Some("id"))], + false, + Some("create table child (id serial primary key, pid int references parent(id))".to_string()), + )) + .expect("create"); + r.block_on(db.drop_table("child".to_string(), Some("drop table child".to_string()))) + .expect("drop child"); + let parent = r.block_on(db.describe_table("parent".to_string(), None)).expect("describe parent"); + assert!(parent.inbound_relationships.is_empty(), "dropping the child cleared the relationship"); +} + +#[test] +fn dropping_a_referenced_parent_is_refused() { + // ADR-0013: a parent with inbound relationships can't be dropped. + let (_p, db, _d) = open(false); + let r = rt(); + make_parent(&db, &r); + r.block_on(db.sql_create_table( + "child".to_string(), + vec![ColumnSpec::new("id", Type::Serial), ColumnSpec::new("pid", Type::Int)], + vec!["id".to_string()], + vec![], + vec![], + vec![fk("pid", "parent", Some("id"))], + false, + Some("create table child (id serial primary key, pid int references parent(id))".to_string()), + )) + .expect("create"); + assert!( + r.block_on(db.drop_table("parent".to_string(), Some("drop table parent".to_string()))).is_err(), + "a referenced parent can't be dropped while the child's FK exists" + ); +} + +#[test] +fn bare_self_reference_resolves_to_own_pk() { + let (_p, db, _d) = open(false); + let r = rt(); + r.block_on(db.sql_create_table( + "emp".to_string(), + vec![ColumnSpec::new("id", Type::Int), ColumnSpec::new("mgr", Type::Int)], + vec!["id".to_string()], + vec![], + vec![], + vec![fk("mgr", "emp", None)], // bare AND self-referential + false, + Some("create table emp (id int primary key, mgr int references emp)".to_string()), + )) + .expect("create self-referential emp with a bare reference"); + let emp = r.block_on(db.describe_table("emp".to_string(), None)).expect("describe"); + assert_eq!(emp.outbound_relationships[0].other_column, "id", "bare self-ref resolved to own PK"); + // Enforced: a non-existent manager is rejected. + r.block_on(db.insert( + "emp".to_string(), + Some(vec!["id".to_string()]), + vec![Value::Number("1".to_string())], + Some("insert".to_string()), + )) + .expect("root row"); + assert!( + r.block_on(db.insert( + "emp".to_string(), + Some(vec!["id".to_string(), "mgr".to_string()]), + vec![Value::Number("2".to_string()), Value::Number("99".to_string())], + Some("insert".to_string()), + )) + .is_err(), + "bare self-ref FK is enforced" + ); +}