rdbms-playground/docs/plans/20260525-adr-0035-sql-ddl-4g.md

Plan: ADR-0035 Phase 4, sub-phase 4g — ALTER TABLE add/drop constraint + add FK

Add the advanced-mode SQL forms:

• ALTER TABLE <T> ADD [CONSTRAINT <name>] CHECK (<expr>) — table-level CHECK, named or unnamed.
• ALTER TABLE <T> ADD UNIQUE (<col>, …) — composite UNIQUE (unnamed; see §2.4).
• ALTER TABLE <T> ADD [CONSTRAINT <name>] FOREIGN KEY (<col>) REFERENCES <P>[(<col>)] [ON DELETE …] [ON UPDATE …] — a relationship.
• ALTER TABLE <T> DROP CONSTRAINT <name> — drop a named CHECK or a named FK (relationship).

Plus: fold the internal-__rdbms_* guard into do_add_constraint / do_add_relationship (the remaining executors of the 4d/4e/4f guard class).

User-confirmed scope (2026-05-25): full 4g in one slice; ADD = CHECK + FK + composite UNIQUE (PRIMARY KEY refused); DROP CONSTRAINT = named CHECK + named FK; named-CHECK round-trip via a project.yaml format extension (the rebuild-only db-column arrival — pre-4g projects gain the column on rebuild).

1. Baseline (at handoff)

• After 4f: 1865 passing, 0 failed, 0 skipped, 1 ignored; clippy clean. Branch main, HEAD 5b76315 (4f).

2. Decisions (settled — user-confirmed 2026-05-25)

1. ADD scope = CHECK + FOREIGN KEY + composite UNIQUE. ADD PRIMARY KEY is refused with a clear message (every playground table already has a PK; adding one is near-always invalid). PRIMARY KEY is not in the grammar's ADD-constraint surface.
2. CHECK migration = rebuild-only. The CHECK_TABLE (__rdbms_playground_table_checks) CREATE schema gains a nullable name column; fresh + rebuilt databases get it. A pre-4g project on disk keeps the 3-column table until rebuild. Unnamed table-CHECK creation continues to work on an old DB (its INSERT never names the column); a named CHECK add on an old DB is refused with a friendly "this project predates named constraints — run rebuild first" message (a single PRAGMA-guarded column-presence check, not an auto-migration).
3. Named-CHECK round-trip = project.yaml format extension. Because rebuild reconstructs from project.yaml (do_rebuild_from_text parses the yaml, wipes the db, re-emits DDL via schema_to_ddl), the CHECK name must live in project.yaml, not just the db column. check_constraints: Vec<String> becomes Vec<TableCheck> where TableCheck { name: Option<String>, expr: String }. The yaml reader accepts both the old bare-string form (- "expr", name = None) and the new mapping form (- {expr: "…", name: "…"}), per the established "optional on read" backward-compat convention.
4. DROP CONSTRAINT scope = named CHECK + named FK. Resolution order: look up <name> in CHECK_TABLE (named table-CHECK) → drop it (rebuild without it + delete the row); else in REL_TABLE (named relationship) → drop it via the existing drop-relationship machinery; else refuse "no such constraint <name> on <T>". An unnamed / column-level / UNIQUE constraint is not a DROP CONSTRAINT target.
5. Composite UNIQUE is unnamed (asymmetry, intentional). A composite UNIQUE constraint carries no user-facing name in our model (PRAGMA-detected via the origin='u' auto-index — ADR-0035 §4a.2), so ADD UNIQUE (cols) creates an anonymous constraint and a CONSTRAINT <name> prefix on UNIQUE is refused ("naming a UNIQUE constraint is not supported — use alter table <T> add unique (cols)"). It therefore cannot be a DROP CONSTRAINT <name> target. This is consistent: ADD-UNIQUE is in scope, DROP-UNIQUE was never in scope (no name to target).
6. FK reuses the existing relationship machinery. ADD [CONSTRAINT <name>] FOREIGN KEY (<col>) REFERENCES <P>[(<col>)] … decomposes to add_relationship (the same executor add 1:n relationship uses): parent-PK validation, bare-REFERENCES <P> → parent single-PK resolution, fk_target_type compatibility, auto-naming when unnamed, name-uniqueness, one undo step. No new FK executor.
7. CREATE TABLE table-CHECKs stay unnamed (out of 4g). Naming a CHECK declared inside CREATE TABLE is a separate consistency item; 4g only introduces names via ALTER … ADD CONSTRAINT <name> CHECK. do_create_table writes name = NULL for its table-CHECKs. (Verify the CREATE grammar does not silently swallow a CONSTRAINT <name> on a table CHECK; if it parses one, it is currently dropped — leave as a noted follow-up, do not expand CREATE here.)
8. Internal-__rdbms_* guard folded into do_add_constraint and do_add_relationship (both the table/parent_table and the child_table for relationships). Closes the 4d/4e/4f guard class.

3. Phase 1 — Requirements checklist (4g)

Round-trip backbone (named table-CHECK)

• CHECK_TABLE CREATE schema gains nullable name TEXT (rebuild-only arrival). read_table_checks reads (name, check_expr) ordered by seq. ReadSchema.check_constraints: Vec<TableCheck>.
• schema_to_ddl emits CONSTRAINT <name> CHECK (<expr>) when named, bare CHECK (<expr>) when not.
• persistence::TableSchema.check_constraints: Vec<TableCheck>; the db→persistence capture (db.rs:~2549) carries names.
• yaml writes the mapping form for named, bare string for unnamed; yaml reads both old (bare string) and new (mapping) forms.
• do_create_table writes name = NULL for its table-CHECKs.
• Round-trip test: a named table-CHECK survives save→load and rebuild; an old-format yaml (bare strings) still loads.

Grammar / dispatch

• AlterTableAction gains AddTableConstraint { name: Option<String>, constraint: TableConstraint } and DropConstraint { name: String }.
• New TableConstraint enum: Check { expr_sql: String } (raw text — sql_expr is validate-only), Unique { columns: Vec<String> }, ForeignKey(Box<SqlForeignKey>) (reuse 4b struct).
• Grammar: AT_ADD_TABLE_CONSTRAINT (add [constraint <name>] (check (…) | unique (…) | foreign key (…) references …)) and AT_DROP_CONSTRAINT (drop constraint <name>), added to AT_ACTION_CHOICES. Reuse sql_create_table's table-element nodes for CHECK / UNIQUE / FK where possible.
• Builder discrimination order in build_sql_alter_table: type → (column ⇒ add/rename/drop column) → add ⇒ add-table-constraint → drop ⇒ drop-constraint. (Checking column before the bare add/drop keeps add column … unique/… check routing to AddColumn.)
• Sub-discriminate the table-constraint by check / unique / foreign. A CONSTRAINT <name> on UNIQUE refuses (§2.5).
• Trailing ; tolerated; four existing AlterTableAction branches still route; alter stays advanced-only; table slot rejects __rdbms_* at parse.

Execution

• ADD CHECK (named/unnamed): dry-run guard (existing rows satisfy the CHECK — reuse dry_run_check), rebuild with the table-CHECK in the DDL, write the CHECK_TABLE row (table_name, seq=next, check_expr, name); auto-show; one undo step. Named add on an old DB (no name column) → friendly rebuild-needed refusal.
• ADD UNIQUE (cols): dry-run guard (no duplicate tuples — reuse dry_run_unique/the composite equivalent), rebuild adding the composite UNIQUE; one undo step. Survives rebuild (existing unique_constraints yaml path).
• ADD FOREIGN KEY: decompose to add_relationship (name, parent, child, actions, create_fk = true?). Reuse 4b resolution (bare REFERENCES <P>, self-ref, type compat). One undo step.
• DROP CONSTRAINT : resolve name in CHECK_TABLE then REL_TABLE; drop accordingly; refuse unknown. One undo step.
• Internal-__rdbms_* guard in do_add_constraint / do_add_relationship (both surfaces) + a test.

Testing

• Tier 1 (sql_alter_table_tests in ddl.rs): parse each new form → the right AlterTableAction; the six-branch dispatch still routes the four column actions; named-UNIQUE refusal; ADD PRIMARY KEY refusal.
• Tier 2/round-trip (persistence/yaml unit tests): named CHECK serialize + parse; old bare-string parse; Vec<TableCheck> save/load.
• Tier 3 (tests/sql_alter_table.rs via run_replay): ADD named CHECK enforced + survives rebuild with its name; ADD UNIQUE enforced + survives rebuild; ADD FOREIGN KEY creates the relationship; DROP CONSTRAINT removes a named CHECK and a named FK; DROP unknown refused; one undo step each.
• Internal guard (tests/column_op_guards.rs): simple add_constraint / add_relationship on __rdbms_* refused.
• Catalog lockstep + vocab audit for the refreshed sql_alter_table help/usage (now listing add/drop constraint + add FK).

4. Architecture & change list (file by file)

• src/persistence/mod.rs: add pub struct TableCheck { pub name: Option<String>, pub expr: String }; TableSchema.check_constraints: Vec<TableCheck>. Update the csv_io.rs / mod.rs constructors that set check_constraints: Vec::new() (type still compiles — empty Vec).
• src/persistence/yaml.rs: serialize a TableCheck as a bare string when name is None, else a {expr, name} mapping; parse both forms (back-compat). Update the parser struct + the round-trip tests.
• src/db.rs:
  • CHECK_TABLE CREATE schema += name TEXT (nullable).
  • read_table_checks → Vec<TableCheck> (read name; tolerate a missing name column on an old DB via a column-presence check → name = None).
  • ReadSchema.check_constraints: Vec<TableCheck>; schema_to_ddl emits CONSTRAINT <name> when named.
  • db→persistence capture (≈2549) maps Vec<TableCheck>.
  • build_read_schema (yaml variant, ≈8125) maps persistence TableCheck → ReadSchema TableCheck.
  • do_create_table table-CHECK INSERT writes name = NULL.
  • New executors: do_alter_add_table_check, do_alter_add_unique, do_drop_constraint_by_name. Worker methods
    • Request variants + handler dispatch (wrapped in snapshot_then).
  • reject_internal_table_name at the top of do_add_constraint / do_add_relationship (+ child_table for the latter).
  • A check_table_has_name_column(conn) helper for the rebuild-needed refusal.
• src/dsl/command.rs: AlterTableAction::{AddTableConstraint, DropConstraint}; TableConstraint enum.
• src/dsl/grammar/ddl.rs: AT_ADD_TABLE_CONSTRAINT, AT_DROP_CONSTRAINT, builder branches + sub-discrimination, the named-UNIQUE / ADD-PRIMARY-KEY refusals.
• src/dsl/grammar/sql_create_table.rs: expose the table-CHECK / UNIQUE / FK element nodes for reuse if not already pub(crate).
• src/runtime.rs: SqlAlterTable arm → the new executors; AddTableConstraint::ForeignKey → add_relationship.
• src/app.rs: build_translate_context arms for the two new actions (Operation::AddConstraint / DropConstraint / AddRelationship).
• src/friendly/{keys.rs,strings/en-US.yaml}: refresh sql_alter_table help/usage; any new refusal message keys.

5. Phase 2 — Candidate approaches (key forks)

Round-trip representation. (R1) Vec<TableCheck{name,expr}> threaded through ReadSchema + persistence (lead — single source of truth, clean rebuild). (R2) parallel Vec<Option<String>> names alongside the existing Vec<String> — rejected (two vectors to keep aligned, error prone). (R3) store names only in the db, not yaml — rejected (names lost on rebuild; breaks DROP CONSTRAINT after rebuild).

Executor structure. (E1) one do_alter_add_table_check + one do_alter_add_unique + FK via add_relationship + one do_drop_constraint_by_name (lead — each maps to one rebuild, mirrors the 4e/4f decomposition). (E2) a single mega-executor switching on a constraint enum — rejected (a fat function; the three adds have genuinely different dry-run guards + metadata writes).

Grammar. (G1) separate Choice branches AT_ADD_TABLE_CONSTRAINT / AT_DROP_CONSTRAINT added to AT_ACTION_CHOICES, builder discriminates by column then add/drop then the constraint keyword (lead — consistent with the existing five branches; reuses the create-table element nodes). (G2) a nested sub-Choice under a single add branch — rejected (complicates the builder more than separate branches).

6. Phase 3 — Selection

R1 + E1 + G1. Satisfies every §3 item with the smallest faithful change: the round-trip backbone is a typed extension (not a parallel array), the executors each reduce to one rebuild + one metadata write (one undo step), and the grammar mirrors the established branch structure. The named-UNIQUE refusal and ADD-PRIMARY-KEY refusal keep the surface honest about what the model can persist.

7. Devil's Advocate review of this plan

• Forks escalated? ADD scope, the migration approach, and DROP scope were put to the user (2026-05-25) and answered (CHECK+FK+UNIQUE / rebuild-only / CHECK+FK). The newly-discovered yaml-format-change implication was surfaced and the user chose "Full 4g now". The named-UNIQUE-refusal and CREATE-CHECK-stays-unnamed micro-decisions are consequences of the model (anonymous composite UNIQUE; ALTER-only naming) — noted here, to be confirmed in the combined /runda. ✓
• Back-compat of the yaml change? The reader accepts both the bare string and the mapping form; a test covers an old-format file. The field stays "optional on read". ✓
• Old-DB named-CHECK add? Guarded by a column-presence check → a friendly engine-neutral rebuild-needed refusal, not a raw engine error (ADR-0035 §9). Unnamed CHECK adds keep working on an old DB. ✓
• One undo step each? Each add/drop is one executor call = one rebuild = one snapshot, like 4e/4f. e2e undo checks. ✓
• Grammar trap? Six concrete-keyword-led branches; the builder keys on column (column ops) then add/drop then the constraint keyword. add column … unique/check still routes to AddColumn (checked via column first). A parse test for every branch + the discrimination edges. ✓
• Engine neutrality? New refusal messages say "the database" / "constraint" in the abstract; vocab audit + catalog lockstep tests guard it. ✓
• Anything dropped? ADD PRIMARY KEY (refused, stated), named UNIQUE (refused, stated), CREATE-TABLE CHECK naming (out of scope, noted). No silent drops.

8. Implementation sequence (test-first)

1. Internal guards — reject_internal_table_name in do_add_constraint / do_add_relationship; column_op_guards.rs tests (red → green). Isolated, lands first.
2. Round-trip backbone — TableCheck type; CHECK_TABLE +name; read_table_checks / ReadSchema / schema_to_ddl / capture / do_create_table / yaml serialize+parse. Persistence/yaml round-trip tests (incl. old-format read). No behaviour change yet (all CHECKs still unnamed until the grammar lands) → full suite stays green.
3. Command + grammar + builder — the two actions + TableConstraint; AT_ADD_TABLE_CONSTRAINT / AT_DROP_CONSTRAINT; the discrimination + refusals; Tier-1 parse tests → exhaustive arms (compiler) → green (parse only).
4. Executors + runtime + catalog — do_alter_add_table_check, do_alter_add_unique, do_drop_constraint_by_name, FK via add_relationship; wire SqlAlterTable; refresh help/usage; Tier-3 e2e (ADD CHECK/UNIQUE/FK, DROP CHECK/FK, refusals, rebuild survival, undo) → green.
5. Full sweep — cargo test (no regression from 1865) + cargo clippy --all-targets -- -D warnings.
6. Docs — ADR-0035 Status + §13 4g; README; requirements Q1. Defer the formal /runda to the combined pass (user steer). Propose commit; wait for approval.

9. Exit gate

• All §3 items satisfied; all tiers green, zero skips; no regression from 1865; written-DA PASS (combined /runda to follow); clippy clean; ADR-0035 §13 4g + README + requirements.md lockstep.