feat: ADR-0035 4b — foreign keys in CREATE TABLE

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 `<col> … REFERENCES <parent>[(<col>)] [ON DELETE/UPDATE
  …]` (a new column constraint) and table-level `[CONSTRAINT <name>]
  FOREIGN KEY (<col>) 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 <parent>` 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.
This commit is contained in:
claude@clouddev1
2026-05-25 15:35:48 +00:00
parent 60111f69d5
commit 76d60591bf
11 changed files with 1588 additions and 81 deletions
+33 -9
View File
@@ -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 (**4b4i**, §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 (**4c4i**, §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 <parent>[(<col>)] [ON DELETE/UPDATE …]` + table-level
`[CONSTRAINT <name>] FOREIGN KEY (<col>) 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 <parent>`** 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 <name>`. 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 <self>`) 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
+1 -1
View File
File diff suppressed because one or more lines are too long
+266
View File
@@ -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
`<col> <type> REFERENCES <parent>[(<pcol>)] [ON DELETE …] [ON UPDATE …]`
and table-level `[CONSTRAINT <name>] FOREIGN KEY (<ccol>) REFERENCES
<parent>[(<pcol>)] [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 <parent>` 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 <name>`** (ADR §4/§5). Auto-name:
`<Parent>_<pcol>_to_<Child>_<ccol>` (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 <parent>[(<pcol>)] [ON DELETE <a>]
[ON UPDATE <a>]` parses (advanced mode); child column = the column it
is attached to.
- [ ] Table-level `[CONSTRAINT <name>] FOREIGN KEY (<ccol>) REFERENCES
<parent>[(<pcol>)] [ON DELETE <a>] [ON UPDATE <a>]` parses; `<ccol>`
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 <parent>` 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 <name>`; 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 <delete|update> <action>` repeat, 0..2) — same keywords as SQL.
- **Inline FK** — new `COL_CONSTRAINT_CHOICES` branch
`REFERENCES <parent_ident> [ '(' <pcol_ident> ')' ] 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 <name_ident>] FOREIGN KEY '(' <ccol_ident> ')' REFERENCES
<parent_ident> [ '(' <pcol_ident> ')' ] 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<String>, child_column: String,
parent_table: String, parent_column: Option<String>, on_delete,
on_update }` accumulates into `Command::SqlCreateTable.foreign_keys`.
### 4.3 Command AST (`src/dsl/command.rs`)
`SqlCreateTable` gains `foreign_keys: Vec<SqlForeignKey>` where
`SqlForeignKey { name: Option<String>, child_column: String,
parent_table: String, parent_column: Option<String>, 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 (<ccol>) REFERENCES <parent>(<resolved_pcol>) ON
DELETE <od> ON UPDATE <ou>` **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` (4e4h).
- 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 <self>`) 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.
+6 -3
View File
@@ -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` (4e4h) —
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 <parent>` supported)). Remaining DDL — `DROP TABLE`
(4c), indexes (4d), `ALTER TABLE` (4e4h) — 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