feat: ADR-0035 4i(a,b) — CREATE TABLE help/usage + describe table constraints; Phase 4 complete
(b) describe shows table-level constraints: TableDescription gains unique_constraints + check_constraints (populated by do_describe_table from read_schema), rendered in a new "Table constraints:" section — composite UNIQUE and table-level CHECK (named + unnamed). The per-column Constraints column already covered single-column NOT NULL/UNIQUE/PK/CHECK. (a) CREATE TABLE help/usage skeleton refreshed for the column DEFAULT/ CHECK/REFERENCES, table-level composite UNIQUE, table CHECK, and table-level FOREIGN KEY forms (4a.2/4a.3/4b) — engine-neutral, vocab-audit clean. With 4i's (c)/(d)/(e) already shipped, this completes sub-phase 4i — the verification sweep — and therefore ADR-0035 Phase 4 (4a–4i). ADR-0035 Status, §13 4i, the ADR index, and requirements.md Q1 updated to "Phase 4 complete". Tests: render_structure table-level-constraints unit test + e2e_describe_shows_table_level_constraints. Full suite 1917 passing / 0 failing / 1 ignored; clippy clean.
This commit is contained in:
claude@clouddev1
@@ -4,17 +4,20 @@
|
||||
|
||||
Accepted. Design agreed with the user (2026-05-24); the approach is
|
||||
**validated end-to-end by sub-phases 4a / 4a.2 / 4a.3 / 4b / 4c / 4d /
|
||||
4e / 4f / 4g / 4h** (`CREATE TABLE` with column- and table-level
|
||||
4e / 4f / 4g / 4h / 4i** (`CREATE TABLE` with column- and table-level
|
||||
constraints and foreign keys, `DROP TABLE [IF EXISTS]`,
|
||||
`CREATE [UNIQUE] INDEX` / `DROP INDEX [IF EXISTS]`, `ALTER TABLE`
|
||||
add/drop/rename column, `ALTER TABLE … ALTER COLUMN TYPE`, `ALTER TABLE`
|
||||
add/drop constraint + add foreign key, and `ALTER TABLE … RENAME TO`,
|
||||
implemented 2026-05-25/26 — plans
|
||||
add/drop constraint + add foreign key, `ALTER TABLE … RENAME TO`, and the
|
||||
4i verification sweep — completion merge, simple/advanced completion
|
||||
colour, describe of table-level constraints, self-ref FK indicator, and
|
||||
the CREATE-TABLE help/usage refresh — implemented 2026-05-25/26 — plans
|
||||
`docs/plans/20260524-adr-0035-sql-ddl-4a.md`, `…-4a2.md`, `…-4a3.md`,
|
||||
`docs/plans/20260525-adr-0035-sql-ddl-4b.md`, `…-4c.md`, `…-4d.md`,
|
||||
`…-4e.md`, `…-4f.md`, `…-4g.md`,
|
||||
`docs/plans/20260526-adr-0035-sql-ddl-4h.md`), so the decision is accepted
|
||||
while the remaining sub-phase (**4i**, §13) continues. This is **Phase 4** of the ADR-0030 roadmap (the
|
||||
`docs/plans/20260526-adr-0035-sql-ddl-4h.md`,
|
||||
`docs/plans/20260526-adr-0035-sql-ddl-4i.md`). **Phase 4 is complete**
|
||||
(4a–4i all shipped). 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.
|
||||
@@ -515,41 +518,38 @@ ADR-0033's structure:
|
||||
update; ADR-0035 §6 scope — user-confirmed; documented collision
|
||||
caveat). One undo step (the whole-project snapshot). Advanced-mode only;
|
||||
closes the rename half of `C1`.
|
||||
- **4i — Verification sweep.** Typing-surface + matrix coverage,
|
||||
engine-neutral error pass, undo-parity check (one step per
|
||||
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) — **4d's index forms already carry their
|
||||
own help/usage** (`ddl.sql_create_index` / `ddl.sql_drop_index` + the
|
||||
`parse.usage.*` keys), since the nodes are new; (b) `describe` display
|
||||
of table-level constraints (composite `UNIQUE` + table `CHECK`) — note
|
||||
the **unique-*index* marker shipped in 4d** (`[unique]` in the
|
||||
structure view + items panel), so only the table-level *constraint*
|
||||
display remains here; (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. (d) **4c
|
||||
shared-entry-word completion merge** — in advanced mode a shared entry
|
||||
word surfaces only the SQL node's continuations, so `drop ` offers
|
||||
only `table` (not the DSL `column`/`relationship`/`index`/`constraint`)
|
||||
and a partial keyword like `drop rel` returns an *empty* list (a
|
||||
mid-word dead end), even though the DSL drops still parse + execute via
|
||||
fallback. Merge the expected sets of all candidate nodes for a shared
|
||||
entry word so advanced completion offers every valid continuation
|
||||
(`drop ` → table + column + relationship + index + constraint; `drop
|
||||
rel` → relationship); verify `create`/`insert`/`update`/`delete`
|
||||
completion stays sensible. **4d widened this:** `create` and `drop`
|
||||
now each have *two* advanced nodes (table + index), so a shared entry
|
||||
word's continuations now span two SQL shapes as well as the DSL ones —
|
||||
the merge matters more. (e) **Discussion flag (user, 2026-05-25):**
|
||||
before/with (d), discuss **visually distinguishing simple- vs
|
||||
advanced-mode completions in the hint UI (likely by colour)** so a
|
||||
learner can see which continuations are DSL and which are SQL — a UX
|
||||
design conversation, not just the mechanical merge.
|
||||
- **4i — Verification sweep (completes Phase 4).** *(Implemented
|
||||
2026-05-26 — plan `docs/plans/20260526-adr-0035-sql-ddl-4i.md`.)*
|
||||
- **(a) `CREATE TABLE` help/usage skeleton** refreshed for the 4a.2
|
||||
`DEFAULT`/`CHECK`/composite-`UNIQUE`, 4a.3 table-`CHECK`, and 4b FK
|
||||
forms (the index forms already carried their own since 4d).
|
||||
- **(b) `describe` of table-level constraints** — `TableDescription`
|
||||
gained `unique_constraints` + `check_constraints`, rendered in a
|
||||
"Table constraints:" section (composite `UNIQUE`, table `CHECK`
|
||||
incl. **named** CHECKs). The per-column `[unique]`-index marker
|
||||
shipped in 4d.
|
||||
- **(c) self-ref FK indicator** — `schema_existence_diagnostics`
|
||||
collects the `CREATE TABLE` target(s) (`IdentSource::NewName`, role
|
||||
`table_name`) and exempts a `Tables` reference matching one from the
|
||||
unknown-table flag, so a self-referencing FK no longer pre-flags the
|
||||
not-yet-created table; a FK to a genuinely-unknown *other* table
|
||||
still flags.
|
||||
- **(d) shared-entry-word completion merge** — at the advanced-mode
|
||||
entry-word boundary, `completion_probe_in_mode` walks every candidate
|
||||
node and unions the viable (`Incomplete`) ones' continuations, so
|
||||
`drop ` offers `table·index·column·relationship·constraint` and `drop
|
||||
rel` → `relationship` (was an empty dead-end). Completion-only (the
|
||||
parse path is untouched); deeper positions keep the committed walk.
|
||||
- **(e) simple-vs-advanced completion colour** — each continuation is
|
||||
classified `Both`/`Advanced`/`Simple` and, **only when the candidate
|
||||
list mixes modes**, coloured (`mode_advanced`/`mode_simple`, `Both` =
|
||||
token-kind) and block-ordered `Both → Advanced → Simple` (user-chosen
|
||||
design, 2026-05-26). Single-mode lists keep the token-kind colours.
|
||||
- **Staples:** matrix/typing-surface coverage extended (completion +
|
||||
describe tests); engine-neutral wording held (the vocab audit covers
|
||||
the new strings); undo-parity is **N/A for 4i** — every change is
|
||||
read-side (completion / diagnostics / describe / help), so no undo
|
||||
steps are introduced.
|
||||
|
||||
## Consequences
|
||||
|
||||
|
||||
+1
-1
File diff suppressed because one or more lines are too long
@@ -256,8 +256,10 @@ handoff-14 cleanup; 449 after B2/C2.)
|
||||
CHECK text that qualifies a column with the old table name** so a fresh
|
||||
rebuild round-trips; refuses same-name / existing-target / `__rdbms_*` /
|
||||
non-existent; auto-named indexes + relationships kept stale per §6
|
||||
scope; one undo step). Remaining: the 4i verification sweep per
|
||||
ADR-0035 §13.)*
|
||||
scope; one undo step), then the **4i verification sweep** (shared-entry-
|
||||
word completion merge + simple/advanced completion colour; `describe` of
|
||||
table-level constraints; self-ref FK pre-submit indicator; CREATE-TABLE
|
||||
help/usage refresh). **ADR-0035 Phase 4 (4a–4i) is complete.**)*
|
||||
- [ ] **Q2** Non-standard syntax rejected with a clear message
|
||||
pointing at the supported subset.
|
||||
*(Design done — ADR-0030 §8: out-of-subset statements are
|
||||
|
||||
@@ -2596,6 +2596,8 @@ mod tests {
|
||||
outbound_relationships: Vec::new(),
|
||||
inbound_relationships: Vec::new(),
|
||||
indexes: Vec::new(),
|
||||
unique_constraints: Vec::new(),
|
||||
check_constraints: Vec::new(),
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -70,6 +70,13 @@ pub struct TableDescription {
|
||||
pub inbound_relationships: Vec<RelationshipEnd>,
|
||||
/// User-created indexes on this table (ADR-0025).
|
||||
pub indexes: Vec<IndexInfo>,
|
||||
/// Table-level composite `UNIQUE (a, b, …)` constraints (ADR-0035
|
||||
/// §4a.2). Single-column UNIQUE rides on the column itself
|
||||
/// (`ColumnDescription::unique`); these are the multi-column ones.
|
||||
pub unique_constraints: Vec<Vec<String>>,
|
||||
/// Table-level `CHECK (…)` constraints with their optional name
|
||||
/// (ADR-0035 §4a.3 / §4g), in declaration order.
|
||||
pub check_constraints: Vec<crate::persistence::TableCheck>,
|
||||
}
|
||||
|
||||
/// One user-created index on a table (ADR-0025).
|
||||
@@ -7390,6 +7397,8 @@ fn do_describe_table(conn: &Connection, name: &str) -> Result<TableDescription,
|
||||
outbound_relationships,
|
||||
inbound_relationships,
|
||||
indexes,
|
||||
unique_constraints: schema.unique_constraints.clone(),
|
||||
check_constraints: schema.check_constraints,
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -261,8 +261,11 @@ help:
|
||||
create: |-
|
||||
create table <T> with pk [<col>(<type>), ...] — create a table
|
||||
sql_create_table: |-
|
||||
create table [if not exists] <T> (<col> <type> [not null] [unique] [primary key], ...
|
||||
[, primary key (<col>, ...)]) — create a table (advanced SQL)
|
||||
create table [if not exists] <T> (
|
||||
<col> <type> [not null] [unique] [primary key] [default <expr>] [check (<expr>)] [references <P>[(<col>)]], ...
|
||||
[, primary key (<col>, ...)] [, unique (<col>, ...)] [, check (<expr>)]
|
||||
[, [constraint <name>] foreign key (<col>) references <P>[(<col>)]])
|
||||
— create a table (advanced SQL)
|
||||
sql_drop_table: |-
|
||||
drop table [if exists] <T> — remove a table (advanced SQL)
|
||||
sql_create_index: |-
|
||||
@@ -475,7 +478,7 @@ parse:
|
||||
# placeholders. ADR-0009's surface conventions apply.
|
||||
usage:
|
||||
create_table: "create table <Name> with pk [<col>(<type>)[, ...]]"
|
||||
sql_create_table: "create table [if not exists] <Name> (<col> <type> [not null] [unique] [primary key], ... [, primary key (<col>, ...)])"
|
||||
sql_create_table: "create table [if not exists] <Name> (<col> <type> [not null] [unique] [primary key] [default <expr>] [check (<expr>)] [references <Parent>[(<col>)]], ... [, primary key (<col>, ...)] [, unique (<col>, ...)] [, check (<expr>)] [, [constraint <name>] foreign key (<col>) references <Parent>[(<col>)]])"
|
||||
sql_drop_table: "drop table [if exists] <Name>"
|
||||
sql_create_index: "create [unique] index [if not exists] [<Name>] on <Table> (<col>[, ...])"
|
||||
sql_drop_index: "drop index [if exists] <Name>"
|
||||
|
||||
@@ -152,6 +152,24 @@ pub fn render_structure(desc: &TableDescription) -> Vec<String> {
|
||||
}
|
||||
}
|
||||
|
||||
// Table-level constraints (ADR-0035 §4i b): composite `UNIQUE (a, b)`
|
||||
// and table `CHECK (…)` constraints. Single-column UNIQUE / NOT NULL /
|
||||
// PK / column-level CHECK already show in the per-column "Constraints"
|
||||
// column above; this section is the table-level constraints that span
|
||||
// columns or stand alone. A named CHECK shows its name.
|
||||
if !desc.unique_constraints.is_empty() || !desc.check_constraints.is_empty() {
|
||||
out.push("Table constraints:".to_string());
|
||||
for cols in &desc.unique_constraints {
|
||||
out.push(format!(" unique ({})", cols.join(", ")));
|
||||
}
|
||||
for chk in &desc.check_constraints {
|
||||
match &chk.name {
|
||||
Some(name) => out.push(format!(" check {name} ({})", chk.expr)),
|
||||
None => out.push(format!(" check ({})", chk.expr)),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
out
|
||||
}
|
||||
|
||||
@@ -730,6 +748,8 @@ mod tests {
|
||||
outbound_relationships: Vec::new(),
|
||||
inbound_relationships: Vec::new(),
|
||||
indexes: Vec::new(),
|
||||
unique_constraints: Vec::new(),
|
||||
check_constraints: Vec::new(),
|
||||
};
|
||||
assert_snapshot!(render_structure(&desc).join("\n"));
|
||||
}
|
||||
@@ -749,6 +769,8 @@ mod tests {
|
||||
on_update: ReferentialAction::NoAction,
|
||||
}],
|
||||
indexes: Vec::new(),
|
||||
unique_constraints: Vec::new(),
|
||||
check_constraints: Vec::new(),
|
||||
};
|
||||
let out = render_structure(&desc).join("\n");
|
||||
assert!(
|
||||
@@ -774,6 +796,8 @@ mod tests {
|
||||
outbound_relationships: Vec::new(),
|
||||
inbound_relationships: Vec::new(),
|
||||
indexes: Vec::new(),
|
||||
unique_constraints: Vec::new(),
|
||||
check_constraints: Vec::new(),
|
||||
};
|
||||
let out = render_structure(&desc).join("\n");
|
||||
// PK appears for id, NOT NULL for name, blank for nick.
|
||||
@@ -796,6 +820,8 @@ mod tests {
|
||||
columns: vec!["Email".to_string()],
|
||||
unique: false,
|
||||
}],
|
||||
unique_constraints: Vec::new(),
|
||||
check_constraints: Vec::new(),
|
||||
};
|
||||
let out = render_structure(&desc).join("\n");
|
||||
assert!(out.contains("Indexes:"), "got:\n{out}");
|
||||
@@ -821,11 +847,43 @@ mod tests {
|
||||
columns: vec!["Email".to_string()],
|
||||
unique: true,
|
||||
}],
|
||||
unique_constraints: Vec::new(),
|
||||
check_constraints: Vec::new(),
|
||||
};
|
||||
let out = render_structure(&desc).join("\n");
|
||||
assert!(out.contains("uidx_email (Email) [unique]"), "got:\n{out}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn render_structure_shows_table_level_constraints() {
|
||||
// ADR-0035 §4i (b): composite UNIQUE and table-level CHECK
|
||||
// (named + unnamed) render in a "Table constraints:" section,
|
||||
// distinct from the per-column "Constraints" column.
|
||||
let desc = TableDescription {
|
||||
name: "T".to_string(),
|
||||
columns: vec![
|
||||
col("a", Type::Int, true, false),
|
||||
col("b", Type::Int, false, false),
|
||||
],
|
||||
outbound_relationships: Vec::new(),
|
||||
inbound_relationships: Vec::new(),
|
||||
indexes: Vec::new(),
|
||||
unique_constraints: vec![vec!["a".to_string(), "b".to_string()]],
|
||||
check_constraints: vec![
|
||||
crate::persistence::TableCheck { name: None, expr: "a < b".to_string() },
|
||||
crate::persistence::TableCheck {
|
||||
name: Some("a_lt_b".to_string()),
|
||||
expr: "a <> b".to_string(),
|
||||
},
|
||||
],
|
||||
};
|
||||
let out = render_structure(&desc).join("\n");
|
||||
assert!(out.contains("Table constraints:"), "got:\n{out}");
|
||||
assert!(out.contains("unique (a, b)"), "got:\n{out}");
|
||||
assert!(out.contains("check (a < b)"), "unnamed check; got:\n{out}");
|
||||
assert!(out.contains("check a_lt_b (a <> b)"), "named check shows its name; got:\n{out}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn render_structure_omits_indexes_section_when_none() {
|
||||
let desc = TableDescription {
|
||||
@@ -834,6 +892,8 @@ mod tests {
|
||||
outbound_relationships: Vec::new(),
|
||||
inbound_relationships: Vec::new(),
|
||||
indexes: Vec::new(),
|
||||
unique_constraints: Vec::new(),
|
||||
check_constraints: Vec::new(),
|
||||
};
|
||||
let out = render_structure(&desc).join("\n");
|
||||
assert!(!out.contains("Indexes:"), "got:\n{out}");
|
||||
@@ -1026,6 +1086,8 @@ mod tests {
|
||||
outbound_relationships: Vec::new(),
|
||||
inbound_relationships: Vec::new(),
|
||||
indexes: Vec::new(),
|
||||
unique_constraints: Vec::new(),
|
||||
check_constraints: Vec::new(),
|
||||
};
|
||||
let out = render_structure(&desc).join("\n");
|
||||
// The lowercase form of the SQLite type should appear.
|
||||
|
||||
@@ -1310,6 +1310,8 @@ mod tests {
|
||||
outbound_relationships: Vec::new(),
|
||||
inbound_relationships: Vec::new(),
|
||||
indexes: Vec::new(),
|
||||
unique_constraints: Vec::new(),
|
||||
check_constraints: Vec::new(),
|
||||
};
|
||||
app.current_table = Some(desc);
|
||||
// Mirror what the App writes when a DSL command succeeds.
|
||||
|
||||
@@ -653,6 +653,50 @@ fn e2e_named_check_metadata_survives_a_fresh_rebuild() {
|
||||
.expect("DROP CONSTRAINT after a fresh rebuild — the CHECK metadata was reconstructed");
|
||||
}
|
||||
|
||||
// --- 4i (b): describe shows table-level constraints ---------------------
|
||||
|
||||
#[test]
|
||||
fn e2e_describe_shows_table_level_constraints() {
|
||||
// ADR-0035 §4i (b): `describe` surfaces composite UNIQUE and
|
||||
// table-level CHECK constraints (named + unnamed) — the executor
|
||||
// populates them on TableDescription from the metadata.
|
||||
let (project, db, _d) = open();
|
||||
let r = rt();
|
||||
std::fs::write(
|
||||
project.path().join("d.commands"),
|
||||
"create table T (a integer primary key, b integer, unique (a, b), check (a < b))\n\
|
||||
alter table T add constraint a_ne_b check (a <> b)\n",
|
||||
)
|
||||
.expect("write script");
|
||||
let events = r.block_on(run_replay(&db, project.path(), "d.commands"));
|
||||
assert!(
|
||||
matches!(events.last(), Some(AppEvent::ReplayCompleted { .. })),
|
||||
"events: {events:?}"
|
||||
);
|
||||
|
||||
let desc = r.block_on(db.describe_table("T".to_string(), None)).expect("describe");
|
||||
assert_eq!(
|
||||
desc.unique_constraints,
|
||||
vec![vec!["a".to_string(), "b".to_string()]],
|
||||
"composite UNIQUE surfaced"
|
||||
);
|
||||
let checks: Vec<(Option<String>, String)> = desc
|
||||
.check_constraints
|
||||
.iter()
|
||||
.map(|c| (c.name.clone(), c.expr.clone()))
|
||||
.collect();
|
||||
assert!(
|
||||
checks.iter().any(|(n, e)| n.is_none() && e.contains("a < b")),
|
||||
"unnamed table CHECK surfaced: {checks:?}"
|
||||
);
|
||||
assert!(
|
||||
checks
|
||||
.iter()
|
||||
.any(|(n, e)| n.as_deref() == Some("a_ne_b") && e.contains("a <> b")),
|
||||
"named table CHECK surfaced with its name: {checks:?}"
|
||||
);
|
||||
}
|
||||
|
||||
// --- 4h: ALTER TABLE … RENAME TO (ADR-0035 §6) --------------------------
|
||||
|
||||
/// Path to a table's CSV in the project data dir.
|
||||
|
||||
@@ -269,6 +269,8 @@ fn fake_table(name: &str, columns: &[(&str, Type, bool)]) -> TableDescription {
|
||||
outbound_relationships: Vec::new(),
|
||||
inbound_relationships: Vec::new(),
|
||||
indexes: Vec::new(),
|
||||
unique_constraints: Vec::new(),
|
||||
check_constraints: Vec::new(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -443,6 +445,8 @@ fn add_relationship_flow_shows_parent_side_with_inbound_section() {
|
||||
on_update: ReferentialAction::NoAction,
|
||||
}],
|
||||
indexes: Vec::new(),
|
||||
unique_constraints: Vec::new(),
|
||||
check_constraints: Vec::new(),
|
||||
};
|
||||
app.update(AppEvent::DslSucceeded {
|
||||
command: Command::AddRelationship {
|
||||
@@ -495,6 +499,8 @@ fn add_relationship_flow_shows_inbound_section_on_parent() {
|
||||
on_update: ReferentialAction::NoAction,
|
||||
}],
|
||||
indexes: Vec::new(),
|
||||
unique_constraints: Vec::new(),
|
||||
check_constraints: Vec::new(),
|
||||
};
|
||||
app.update(AppEvent::DslSucceeded {
|
||||
command: Command::AddColumn {
|
||||
|
||||
Reference in New Issue
Block a user