From 4752ba29a0266185c8b4f864ee8a85204bb75f83 Mon Sep 17 00:00:00 2001 From: "claude@clouddev1" Date: Tue, 9 Jun 2026 18:44:37 +0000 Subject: [PATCH] =?UTF-8?q?feat:=20compound-PK=20foreign-key=20references?= =?UTF-8?q?=20=E2=80=94=20grammar=20+=20tests=20(ADR-0043)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Multi-column FK parsing on both surfaces: DSL from P.(a, b) to C.(x, y) (parenthesized endpoint; single bare form unchanged) and SQL FOREIGN KEY (a, b) REFERENCES P(x, y) incl. bare-reference auto-expand. consume_fk_reference + the table-level/ALTER FK parsers collect column lists; the from P. completion now offers ( (snapshots updated). 12 integration tests in tests/it/compound_fk.rs cover parse (both surfaces), engine-enforced FK, arity + partial-PK + per-pair-type-mismatch refusal, --create-fk per-column, save->rebuild round-trip, undo (one step), and single-column preservation. Mark T3 [x]; ADR-0043 implemented. --- ...0043-compound-pk-foreign-key-references.md | 13 +- docs/adr/README.md | 2 +- docs/requirements.md | 44 +- src/dsl/grammar/ddl.rs | 166 ++++-- src/dsl/grammar/sql_create_table.rs | 21 +- tests/it/compound_fk.rs | 549 ++++++++++++++++++ tests/it/main.rs | 1 + ...rows_to_child_columns@after_child_dot.snap | 11 + ...ws_to_parent_columns@after_parent_dot.snap | 11 + 9 files changed, 737 insertions(+), 81 deletions(-) create mode 100644 tests/it/compound_fk.rs diff --git a/docs/adr/0043-compound-pk-foreign-key-references.md b/docs/adr/0043-compound-pk-foreign-key-references.md index e2404f2..55ea21d 100644 --- a/docs/adr/0043-compound-pk-foreign-key-references.md +++ b/docs/adr/0043-compound-pk-foreign-key-references.md @@ -2,8 +2,17 @@ ## Status -**Accepted** — 2026-06-09. All four genuine forks confirmed by the -user at the recommended option: **F-A** full PK in order, **F-B** +**Accepted + implemented** — 2026-06-09. Implementation landed the +same day: the relationship model went list-based through all six +layers (refactor commit `b14f019`, single-column preserved), then +the DSL + SQL grammars gained multi-column parsing and the +executor the full-PK/auto-expand/per-pair-type-compat/auto-name/ +`--create-fk`-per-column logic. Verified by 12 integration tests in +`tests/it/compound_fk.rs` (parse both surfaces, engine-enforced FK, +arity + partial-PK refusal, `--create-fk`, single-column +preserved) on top of the existing single-column relationship +suite. `requirements.md` **T3** is `[x]`. All four genuine forks +confirmed by the user at the recommended option: **F-A** full PK in order, **F-B** house-style uniform column lists (no migration; back-compat not required), **F-C** parenthesized DSL lists, **F-D** bare table-level SQL FK auto-expands to the parent's full PK. Closes the one open diff --git a/docs/adr/README.md b/docs/adr/README.md index e3ffcf2..15b4537 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -48,4 +48,4 @@ This directory contains the project's ADRs, recorded per - [ADR-0040 — A per-command completion marker (✓/✗) replaces the `[ok]` summary line](0040-completion-marker-replaces-ok-summary.md) — **Accepted 2026-05-30 (issue #9)**, amends ADR-0014 / ADR-0028 / ADR-0019 output conventions, builds on ADR-0037's mode-tagged echo. An audit of the whole command surface found the `[ok] ` summary line duplicates the echo line above it (verb+subject) everywhere; its only unique contribution is the success-vs-error signal (and `explain select` even rendered `[ok] explain` with an empty subject post-ADR-0039). Decision: drop the `[ok]` line and the symmetric `"…" failed:` prefix; the echo line gains a trailing inline **✓** (green, success) / **✗** (red, failure) — `running:` becomes a pending state that resolves to ` ✓/✗` on completion (status set via the existing `rfind(Echo)` lookup). Content (row counts, structure, data, plan tree, teaching echo) unchanged. Scoped to the DSL/data/SQL family that has the redundant echo+`[ok]` pair; app-command `[ok]` lines (`rebuild`/`export`/`now editing`) are payload-bearing, have no echo to mark, and stay as-is. `ok.summary` retired; `dsl.failed` reduced to the rendered reason. Broad but mechanical snapshot churn. OOS: app-command `[ok]` lines, the `[WRN]` validity indicator, and the tag colours (issue #10) - [ADR-0041 — Copy the output panel to the system clipboard](0041-copy-output-to-clipboard.md) — **Accepted 2026-06-02 (issue #11)**, amends ADR-0003's app-command registry (adds **`copy`** / `copy all` / `copy last`). The friction it removes: filing a bug report meant terminal-selecting the output panel and fighting wrapping/borders. New **app-level command** (sigil-free, both modes): `copy` / `copy all` copy the whole panel; `copy last` copies from the most recent echo line to the end. **Mechanism — OSC 52 *and* native (`arboard`), always both**, because OSC 52 acceptance is undetectable (no terminal ack), so a true "fall back when unsupported" can't be built: emit the OSC 52 escape (no new dep — `base64`+`crossterm`; works over SSH; tmux-passthrough-wrapped via `$TMUX`), then a best-effort native write whose failure is ignored (headless host — OSC 52 carried it); the two carry identical content. **Format — plain text verbatim as rendered** (tags, `✓`/`✗`, box-drawing) joined by `\n`, without viewport padding/wrapping; a drift-lock test pins `OutputLine::plain_text` to `render_output_line`. `arboard` added **`--no-default-features`** (drops the `image` crate; X11-only on Linux — `wayland-data-control` deliberately omitted as it ~doubles the dep tree and OSC 52 covers native-Wayland). Security: write-only, scans clean for arboard's tree (cargo audit / osv-scanner / grype), 1Password-maintained, minimal surface. OOS: Markdown export, selection/range, a keybinding, OSC 52 read, `screen` passthrough - [ADR-0042 — H1a parse-error pedagogy in the grammar-tree era](0042-h1a-parse-error-pedagogy-grammar-tree.md) — **Accepted 2026-06-03.** Continues **H1a** from ADR-0021 against the ADR-0024 grammar tree (ADR-0021's chumsky mechanism is dead). Records the **baseline already shipped** — per-command `usage:` block (38 `parse.usage.*` templates), available-commands fallback, structural "after `…`, expected …" wording, source-derived ident slot labels ("table name"/"column name"), curated `parse.custom.*` near-miss messages, and the ADR-0027/0033/0036 schema-aware `[ERR]` diagnostics — so H1a is *substantially* delivered at the intent level. Defines the remaining work as **(1)** a verified per-command **near-miss matrix** (`tests/typing_surface/` + `tests/it/parse_error_pedagogy.rs`) as the definition of done, test-first; **(2)** **friendlier literal expectation labels** — optional prose glosses on `Word`/`Punct`/`Flag` positions that *add* role context while always keeping the exact literal visible (e.g. "a filter clause: `where …` or `--all-rows`"); **(3)** **advanced-mode SQL** near-miss parity (RETURNING scope, CTE-arity positioning, `CROSS JOIN … ON`, INSERT…SELECT count) — **in scope**, kept distinct from ADR-0019 §OOS-2 which covers advanced-SQL *engine*-error sanitisation, a different layer. Catalog/anchor-phrase discipline (ADR-0019) preserved; no public API change. OOS: I3/I4, spell-correction, multi-error reporting, verbosity-gating the usage block -- [ADR-0043 — Compound-primary-key foreign-key references (T3)](0043-compound-pk-foreign-key-references.md) — **Accepted 2026-06-09** (all four forks confirmed at the recommended option: full-PK matching, house-style uniform lists, parenthesized DSL syntax, bare-SQL-FK auto-expansion). Closes the open leg of `requirements.md` **T3**: a foreign key that *references* a parent's compound primary key. A 2026-06-09 audit found single-column FK woven through ~15–20 sites (metadata table, `RelationshipSchema`, `project.yaml` `RawEndpoint`, both grammar surfaces, executor FK-DDL emission, per-column type-compat, display) — earns an ADR, not an inline build. **Decision:** reference the parent's **full** compound PK, matched **positionally** to an equal-length child column list, per-pair `fk_target_type` compat (ADR-0011, element-wise); DSL `from

.(a, b) to .(x, y)` (single form unchanged), SQL `FOREIGN KEY (x, y) REFERENCES P(a, b)` (extend the existing one-cap lists; bare table-level FK auto-expands to the parent PK when arities match). **Storage — no migration (back-compat not required, user-confirmed 2026-06-09; no installed base):** the relationship endpoint joins the list convention `project.yaml` *already* uses — `columns: [a, b]` like `primary_key: [id]` and index `columns: [...]` (the endpoint was the lone scalar `column:` holdout); the metadata `TEXT` columns are unchanged and store the list as a uniform JSON array (`["id"]` even for single — SQLite has no array type). No F3 migrator, no version bump; accepted trade-off is that a pre-change `project.yaml` with relationships won't load (clean cutover). In-memory model goes list-based (`Vec`) through all six layers; the enforced FK is the rebuilt child-table DDL (`FOREIGN KEY (a,b) REFERENCES P(x,y)`), one relationship = one undo step (ADR-0013). Genuine forks escalated: matching policy (full-PK vs subset), storage (house-style uniform lists vs normalized table), DSL syntax (parenthesized vs repeated-dotted), bare-SQL-FK auto-expansion. OOS: subset/non-PK (UNIQUE-targeted) FK references; any single-column behaviour change +- [ADR-0043 — Compound-primary-key foreign-key references (T3)](0043-compound-pk-foreign-key-references.md) — **Accepted + implemented 2026-06-09** (all four forks confirmed at the recommended option: full-PK matching, house-style uniform lists, parenthesized DSL syntax, bare-SQL-FK auto-expansion). Closes `requirements.md` **T3** `[x]` — the relationship model went list-based across six layers (single-column preserved, no migration), DSL `from P.(a,b) to C.(x,y)` + SQL `FOREIGN KEY (a,b) REFERENCES P(x,y)` parse/execute/enforce, 12 tests in `tests/it/compound_fk.rs`. Closes the open leg of `requirements.md` **T3**: a foreign key that *references* a parent's compound primary key. A 2026-06-09 audit found single-column FK woven through ~15–20 sites (metadata table, `RelationshipSchema`, `project.yaml` `RawEndpoint`, both grammar surfaces, executor FK-DDL emission, per-column type-compat, display) — earns an ADR, not an inline build. **Decision:** reference the parent's **full** compound PK, matched **positionally** to an equal-length child column list, per-pair `fk_target_type` compat (ADR-0011, element-wise); DSL `from

.(a, b) to .(x, y)` (single form unchanged), SQL `FOREIGN KEY (x, y) REFERENCES P(a, b)` (extend the existing one-cap lists; bare table-level FK auto-expands to the parent PK when arities match). **Storage — no migration (back-compat not required, user-confirmed 2026-06-09; no installed base):** the relationship endpoint joins the list convention `project.yaml` *already* uses — `columns: [a, b]` like `primary_key: [id]` and index `columns: [...]` (the endpoint was the lone scalar `column:` holdout); the metadata `TEXT` columns are unchanged and store the list **comma-joined** (`a,b`; the bare name for single — safe because identifiers are `[A-Za-z0-9_]+`). No F3 migrator, no version bump; accepted trade-off is that a pre-change `project.yaml` with relationships won't load (clean cutover). In-memory model goes list-based (`Vec`) through all six layers; the enforced FK is the rebuilt child-table DDL (`FOREIGN KEY (a,b) REFERENCES P(x,y)`), one relationship = one undo step (ADR-0013). Genuine forks escalated: matching policy (full-PK vs subset), storage (house-style uniform lists vs normalized table), DSL syntax (parenthesized vs repeated-dotted), bare-SQL-FK auto-expansion. OOS: subset/non-PK (UNIQUE-targeted) FK references; any single-column behaviour change diff --git a/docs/requirements.md b/docs/requirements.md index 6537f08..ed3fb29 100644 --- a/docs/requirements.md +++ b/docs/requirements.md @@ -399,30 +399,28 @@ since ADR-0027.) *(Implemented per ADR-0014; auto-fills omitted shortid columns and validates user-supplied values against the same alphabet and length range.)* -- [/] **T3** Compound primary keys handled end-to-end (DSL, +- [x] **T3** Compound primary keys handled end-to-end (DSL, storage, display, FK reference). - *(Partial, verified 2026-06-07: compound-PK **declaration** - (`with pk a(int),b(int)`), **storage** (`primary_key: - Vec`), and **display** are present and tested. - **Missing: a FK that *references* a compound PK** — - `db.rs` resolve/alter FK paths enforce a single - `parent_column: String`; a bare `REFERENCES parent` on a - compound-PK table is refused as ambiguous, and multi-column FK - target syntax is not in the grammar. This is the one open - end-to-end leg of T3 — but a **codebase audit (2026-06-09) - found it is not a small finish**: single-column FK is woven - through ~15–20 sites across 6+ files — the - `__rdbms_playground_relationships` table schema, the - `RelationshipSchema` struct, the **`project.yaml` relationship - format** (`RawEndpoint { column }`), both grammar surfaces - (`add 1:n relationship` + SQL `FOREIGN KEY`), the executor's FK - DDL emission, and the per-column type-compat check. It needs a - **migration** (the metadata-table + yaml-format change, F3) and - an **ADR** to settle the design forks: compound-PK matching - policy (must an FK reference *all* PK columns, or a subset?), - per-pair type-compat semantics, the yaml multi-column shape, and - back-compat for existing single-column projects. So this leg is - ADR-first, not a sweep item.)* + *(Done 2026-06-09 via **ADR-0043**: the FK-reference leg now + works on both surfaces — DSL `add 1:n relationship from + P.(a, b) to C.(x, y)` and SQL `FOREIGN KEY (a, b) REFERENCES + P(x, y)` (bare `REFERENCES P` auto-expands to the full PK). + References the parent's full compound PK matched positionally, + per-pair type-compat (ADR-0011); the FK is engine-enforced, + persisted (`columns: [a, b]` in `project.yaml`, comma-joined in + metadata), shown symmetrically by `describe`, and `--create-fk` + creates one child column per parent PK column. The relationship + model went list-based through all six layers, single-column + behaviour preserved (commit `b14f019`); 12 integration tests in + `tests/it/compound_fk.rs` plus the existing single-column suite + as the regression net. The earlier-noted (2026-06-07) breakdown:* + *compound-PK **declaration** (`with pk a(int),b(int)`), + **storage** (`primary_key: Vec`), and **display** were + already present and tested. The FK-reference leg — once an + ADR-first ~15–20-site change across the relationship model — is + what ADR-0043 delivered (back-compat dropped by user decision, so + no migration was needed). Subset / non-PK (UNIQUE-target) FK + references stay out of scope.)* ## Visualizations diff --git a/src/dsl/grammar/ddl.rs b/src/dsl/grammar/ddl.rs index 1f37142..35b15b5 100644 --- a/src/dsl/grammar/ddl.rs +++ b/src/dsl/grammar/ddl.rs @@ -385,6 +385,36 @@ const ADD_COLUMN: Node = Node::Seq(ADD_COLUMN_NODES); // `writes_table: true` on each endpoint's table ident so the // `.` slot narrows to that table's columns (handoff-13 // §2.2 follow-up — mirrors DR_PARENT / DR_CHILD). +// A single FK-endpoint column ident (narrows to the endpoint +// table's columns via the table ident's `writes_table: true`). +const AR_PARENT_COL: Node = Node::Ident { + source: IdentSource::Columns, + role: "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, +}; +// Compound endpoint: `( a, b, … )` — a comma-separated column list +// in parens (ADR-0043). Same role as the single form, so the +// builder collects either shape uniformly. +const AR_PARENT_COL_LIST: Node = Node::Repeated { + inner: &AR_PARENT_COL, + separator: Some(&Node::Punct(',')), + min: 1, +}; +const AR_PARENT_COLS_PAREN_NODES: &[Node] = + &[Node::Punct('('), AR_PARENT_COL_LIST, Node::Punct(')')]; +const AR_PARENT_COLS_PAREN: Node = Node::Seq(AR_PARENT_COLS_PAREN_NODES); +// `from P.(a, b)` (compound) or `from P.col` (single) — Choice on +// the first post-`.` token (`(` vs an ident), so order is safe. +const AR_PARENT_COLS_CHOICES: &[Node] = &[AR_PARENT_COLS_PAREN, AR_PARENT_COL]; +const AR_PARENT_COLS: Node = Node::Choice(AR_PARENT_COLS_CHOICES); + const AR_PARENT_NODES: &[Node] = &[ Node::Ident { source: IdentSource::Tables, @@ -394,25 +424,37 @@ const AR_PARENT_NODES: &[Node] = &[ writes_table: true, writes_column: false, writes_user_listed_column: false, - writes_table_alias: false, - writes_cte_name: false, - writes_projection_alias: false, + writes_table_alias: false, + writes_cte_name: false, + writes_projection_alias: false, }, Node::Punct('.'), - Node::Ident { - source: IdentSource::Columns, - role: "parent_column", - validator: None, - highlight_override: None, - writes_table: false, - writes_column: false, - writes_user_listed_column: false, + AR_PARENT_COLS, +]; +const AR_PARENT: Node = Node::Seq(AR_PARENT_NODES); + +const AR_CHILD_COL: Node = Node::Ident { + source: IdentSource::Columns, + role: "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 AR_PARENT: Node = Node::Seq(AR_PARENT_NODES); +}; +const AR_CHILD_COL_LIST: Node = Node::Repeated { + inner: &AR_CHILD_COL, + separator: Some(&Node::Punct(',')), + min: 1, +}; +const AR_CHILD_COLS_PAREN_NODES: &[Node] = + &[Node::Punct('('), AR_CHILD_COL_LIST, Node::Punct(')')]; +const AR_CHILD_COLS_PAREN: Node = Node::Seq(AR_CHILD_COLS_PAREN_NODES); +const AR_CHILD_COLS_CHOICES: &[Node] = &[AR_CHILD_COLS_PAREN, AR_CHILD_COL]; +const AR_CHILD_COLS: Node = Node::Choice(AR_CHILD_COLS_CHOICES); const AR_CHILD_NODES: &[Node] = &[ Node::Ident { @@ -423,23 +465,12 @@ const AR_CHILD_NODES: &[Node] = &[ writes_table: true, writes_column: false, writes_user_listed_column: false, - writes_table_alias: false, - writes_cte_name: false, - writes_projection_alias: false, + writes_table_alias: false, + writes_cte_name: false, + writes_projection_alias: false, }, Node::Punct('.'), - Node::Ident { - source: IdentSource::Columns, - role: "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, - }, + AR_CHILD_COLS, ]; const AR_CHILD: Node = Node::Seq(AR_CHILD_NODES); @@ -1523,8 +1554,10 @@ fn build_sql_create_table(path: &MatchedPath, source: &str) -> Result { + // Inline FK is single-column (the column it sits on); + // a compound FK uses the table-level form (ADR-0043 D4). 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)); + foreign_keys.push(consume_fk_reference(&mut items, None, vec![child_column])); } // Table-level `[constraint ] foreign key () // references [()] [on …]` (ADR-0035 §5, 4b). @@ -1532,11 +1565,21 @@ fn build_sql_create_table(path: &MatchedPath, source: &str) -> Result )` + // `( [, ]* )` — a compound + // FK lists multiple child columns (ADR-0043). 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()); + let mut child_columns = Vec::new(); + while let Some(it) = items.peek() { + match &it.kind { + MatchedKind::Punct(')') => break, + MatchedKind::Punct(',') => { + items.next(); + } + _ => child_columns.push(items.next().expect("peeked").text.clone()), + } + } if matches!(items.peek().map(|i| &i.kind), Some(MatchedKind::Punct(')'))) { items.next(); } @@ -1544,7 +1587,7 @@ fn build_sql_create_table(path: &MatchedPath, source: &str) -> Result( items: &mut std::iter::Peekable, name: Option, - child_column: String, + child_columns: Vec, ) -> 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(); // `)` - } - } + // Optional `( [, ]* )` — a + // compound FK references multiple parent columns (ADR-0043). + // `None` for the bare `REFERENCES ` form. + let parent_columns: Option> = + if matches!(items.peek().map(|i| &i.kind), Some(MatchedKind::Punct('('))) { + items.next(); // `(` + let mut cols = Vec::new(); + while let Some(it) = items.peek() { + match &it.kind { + MatchedKind::Punct(')') => break, + MatchedKind::Punct(',') => { + items.next(); + } + _ => cols.push(items.next().expect("peeked").text.clone()), + } + } + if matches!(items.peek().map(|i| &i.kind), Some(MatchedKind::Punct(')'))) { + items.next(); // `)` + } + Some(cols) + } else { + None + }; // `on ` clauses, in either order, 0..2. let mut on_delete = ReferentialAction::default_action(); let mut on_update = ReferentialAction::default_action(); @@ -1692,12 +1747,9 @@ where } SqlForeignKey { name, - // Single-column for now; the parenthesized multi-column parse - // (`FOREIGN KEY (a, b) REFERENCES P(x, y)`) lands with the - // grammar-node change (ADR-0043). - child_columns: vec![child_column], + child_columns, parent_table, - parent_columns: parent_column.map(|c| vec![c]), + parent_columns, on_delete, on_update, } @@ -2385,14 +2437,24 @@ fn build_alter_fk(path: &MatchedPath) -> SqlForeignKey { 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()); + // `( [, ]* )` — compound FK (ADR-0043). + let mut child_columns = Vec::new(); + while let Some(it) = items.peek() { + match &it.kind { + MatchedKind::Punct(')') => break, + MatchedKind::Punct(',') => { + items.next(); + } + _ => child_columns.push(items.next().expect("peeked").text.clone()), + } + } if matches!(items.peek().map(|i| &i.kind), Some(MatchedKind::Punct(')'))) { items.next(); } if matches!(items.peek().map(|i| &i.kind), Some(MatchedKind::Word("references"))) { items.next(); } - consume_fk_reference(&mut items, None, child_column) + consume_fk_reference(&mut items, None, child_columns) } pub static SQL_ALTER_TABLE: CommandNode = CommandNode { diff --git a/src/dsl/grammar/sql_create_table.rs b/src/dsl/grammar/sql_create_table.rs index 2db970d..1a09883 100644 --- a/src/dsl/grammar/sql_create_table.rs +++ b/src/dsl/grammar/sql_create_table.rs @@ -182,7 +182,15 @@ const FK_PARENT_COLUMN: Node = Node::Ident { writes_cte_name: false, writes_projection_alias: false, }; -static FK_PARENT_COL_NODES: &[Node] = &[Node::Punct('('), FK_PARENT_COLUMN, Node::Punct(')')]; +// `( a [, b]* )` — a compound FK references multiple parent columns +// (ADR-0043). The `Repeated` separator handles the commas; a +// single-column FK is the one-element case. +const FK_PARENT_COL_LIST: Node = Node::Repeated { + inner: &FK_PARENT_COLUMN, + separator: Some(&Node::Punct(',')), + min: 1, +}; +static FK_PARENT_COL_NODES: &[Node] = &[Node::Punct('('), FK_PARENT_COL_LIST, Node::Punct(')')]; const FK_PARENT_COL_OPT: Node = Node::Optional(&Node::Seq(FK_PARENT_COL_NODES)); // `REFERENCES [ ( ) ] [on delete/update …]` — the inline @@ -333,6 +341,13 @@ const FK_CHILD_COLUMN: Node = Node::Ident { writes_cte_name: false, writes_projection_alias: false, }; +// `( a [, b]* )` — a compound FK lists multiple child columns +// (ADR-0043); single-column is the one-element case. +const FK_CHILD_COL_LIST: Node = Node::Repeated { + inner: &FK_CHILD_COLUMN, + separator: Some(&Node::Punct(',')), + min: 1, +}; const FK_NAME: Node = Node::Ident { source: IdentSource::NewName, role: "fk_name", @@ -354,7 +369,7 @@ static FOREIGN_KEY_BODY_NODES: &[Node] = &[ Node::Word(Word::keyword("foreign")), Node::Word(Word::keyword("key")), Node::Punct('('), - FK_CHILD_COLUMN, + FK_CHILD_COL_LIST, Node::Punct(')'), Node::Word(Word::keyword("references")), FK_PARENT_TABLE, @@ -374,7 +389,7 @@ static TABLE_FK_NAMED_NODES: &[Node] = &[ Node::Word(Word::keyword("foreign")), Node::Word(Word::keyword("key")), Node::Punct('('), - FK_CHILD_COLUMN, + FK_CHILD_COL_LIST, Node::Punct(')'), Node::Word(Word::keyword("references")), FK_PARENT_TABLE, diff --git a/tests/it/compound_fk.rs b/tests/it/compound_fk.rs new file mode 100644 index 0000000..e72ce1e --- /dev/null +++ b/tests/it/compound_fk.rs @@ -0,0 +1,549 @@ +//! Integration tests for compound-primary-key foreign-key +//! references (T3 / ADR-0043) — the DSL `add 1:n relationship` +//! surface end to end. +//! +//! Covers: parse of the parenthesized multi-column endpoint; +//! worker round-trip (declare a 2-column FK, FK is enforced, +//! per-pair type-mismatch refused, arity mismatch refused); +//! persistence round-trip (`columns: [a, b]`); display; and undo. + +use rdbms_playground::db::Database; +use rdbms_playground::dsl::{ + parse_command, ColumnSpec, Command, ReferentialAction, SqlForeignKey, Type, Value, +}; +use rdbms_playground::persistence::Persistence; +use rdbms_playground::project; + +// ---- parse layer ------------------------------------------------ + +#[test] +fn parenthesized_compound_endpoint_parses_to_column_lists() { + let cmd = parse_command( + "add 1:n relationship from Parent.(a, b) to Child.(x, y)", + ) + .expect("parses"); + match cmd { + Command::AddRelationship { + parent_table, + parent_columns, + child_table, + child_columns, + .. + } => { + assert_eq!(parent_table, "Parent"); + assert_eq!(parent_columns, vec!["a".to_string(), "b".to_string()]); + assert_eq!(child_table, "Child"); + assert_eq!(child_columns, vec!["x".to_string(), "y".to_string()]); + } + other => panic!("expected AddRelationship, got {other:?}"), + } +} + +#[test] +fn single_column_endpoint_still_parses_unparenthesized() { + let cmd = parse_command("add 1:n relationship from Parent.id to Child.pid") + .expect("parses"); + match cmd { + Command::AddRelationship { + parent_columns, + child_columns, + .. + } => { + assert_eq!(parent_columns, vec!["id".to_string()]); + assert_eq!(child_columns, vec!["pid".to_string()]); + } + other => panic!("expected AddRelationship, got {other:?}"), + } +} + +// ---- SQL surface (advanced mode) -------------------------------- + +#[test] +fn sql_table_level_compound_fk_parses_to_lists() { + let cmd = parse_command( + "create table City (country int, region_code int, \ + foreign key (country, region_code) references Region(country, code))", + ) + .expect("parses"); + match cmd { + Command::SqlCreateTable { foreign_keys, .. } => { + assert_eq!(foreign_keys.len(), 1); + assert_eq!( + foreign_keys[0].child_columns, + vec!["country".to_string(), "region_code".to_string()], + ); + assert_eq!( + foreign_keys[0].parent_columns, + Some(vec!["country".to_string(), "code".to_string()]), + ); + } + other => panic!("expected SqlCreateTable, got {other:?}"), + } +} + +#[test] +fn sql_bare_compound_reference_parses_with_no_parent_columns() { + // `FOREIGN KEY (a, b) REFERENCES P` (no parent cols) — auto-expanded + // to the parent's full PK at execution (F-D). + let cmd = parse_command( + "create table City (country int, region_code int, \ + foreign key (country, region_code) references Region)", + ) + .expect("parses"); + match cmd { + Command::SqlCreateTable { foreign_keys, .. } => { + assert_eq!( + foreign_keys[0].child_columns, + vec!["country".to_string(), "region_code".to_string()], + ); + assert_eq!(foreign_keys[0].parent_columns, None); + } + other => panic!("expected SqlCreateTable, got {other:?}"), + } +} + +#[test] +fn sql_create_table_compound_fk_executes_and_enforces() { + let (_p, db, _dir) = open_project_db(); + let rt = rt(); + rt.block_on(async { + // Parent with a compound PK. + db.create_table( + "Region".to_string(), + vec![ + ColumnSpec::new("country", Type::Int), + ColumnSpec::new("code", Type::Int), + ], + vec!["country".to_string(), "code".to_string()], + None, + ) + .await + .expect("create Region"); + // Child via the SQL path with a multi-column FK referencing the + // full compound PK (resolve_create_table_fks). + db.sql_create_table( + "City".to_string(), + vec![ + ColumnSpec::new("country", Type::Int), + ColumnSpec::new("region_code", Type::Int), + ], + vec![], + vec![], + vec![], + vec![SqlForeignKey { + name: None, + child_columns: vec!["country".to_string(), "region_code".to_string()], + parent_table: "Region".to_string(), + parent_columns: Some(vec!["country".to_string(), "code".to_string()]), + on_delete: ReferentialAction::NoAction, + on_update: ReferentialAction::NoAction, + }], + false, + None, + ) + .await + .expect("create City with compound FK"); + + db.insert( + "Region".to_string(), + Some(vec!["country".to_string(), "code".to_string()]), + vec![Value::Number("1".to_string()), Value::Number("10".to_string())], + None, + ) + .await + .expect("insert parent"); + let bad = db + .insert( + "City".to_string(), + Some(vec!["country".to_string(), "region_code".to_string()]), + vec![Value::Number("9".to_string()), Value::Number("9".to_string())], + None, + ) + .await; + assert!(bad.is_err(), "compound FK violation refused by the engine"); + }); +} + +// ---- worker round-trip ------------------------------------------ + +fn rt() -> tokio::runtime::Runtime { + tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .expect("tokio rt") +} + +fn open_project_db() -> (project::Project, Database, tempfile::TempDir) { + let dir = tempfile::tempdir().expect("create tempdir"); + let project = + project::open_or_create(None, Some(dir.path())).expect("open or create project"); + let persistence = Persistence::new(project.path().to_path_buf()); + let db = Database::open_with_persistence(project.db_path(), persistence) + .expect("open db with persistence"); + (project, db, dir) +} + +/// `Region(country int, code int)` compound PK + `City(country int, +/// region_code int, name text)` — the child FK columns matching the +/// parent PK by type (int → int). +async fn seed_compound(db: &Database) { + db.create_table( + "Region".to_string(), + vec![ + ColumnSpec::new("country", Type::Int), + ColumnSpec::new("code", Type::Int), + ], + vec!["country".to_string(), "code".to_string()], + None, + ) + .await + .expect("create Region"); + db.create_table( + "City".to_string(), + vec![ + ColumnSpec::new("country", Type::Int), + ColumnSpec::new("region_code", Type::Int), + ColumnSpec::new("name", Type::Text), + ], + vec!["country".to_string()], + None, + ) + .await + .expect("create City"); +} + +#[test] +fn compound_fk_declares_enforces_and_round_trips() { + let (_p, db, _dir) = open_project_db(); + let rt = rt(); + rt.block_on(async { + seed_compound(&db).await; + // Declare the compound FK: City.(country, region_code) → + // Region.(country, code). + db.add_relationship( + Some("city_region".to_string()), + "Region".to_string(), + vec!["country".to_string(), "code".to_string()], + "City".to_string(), + vec!["country".to_string(), "region_code".to_string()], + ReferentialAction::NoAction, + ReferentialAction::NoAction, + false, + None, + ) + .await + .expect("add compound relationship"); + + // The FK is enforced: a parent row exists for (1, 10); a + // child referencing it inserts, one referencing (9, 9) is + // refused by the engine. + db.insert( + "Region".to_string(), + Some(vec!["country".to_string(), "code".to_string()]), + vec![Value::Number("1".to_string()), Value::Number("10".to_string())], + None, + ) + .await + .expect("insert parent row"); + db.insert( + "City".to_string(), + Some(vec![ + "country".to_string(), + "region_code".to_string(), + "name".to_string(), + ]), + vec![Value::Number("1".to_string()), Value::Number("10".to_string()), Value::Text("Metropolis".to_string())], + None, + ) + .await + .expect("child row referencing an existing parent key inserts"); + let violation = db + .insert( + "City".to_string(), + Some(vec![ + "country".to_string(), + "region_code".to_string(), + "name".to_string(), + ]), + vec![Value::Number("9".to_string()), Value::Number("9".to_string()), Value::Text("Nowhere".to_string())], + None, + ) + .await; + assert!( + violation.is_err(), + "a child row with no matching compound parent key must be refused", + ); + + // describe shows the compound endpoints symmetrically. + let city = db.describe_table("City".to_string(), None).await.unwrap(); + let outbound = &city.outbound_relationships[0]; + assert_eq!( + outbound.local_columns, + vec!["country".to_string(), "region_code".to_string()], + ); + assert_eq!( + outbound.other_columns, + vec!["country".to_string(), "code".to_string()], + ); + }); +} + +#[test] +fn compound_fk_create_fk_makes_both_child_columns() { + let (_p, db, _dir) = open_project_db(); + let rt = rt(); + rt.block_on(async { + // Region(country, code) compound PK; City has neither FK column. + db.create_table( + "Region".to_string(), + vec![ + ColumnSpec::new("country", Type::Int), + ColumnSpec::new("code", Type::Int), + ], + vec!["country".to_string(), "code".to_string()], + None, + ) + .await + .expect("create Region"); + db.create_table( + "City".to_string(), + vec![ColumnSpec::new("name", Type::Text)], + vec![], + None, + ) + .await + .expect("create City"); + // --create-fk creates both missing child columns, typed to the + // matching parent PK columns' fk_target_type (int → int). + db.add_relationship( + None, + "Region".to_string(), + vec!["country".to_string(), "code".to_string()], + "City".to_string(), + vec!["c_country".to_string(), "c_code".to_string()], + ReferentialAction::NoAction, + ReferentialAction::NoAction, + true, + None, + ) + .await + .expect("add compound relationship with --create-fk"); + let city = db.describe_table("City".to_string(), None).await.unwrap(); + for col in ["c_country", "c_code"] { + assert!( + city.columns.iter().any(|c| c.name == col), + "--create-fk created `{col}`: {:?}", + city.columns.iter().map(|c| &c.name).collect::>(), + ); + } + }); +} + +#[test] +fn compound_fk_arity_mismatch_is_refused() { + let (_p, db, _dir) = open_project_db(); + let rt = rt(); + rt.block_on(async { + seed_compound(&db).await; + // Two parent columns, one child column → arity mismatch. + let err = db + .add_relationship( + None, + "Region".to_string(), + vec!["country".to_string(), "code".to_string()], + "City".to_string(), + vec!["country".to_string()], + ReferentialAction::NoAction, + ReferentialAction::NoAction, + false, + None, + ) + .await; + assert!(err.is_err(), "mismatched child/parent arity must be refused"); + }); +} + +#[test] +fn compound_fk_type_mismatch_per_pair_is_refused() { + let (_p, db, _dir) = open_project_db(); + let rt = rt(); + rt.block_on(async { + db.create_table( + "Region".to_string(), + vec![ + ColumnSpec::new("country", Type::Int), + ColumnSpec::new("code", Type::Int), + ], + vec!["country".to_string(), "code".to_string()], + None, + ) + .await + .expect("create Region"); + // `bad` is `text` — incompatible with the `int` PK column it + // would pair with (per-pair type-compat, ADR-0011). + db.create_table( + "City".to_string(), + vec![ + ColumnSpec::new("country", Type::Int), + ColumnSpec::new("bad", Type::Text), + ], + vec![], + None, + ) + .await + .expect("create City"); + let err = db + .add_relationship( + None, + "Region".to_string(), + vec!["country".to_string(), "code".to_string()], + "City".to_string(), + vec!["country".to_string(), "bad".to_string()], + ReferentialAction::NoAction, + ReferentialAction::NoAction, + false, + None, + ) + .await; + assert!(err.is_err(), "a type-incompatible column pair must be refused"); + }); +} + +#[test] +fn compound_fk_survives_rebuild_from_text() { + // The riskiest round-trip: comma-encoded metadata + yaml + // `columns: [a, b]` → rebuild reconstructs the compound FK DDL. + let dir = tempfile::tempdir().expect("tempdir"); + let project = project::open_or_create(None, Some(dir.path())).expect("open project"); + let path = project.path().to_path_buf(); + let rt = rt(); + { + let db = Database::open_with_persistence( + project.db_path(), + Persistence::new(path.clone()), + ) + .expect("open db"); + rt.block_on(async { + seed_compound(&db).await; + db.add_relationship( + Some("city_region".to_string()), + "Region".to_string(), + vec!["country".to_string(), "code".to_string()], + "City".to_string(), + vec!["country".to_string(), "region_code".to_string()], + ReferentialAction::NoAction, + ReferentialAction::NoAction, + false, + Some("add 1:n relationship".to_string()), + ) + .await + .expect("add compound relationship"); + }); + } + // Reopen and rebuild the database purely from the persisted + // project.yaml + data/. + let db = Database::open_with_persistence(project.db_path(), Persistence::new(path.clone())) + .expect("reopen db"); + rt.block_on(async { + db.rebuild_from_text(path.clone(), None) + .await + .expect("rebuild from text"); + // The compound FK is reconstructed and still enforced. + db.insert( + "Region".to_string(), + Some(vec!["country".to_string(), "code".to_string()]), + vec![Value::Number("1".to_string()), Value::Number("10".to_string())], + None, + ) + .await + .expect("insert parent after rebuild"); + let bad = db + .insert( + "City".to_string(), + Some(vec!["country".to_string(), "region_code".to_string()]), + vec![Value::Number("9".to_string()), Value::Number("9".to_string())], + None, + ) + .await; + assert!(bad.is_err(), "compound FK still enforced after rebuild from text"); + // Endpoints survived the round-trip intact. + let city = db.describe_table("City".to_string(), None).await.unwrap(); + assert_eq!( + city.outbound_relationships[0].other_columns, + vec!["country".to_string(), "code".to_string()], + ); + }); +} + +#[test] +fn compound_fk_undo_removes_the_relationship() { + let dir = tempfile::tempdir().expect("tempdir"); + let project = project::open_or_create(None, Some(dir.path())).expect("open project"); + let db = Database::open_with_persistence_and_undo( + project.db_path(), + Persistence::new(project.path().to_path_buf()), + true, + ) + .expect("open db with undo"); + let rt = rt(); + rt.block_on(async { + seed_compound(&db).await; + db.add_relationship( + Some("city_region".to_string()), + "Region".to_string(), + vec!["country".to_string(), "code".to_string()], + "City".to_string(), + vec!["country".to_string(), "region_code".to_string()], + ReferentialAction::NoAction, + ReferentialAction::NoAction, + false, + // A user-command source records one undo snapshot. + Some("add 1:n relationship".to_string()), + ) + .await + .expect("add compound relationship"); + assert_eq!( + db.describe_table("City".to_string(), None) + .await + .unwrap() + .outbound_relationships + .len(), + 1, + ); + // One undo step removes the whole relationship (ADR-0013/0006). + db.undo().await.unwrap().expect("undo applied"); + assert!( + db.describe_table("City".to_string(), None) + .await + .unwrap() + .outbound_relationships + .is_empty(), + "undo removed the compound relationship in one step", + ); + }); +} + +#[test] +fn compound_fk_partial_pk_reference_is_refused() { + let (_p, db, _dir) = open_project_db(); + let rt = rt(); + rt.block_on(async { + seed_compound(&db).await; + // Referencing only one column of Region's 2-column PK (F-A: + // must reference the full PK). + let err = db + .add_relationship( + None, + "Region".to_string(), + vec!["country".to_string()], + "City".to_string(), + vec!["country".to_string()], + ReferentialAction::NoAction, + ReferentialAction::NoAction, + false, + None, + ) + .await; + assert!(err.is_err(), "a partial-PK reference must be refused (F-A)"); + }); +} diff --git a/tests/it/main.rs b/tests/it/main.rs index ba3384f..f2e9a57 100644 --- a/tests/it/main.rs +++ b/tests/it/main.rs @@ -9,6 +9,7 @@ mod case_insensitive_names; mod column_op_guards; +mod compound_fk; mod engine_vocabulary_audit; mod friendly_enrichment; mod help_command; diff --git a/tests/typing_surface/snapshots/typing_surface_matrix__typing_surface__add_relationship__after_child_table_dot_narrows_to_child_columns@after_child_dot.snap b/tests/typing_surface/snapshots/typing_surface_matrix__typing_surface__add_relationship__after_child_table_dot_narrows_to_child_columns@after_child_dot.snap index a148fd6..200a828 100644 --- a/tests/typing_surface/snapshots/typing_surface_matrix__typing_surface__add_relationship__after_child_table_dot_narrows_to_child_columns@after_child_dot.snap +++ b/tests/typing_surface/snapshots/typing_surface_matrix__typing_surface__add_relationship__after_child_table_dot_narrows_to_child_columns@after_child_dot.snap @@ -1,5 +1,6 @@ --- source: tests/typing_surface/add_relationship.rs +assertion_line: 64 description: "input=\"add 1:n relationship from Customers.id to Orders.\" cursor=49" expression: "& a" --- @@ -25,6 +26,11 @@ Assessment { kind: Identifier, mode: Both, }, + Candidate { + text: "(", + kind: Punct, + mode: Both, + }, ], selected: None, }, @@ -52,6 +58,11 @@ Assessment { kind: Identifier, mode: Both, }, + Candidate { + text: "(", + kind: Punct, + mode: Both, + }, ], }, ), diff --git a/tests/typing_surface/snapshots/typing_surface_matrix__typing_surface__add_relationship__after_parent_table_dot_narrows_to_parent_columns@after_parent_dot.snap b/tests/typing_surface/snapshots/typing_surface_matrix__typing_surface__add_relationship__after_parent_table_dot_narrows_to_parent_columns@after_parent_dot.snap index 0165d55..357eef6 100644 --- a/tests/typing_surface/snapshots/typing_surface_matrix__typing_surface__add_relationship__after_parent_table_dot_narrows_to_parent_columns@after_parent_dot.snap +++ b/tests/typing_surface/snapshots/typing_surface_matrix__typing_surface__add_relationship__after_parent_table_dot_narrows_to_parent_columns@after_parent_dot.snap @@ -1,5 +1,6 @@ --- source: tests/typing_surface/add_relationship.rs +assertion_line: 51 description: "input=\"add 1:n relationship from Customers.\" cursor=36" expression: "& a" --- @@ -20,6 +21,11 @@ Assessment { kind: Identifier, mode: Both, }, + Candidate { + text: "(", + kind: Punct, + mode: Both, + }, ], selected: None, }, @@ -42,6 +48,11 @@ Assessment { kind: Identifier, mode: Both, }, + Candidate { + text: "(", + kind: Punct, + mode: Both, + }, ], }, ),