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 <name>**: 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.