Files
rdbms-playground/docs/plans/20260525-adr-0035-sql-ddl-4b.md
claude@clouddev1 76d60591bf 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.
2026-05-25 15:35:48 +00:00

267 lines
13 KiB
Markdown
Raw Permalink Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# 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.