Files
rdbms-playground/docs/plans/20260526-adr-0035-sql-ddl-4h.md
claude@clouddev1 f7e77a86f8 feat: ADR-0035 4h — ALTER TABLE … RENAME TO
The one genuinely new low-level op in Phase 4: a native engine RENAME TO
plus one-transaction reconciliation (commit-db-last) of everything the
engine does not track —

- every metadata row naming the table: __rdbms_playground_columns, both
  ends of __rdbms_playground_relationships (FK parent, child, and
  self-referential), and __rdbms_playground_table_checks;
- the CSV file, via the existing persistence rewrite+delete path
  (rewritten_tables=[new], deleted_tables=[old]) — no new method;
- CHECK text that qualifies a column with the old table name
  (T.age → U.age, column- and table-level): the engine rewrites the live
  CHECK but the stored text would drift and break a fresh rebuild (a
  planning-/runda finding); rewrite_check_table_qualifier keeps them in
  step. Bounded — a CHECK references only its own table.

Grammar: a fifth AlterTableAction (RenameTable { new }), added by
splitting the `rename` verb into one branch with an inner Choice on a
distinct second keyword (column vs to); the new-name slot mirrors the
CREATE TABLE name slot (NewName + reject_internal_table validator).

Refusals are engine-neutral and case-insensitive (the engine matches
names that way): same-name, case-only, existing-target, __rdbms_*, and
non-existent source. Auto-named indexes and relationships keep their
stale names (only table-name columns update — §6 scope). One undo step;
advanced-mode only; closes the rename half of C1.

Tests: 8 Tier-3 e2e + rewrite-helper unit tests + parse-dispatch tests.
Full suite 1903 passing / 0 failing / 1 ignored; clippy clean.
2026-05-26 08:38:39 +00:00

25 KiB
Raw Permalink Blame History

Plan: ADR-0035 Phase 4, sub-phase 4h — ALTER TABLE … RENAME TO

Add the advanced-mode SQL form:

  • ALTER TABLE <old> RENAME TO <new> — rename a table. Advanced-mode only (no simple-mode rename-table verb; ADR-0035 §6). Closes the rename half of C1 for the advanced surface.

This is the one genuinely new low-level op in Phase 4 (ADR-0035 §6) — not a reuse of an existing executor. Within one transaction it renames the table in the database, renames its data/<old>.csvdata/<new>.csv (via the persistence layer), and updates every metadata row that names it.

User-confirmed scope (2026-05-26):

  • Same-name rename (rename T to T) → refuse with a friendly error (mirrors the existing rename column identical-rename refusal).
  • CHECK-text drift → rewrite the stored CHECK text (the §7 DA finding). The engine rewrites table-qualified column references inside the renamed table's own CHECKs in the live schema (CHECK (T.age>0)CHECK ("U".age>0) — confirmed empirically on SQLite 3.48). Our stored CHECK text (both __rdbms_playground_columns.check_expr and __rdbms_playground_table_checks.check_expr) must be rewritten the same way, or a fresh rebuild emits CHECK (T.age>0) for a table now named U and fails. Bounded problem: a CHECK constraint may reference only the table's own columns (SQLite forbids subqueries / other tables in CHECK), so the only table qualifier that can appear is the old table name — the rewrite target is unambiguous (old/"old"new/ "new"). Reuses/extends the 4e CHECK tokenizer (check_references_ column, db.rs:5489) which already skips string literals and is case-insensitive.
  • Auto-named labels (indexes and relationships) → left stale on rename. ADR-0035 §6 lists only CSV + column/relationship/table-CHECK metadata to update; auto-named indexes (<table>_<cols>_idx) and auto-named relationships ({parent}_{pcol}_to_{child}_{ccol}, db.rs:5982) keep their old names — functional, just cosmetically referencing the old table name. Documented caveat: index names are schema-global and relationship names are UNIQUE, so recreating a table under the old name later could collide with a stale label; this is an accepted consequence (cosmetic refresh is a possible 4i/follow-up, out of 4h).

Decided-and-noted (conventional defaults, no user fork):

  • Rename to an existing other tablerefuse "table <new> already exists" (an explicit, engine-neutral pre-check before the native rename, which would otherwise surface engine wording).
  • Rename of an FK parent or childallowed (unlike DROP, which refuses inbound FKs). The native rename rewrites child FK references in the live schema; we update both ends of the relationship metadata.
  • Success feedbackauto-show the renamed table under its new name (returns a TableDescription, mirroring rename column).
  • Target name → parse-time reject_internal_table validator on the new-name slot (mirrors the CREATE TABLE name slot) + executor reject_internal_table_name guard for defense in depth.

1. Baseline (at handoff 40)

  • After 4g + the rebuild fix: 1885 passing, 0 failed, 0 skipped, 1 ignored (the friendly/mod.rs ```ignore doctest); clippy clean (cargo clippy --all-targets -- -D warnings). Branch main, HEAD 6112859 (the handoff-40 docs commit; 6ff97f6/50a889e/6112859 are local-only — normal). Re-verified this session: 1885 / 0 / 0 / 1.

2. Decisions (settled)

  1. One new low-level executor, do_rename_table. No existing reuse (§6 is explicit: rename is a genuinely new op). It uses the engine's native ALTER TABLE <old> RENAME TO <new> (structure-preserving — no rebuild needed), then updates the three __rdbms_* metadata tables, then drives persistence, then commits the db last (ADR-0015 §6 ordering). Mirrors the shape of do_rename_column (src/db.rs:4314).

  2. CSV rename reuses the existing persistence machinery — no new method. finalize_persistence (src/db.rs:2663) already (a) on schema_dirty rewrites the entire project.yaml from the live db schema — so the rename is reflected automatically — and (b) writes a CSV per rewritten_tables entry (read in-tx by name) and deletes a CSV per deleted_tables entry. So:

    Changes { schema_dirty: true,
              rewritten_tables: vec![new.to_string()],   // writes data/<new>.csv
              deleted_tables:   vec![old.to_string()] }  // removes data/<old>.csv
    

    The renamed table is read by its new name (visible in-tx after the native rename), serialised to data/<new>.csv, and data/<old>.csv is deleted. Empty tables produce no CSV on either side (the existing write_table_data empty-→-delete rule), preserving the ADR-0015 "empty tables → no CSV" invariant. NULL-vs-empty fidelity is preserved because the rows are re-serialised from the db (where NULL is NULL), not byte-copied. No rename_table_data method, no Changes field added. (The Explore-suggested "add a file-rename method" is rejected in §5 R-alt.)

  3. Metadata updates — all three tables, both relationship ends, plus CHECK-text reconciliation. Within the tx, after the native rename:

    • __rdbms_playground_columns (META_TABLE): table_name old→new; and rewrite any column-level check_expr whose text qualifies a reference with the old table name (old./"old".new./"new". — §2.9).
    • __rdbms_playground_relationships (REL_TABLE): parent_table old→new and child_table old→new (two UPDATEs — covers a table that is an FK parent, a child, or self-referential at once). The relationship name is not touched (left stale per the user decision; FK endpoints are stored as table-name values, not as expression text, so they need no rewrite — only the column UPDATE).
    • __rdbms_playground_table_checks (CHECK_TABLE): table_name old→new; and rewrite each table-level check_expr the same way (§2.9). No index metadata table exists (indexes are PRAGMA-derived with a unique flag; ADR-0025 Amd 1), so nothing to update there — indexes follow the renamed table natively and keep their (stale) names per the user decision. DEFAULT expressions do not drift (SQLite defaults cannot reference the table).
  4. Grammar — split the rename verb into an inner Choice (the §6.1 "no same-leading-keyword Choice siblings" rule). Today AT_RENAME_COLUMN is the lone rename-led branch in AT_ACTION_CHOICES. Replace it with one AT_RENAME branch (rename + an inner Choice) whose two tails lead on distinct second keywords: column (→ rename column) and to (→ rename table). Mirrors the 4g add/drop restructure.

  5. New-name ident slot is distinct from the target slot. The table being altered binds role table_name (AT_TABLE_NAME, IdentSource::Tables). The rename target binds a new role new_table_name (IdentSource::NewName, validator: Some(reject_internal_table), all writes_*: false, wrapped in NEW_NAME_HINT) — mirrors the CREATE TABLE name slot for parse-time __rdbms_* refusal and the NEW_COLUMN_NAME hint treatment. Distinct roles keep require_ident(path, "new_table_name") unambiguous.

  6. Builder discrimination (build_sql_alter_table, ddl.rs:2131): insert a rename branch before the final else (DropConstraint). Order becomes typecolumnaddrename → else drop. By the time control reaches the rename check, column is absent (caught earlier), so rename present ⇒ table rename: AlterTableAction::RenameTable { new: require_ident(path, "new_table_name")? }.

  7. One undo step. do_rename_table is one user mutation carrying a source, snapshotted by the worker snapshot_then hook (whole-project snapshot — db backup + yaml/csv copy), so a rename is exactly one undo step. Same wiring as every other SqlAlterTable action.

  8. Replay / history. finalize_persistence appends the literal SQL line to history.log; alter is a schema-write entry word (not in the ADR-0034 Amd 1 app-lifecycle skip set), so the rename replays as a write with no replay-filter change.

  9. CHECK-text qualifier rewrite (§7 DA Finding 1). A new helper rewrite_check_table_qualifier(check_expr, old, new) -> String rewrites every occurrence of the old table name used as a qualifier (immediately followed by .), in both the bare (old.) and quoted ("old".) forms, case-insensitively, skipping string literals — extending the 4e tokenizer that check_references_column already uses. A bare token equal to the old name but not followed by . (e.g. a column literally named like the table) is left untouched, so the common unqualified CHECK (age > 0) is a no-op. Applied in do_rename_table to every column-level check_expr (META) and every table-level check_expr (CHECK_TABLE) of the renamed table. Re-enters in advanced mode (ADR-0030 §11) — the rewritten text is still valid SQL the user could retype.

  10. Existence check is explicit (§7 DA Finding 2). read_schema does not error on a missing table (pragma_table_info returns zero rows). The "no such table" guard uses an explicit do_list_tables(conn)?.iter().any(|t| t == old) check, not a reliance on read_schema failing.

3. Phase 1 — Requirements checklist (4h)

Grammar / dispatch

  • AlterTableAction gains RenameTable { new: String }.
  • NEW_TABLE_NAME ident node (role new_table_name, IdentSource::NewName, reject_internal_table validator, NEW_NAME_HINT).
  • AT_RENAME = Seq[rename, Choice[AT_RENAME_COLUMN_TAIL, AT_RENAME_TABLE_TAIL]]; the two tails lead on distinct keywords (column / to). AT_RENAME_COLUMN_TAIL = column <old> to <new>; AT_RENAME_TABLE_TAIL = to <new>. AT_ACTION_CHOICES swaps AT_RENAME_COLUMNAT_RENAME.
  • build_sql_alter_table routes rename (no column) → RenameTable.
  • Existing four action branches still route (add/drop/rename/alter column, alter-column-type, add/drop constraint); trailing ; tolerated; alter stays advanced-only; source table slot rejects __rdbms_* at parse (existing AT_TABLE_NAME validator); target slot rejects __rdbms_* at parse (new validator).

Execution

  • do_rename_table(conn, persistence, source, old, new): reject_internal_table_name(old) + (new); existence — explicit do_list_tables contains old (→ friendly "no such table", not a reliance on read_schema erroring — §2.10); same-name (old==new) → friendly refusal; existing-target (do_list_tables contains new) → friendly "already exists" refusal (pre-empts the engine's own collision wording); tx: native ALTER TABLE … RENAME TO + the metadata UPDATEs (§2.3) + the CHECK-text rewrite (§2.9, both META and CHECK_TABLE check_expr); do_describe_table(conn, new) for auto-show; finalize_persistence with the §2.2 Changes; tx.commit().
  • rewrite_check_table_qualifier helper (§2.9) + its own unit tests (bare T.ageU.age; quoted "T".age"U".age; case-insensitive; string literal 'T.x' untouched; bare column named T untouched; unqualified age > 0 unchanged).
  • Worker Request::RenameTable { name, new, source, reply }; Database::rename_table(table, new, source) method; handler arm wrapped in snapshot_then (one undo step).
  • runtime.rs SqlAlterTable match: RenameTable { new }database.rename_table(table, new, src) mapped like the RenameColumn arm.
  • app.rs build_translate_context: RenameTable { .. }(Operation::RenameTable, Some(table), None).
  • Operation::RenameTable added; keyword() arm → "rename table".

Testing

  • Tier 1 (sql_alter_table_tests in ddl.rs): parse alter table T rename to URenameTable { new: "U" }; alter table T rename column a to b still → RenameColumn; the other four actions still route; target __rdbms_* refused at parse.
  • Tier 3 (tests/sql_alter_table.rs via run_replay):
    • rename a table with rows → the CSV follows (data/<new>.csv present, data/<old>.csv gone), data intact incl. a NULL.
    • rename an FK parent → relationship metadata parent_table updates; the child's FK still enforces (a violating child insert is rejected under the new name).
    • rename an FK childchild_table updates; FK still enforces.
    • rename a self-referential table → both ends update, no PK conflict on REL_TABLE.
    • rename a table with a table-level CHECKtable_checks rows follow; the CHECK still enforces.
    • rename a table with a table-qualified CHECK (both a column-level CHECK (T.age > 0) and a table-level CHECK (T.a <> T.b)) → the stored check_expr is rewritten to the new name, the CHECK still enforces, and the project survives a fresh rebuild (the precise §7 Finding-1 regression — without the rewrite, rebuild fails with "no such table T").
    • rename a table with an index → index still present + functional (name unchanged, per the user decision).
    • survives a fresh rebuild — delete the .db, rebuild from project.yaml/CSV: the renamed table + all its metadata round-trip (the §6.4 fresh-rebuild discipline).
    • one undo step: rename, undo, the table is back under its old name with its rows/relationship/CHECK; redo reapplies.
    • refusals: rename to an existing other table; rename to the same name; rename to an __rdbms_* name (executor guard, in case the parse validator is bypassed by a synthesised command); rename a non-existent table.
  • Catalog lockstep + vocab audit for the refreshed sql_alter_table usage (now listing rename to <NewName>); the wording stays engine-neutral.

4. Architecture & change list (file by file)

  • src/dsl/command.rs: AlterTableAction::RenameTable { new: String }.
  • src/dsl/grammar/ddl.rs:
    • NEW_TABLE_NAME_IDENT / NEW_TABLE_NAME nodes (≈ near AT_RENAME_COLUMN, mirroring NEW_COLUMN_NAME at line 501 + the CREATE TABLE name slot's reject_internal_table validator).
    • AT_RENAME_COLUMN_TAIL (the existing column … body minus the leading rename), AT_RENAME_TABLE_TAIL (to <new>), AT_RENAME_TAIL (Choice), AT_RENAME (Seq[rename, tail]); swap into AT_ACTION_CHOICES (line 1998).
    • build_sql_alter_table (line 2131): the rename branch + doc-comment update (discrimination now type → column → add → rename → drop).
  • src/db.rs:
    • Request::RenameTable variant (the worker request enum, ≈452650).
    • Database::rename_table method (mirror rename_column).
    • handler dispatch arm (≈ the RenameColumn arm) wrapped in snapshot_then.
    • do_rename_table executor (model: do_rename_column at 4314 + do_drop_table at 3227 for the persistence-cleanup shape). Uses an explicit do_list_tables existence/collision check (§2.10), not read_schema-erroring.
    • rewrite_check_table_qualifier helper near the 4e CHECK tokenizer (check_references_column, db.rs:5489); applied in do_rename_table to META + CHECK_TABLE check_expr (§2.9).
  • src/runtime.rs: SqlAlterTable inner match → RenameTable arm.
  • src/app.rs: build_translate_context inner match → RenameTable arm (≈1595).
  • src/friendly/translate.rs: Operation::RenameTable + keyword() arm "rename table".
  • src/friendly/strings/en-US.yaml: add the rename to <NewName> line to the sql_alter_table usage; any new refusal-message keys (same-name / existing-target) — engine-neutral, vocab-audit-clean. (parse.usage.sql_alter_table / help.ddl.sql_alter_table keys already registered in keys.rs.)

5. Phase 2 — Candidate approaches (key forks)

Rename mechanism. (M1) the engine's native ALTER TABLE … RENAME TO

  • manual metadata UPDATEs (lead — structure-preserving, atomic, no data movement; the engine also rewrites child FK references in the live schema since legacy_alter_table is off by default). (M2) the ADR-0013 rebuild-table primitive (create new, copy rows, drop old) — rejected (heavyweight; rebuilds the whole table to change only its name; rename is not a structural change). (M3) drop + recreate — rejected (loses rows; absurd for a rename).

CSV persistence. (R1) reuse finalize_persistence with rewritten_tables=[new] + deleted_tables=[old] (lead — zero new machinery; re-serialises by the new name, deletes the old, handles empty-table-no-CSV for free). (R-alt) add a Persistence::rename_table_data

  • a Changes.renamed_tables field and fs::rename the file — rejected (new public method + new struct field for behaviour the existing rewrite+delete path already delivers; byte-copy buys nothing over re-serialisation since rows come from the db). (R3) leave the CSV under the old name and special-case the loader — rejected (breaks the data/<table>.csv invariant; confuses a human reading the project dir).

Grammar. (G1) split rename into one branch with an inner Choice on the distinct second keyword (column / to) (lead — the established §6.1 + 4g pattern; trap-safe). (G2) two sibling rename-led branches in AT_ACTION_CHOICESrejected (the walker Choice does not backtrack between same-leading-keyword branches; this is exactly the 4g trap). (G3) make the column keyword optional and disambiguate purely in the builder — rejected (ambiguous grammar; the optional-keyword shape invites the same backtracking trap and muddies completion).

Target __rdbms_* refusal. (V1) parse-time validator on the new-name slot (mirrors CREATE TABLE) and an executor guard (lead — earliest feedback + defense in depth; the worker is directly reachable by synthesised commands/tests). (V2) executor guard only — weaker (loses the pre-submit [ERR] indicator the CREATE name slot gives). (V3) parse-only — rejected (a synthesised RenameTable command would slip a metadata-table rename past the guard).

6. Phase 3 — Selection

M1 + R1 + G1 + V1. Satisfies every §3 item with the smallest faithful change: native rename is the one new low-level op §6 calls for; the CSV rename rides the existing rewrite+delete path (no new persistence surface); the grammar mirrors the trap-safe 4g restructure; the target gets the same parse-time __rdbms_* refusal as CREATE TABLE plus an executor guard. The same-name and existing-target refusals (user-confirmed / conventional) keep the surface honest; auto-named index and relationship names are left as-is per the ADR §6 scope and the user decision; and the CHECK-text rewrite (§2.9) keeps the stored metadata in step with the live schema so a fresh rebuild round-trips.

7. Devil's Advocate review of this plan

Planning /runda pass (2026-05-26) — three findings, all resolved:

  • Finding 1 (BLOCKING, resolved → rewrite). CHECK-expression text drift: empirically confirmed that SQLite (3.48, legacy_alter_table off) rewrites a table-qualified column reference in the renamed table's live CHECK (T.age"U".age), while the stored check_expr would stay T.age and break a fresh rebuild. The original plan was silent on it. Resolved by §2.9 (rewrite the stored CHECK text in both metadata tables), user-confirmed; bounded because a CHECK can only reference its own table; regression test added (§3). This is the exact class as the 50a889e rebuild-metadata bug and the 4e column-CHECK drift — the /runda "probe, don't reason" pass earned its keep again.

  • Finding 2 (correction, resolved). read_schema does not error on a missing table; the existence/“no such table” guard is now an explicit do_list_tables check (§2.10).

  • Finding 3 (consistency, resolved → leave stale). Auto-named relationships embed the table name (db.rs:5982) exactly like auto- named indexes; both are left stale on rename per the user decision, with the UNIQUE/global-name collision caveat documented.

  • Forks escalated? All four genuine forks (same-name behaviour; auto-named-label handling; CHECK-text drift; relationship-vs-index consistency) were put to the user (2026-05-26) and answered. The conventional edges (existing-target refuse; FK-parent/child allowed; auto-show; target-name validation) are decided-and-noted with rationale, inviting correction. No silent autonomous design call. ✓

  • Grammar trap (the recurring 4g bite)? The two rename tails lead on distinct keywords (column vs to) under one rename branch — exactly the §6.1 rule. A parse test for both tails + the four other actions guards it. ✓

  • New-name role collision? The target binds a distinct role (new_table_name), so require_ident cannot confuse it with the table_name target slot. ✓

  • Fresh-rebuild metadata loss (the 50a889e class)? 4h changes metadata values (renames), not the schema — do_rebuild_from_text already wipes + repopulates META/REL/CHECK from yaml, and the yaml is rewritten under the new name by schema_dirty. A fresh-rebuild test is mandatory (§3) and is the precise probe for this class. ✓

  • Both relationship ends + self-ref? Two UPDATEs (parent_table, child_table); a self-referential table updates both with no REL_TABLE PK (child_table, child_column) conflict (single row, new was not previously a child_table). Explicit self-ref test. ✓

  • CSV fidelity (NULL vs empty, empty tables)? Re-serialised from the db, not byte-copied — NULL stays NULL; an empty renamed table writes no <new>.csv and deletes <old>.csv (the existing empty-→-delete rule). Test renames a table with a NULL cell. ✓

  • FK enforcement after rename? legacy_alter_table is off, so the native rename rewrites child FK references in the live schema; the metadata UPDATE keeps REL_TABLE consistent; rebuild regenerates FK DDL from the updated metadata. Tests assert enforcement under the new name + a rebuild round-trip. ✓

  • Engine neutrality? The same-name / existing-target refusals are authored engine-neutral ("table", "the database"); the native rename's own collision error is pre-empted by the explicit do_list_tables check so the engine wording never surfaces. Vocab audit + catalog lockstep guard it. ✓

  • One undo step? One executor call = one snapshot_then = one whole-project snapshot. e2e undo/redo test. ✓

  • Anything dropped? Auto-named index refresh (out of scope, user decision, noted as a possible 4i/follow-up). No silent drops.

8. Implementation sequence (test-first)

  1. Command + Operation + grammar + builder. Add RenameTable variant, Operation::RenameTable, the NEW_TABLE_NAME node + the AT_RENAME split, the builder branch. Tier-1 parse tests (rename-table vs rename-column dispatch; the four other actions; target __rdbms_* refusal) → red, then exhaustive arms (compiler finds runtime.rs / app.rs / keyword()) → green (parse only).
  2. CHECK-text rewrite helper. rewrite_check_table_qualifier (§2.9) with unit tests first (bare/quoted/case/string-literal/bare-column/ unqualified) → red → green. Isolated, lands before the executor uses it.
  3. Executor + worker wiring. do_rename_table (explicit existence check; metadata UPDATEs; the CHECK-text rewrite over META + CHECK_TABLE), Request::RenameTable, Database::rename_table, the snapshot_then handler arm, the runtime.rs + app.rs arms. Tier-3 e2e (rows/CSV, FK parent, FK child, self-ref, table-CHECK, table-qualified CHECK + fresh rebuild (the Finding-1 regression), index, fresh rebuild, undo/redo, all four refusals) → red where they exercise new behaviour, then green.
  4. Catalog + docs. Refresh sql_alter_table usage (rename to); add refusal-message keys; ADR-0035 Status + §13 4h; README; requirements.md Q1/C1 — all lockstep.
  5. Full sweep. cargo test (no regression from 1885) + cargo clippy --all-targets -- -D warnings.
  6. Finished-slice /runda (per handoff §7 — budget at least one, covering 4h; it found the rebuild bug last session). Fix anything it surfaces, re-green.
  7. Commit proposal — propose the message, wait for approval. No AI attribution. (Push is the user's step.)

9. Exit gate

  • All §3 items satisfied; all tiers green, zero skips; no regression from 1885; written-DA / /runda PASS; clippy clean; ADR-0035 §13 4h + README
    • requirements.md lockstep. After 4h, only 4i (the verification sweep) remains to complete Phase 4.