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.
This commit is contained in:
claude@clouddev1
@@ -4,15 +4,17 @@
|
||||
|
||||
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** (`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`, and `ALTER TABLE` add/drop constraint
|
||||
+ add foreign key, implemented 2026-05-25 — plans
|
||||
4e / 4f / 4g / 4h** (`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
|
||||
`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`), so the decision is accepted while the
|
||||
remaining sub-phases (**4h–4i**, §13) continue. This is **Phase 4** of the ADR-0030 roadmap (the
|
||||
`…-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
|
||||
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.
|
||||
@@ -482,6 +484,37 @@ ADR-0033's structure:
|
||||
into `do_add_constraint` / `do_add_relationship`, completing the
|
||||
4d/4e/4f guard class. One undo step per statement.
|
||||
- **4h — `ALTER TABLE … RENAME TO`** (the §6 new low-level op).
|
||||
*(Implemented 2026-05-26 — plan
|
||||
`docs/plans/20260526-adr-0035-sql-ddl-4h.md`.)* The one genuinely new
|
||||
low-level executor in Phase 4 (`do_rename_table`): a native engine
|
||||
`RENAME TO` (structure-preserving — no rebuild) plus reconciliation, in
|
||||
one transaction (commit-db-last), of everything the engine does not
|
||||
track — every metadata row that names the table
|
||||
(`__rdbms_playground_columns`, **both ends** of
|
||||
`__rdbms_playground_relationships`, `__rdbms_playground_table_checks`),
|
||||
the CSV file (via the existing persistence rewrite+delete path:
|
||||
`rewritten_tables = [new]`, `deleted_tables = [old]` — no new
|
||||
persistence method), and **CHECK text that qualifies a column with the
|
||||
old table name** (`T.age` → `U.age`, both column- and table-level — a
|
||||
planning-`/runda` finding: the engine rewrites the *live* CHECK but the
|
||||
*stored* text would drift and break a fresh rebuild;
|
||||
`rewrite_check_table_qualifier` keeps them in step; bounded because a
|
||||
CHECK references only its own table). Grammar: a fifth action,
|
||||
`AlterTableAction::RenameTable { new }`, added by splitting the `rename`
|
||||
verb into one branch with an inner `Choice` on a distinct second keyword
|
||||
(`column` → rename-column, `to` → rename-table — the §6.1 trap-safe
|
||||
pattern); the new-name slot mirrors the `CREATE TABLE` name slot
|
||||
(`IdentSource::NewName` + the `reject_internal_table` parse validator).
|
||||
**Refusals (user-confirmed 2026-05-26):** rename to the same name, to an
|
||||
existing other table, to an `__rdbms_*` name, or of a non-existent
|
||||
table. Collision checks are **case-insensitive** (the engine matches
|
||||
names that way), with an engine-neutral pre-check so a case-only rename
|
||||
or a case-insensitive clash never surfaces the raw engine error
|
||||
(a finished-slice `/runda` finding). **Auto-named indexes *and*
|
||||
relationships keep their stale names** (only the table-name *columns*
|
||||
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
|
||||
|
||||
+1
-1
File diff suppressed because one or more lines are too long
@@ -0,0 +1,437 @@
|
||||
# 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>.csv` → `data/<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* table** → **refuse** "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 child** → **allowed** (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 feedback** → **auto-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:
|
||||
```rust
|
||||
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 `type` → `column` → `add` → **`rename`** → 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_COLUMN` → `AT_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.age`→`U.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 U` → `RenameTable { 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 child** → `child_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 CHECK** → `table_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, ≈452–650).
|
||||
- `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_CHOICES` — *rejected* (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.
|
||||
+11
-2
@@ -150,7 +150,10 @@ handoff-14 cleanup; 449 after B2/C2.)
|
||||
## DSL data commands
|
||||
|
||||
- [ ] **C1** Table operations: create / drop / rename.
|
||||
*(Progress: create + drop done; rename pending.)*
|
||||
*(Progress: create + drop done; **rename done on the advanced
|
||||
surface** — `ALTER TABLE … RENAME TO`, ADR-0035 §6 / 4h. A simple-mode
|
||||
rename-table verb is deliberately not provided — table rename is
|
||||
advanced-mode only.)*
|
||||
- [x] **C2** Column operations: add / drop / rename / change
|
||||
type. `drop column` and `rename column` use SQLite native
|
||||
ALTER TABLE (3.35+ / 3.25+); `change column` routes through
|
||||
@@ -247,7 +250,13 @@ handoff-14 cleanup; 449 after B2/C2.)
|
||||
`name` column on `__rdbms_playground_table_checks` + a `project.yaml`
|
||||
`check_constraints` `{expr, name}` extension; the internal-table guard
|
||||
completed across `do_add_constraint`/`do_add_relationship`)).
|
||||
Remaining DDL — `ALTER TABLE … RENAME TO` (4h) — is phased per
|
||||
then `ALTER TABLE … RENAME TO` (4h — the one genuinely new low-level
|
||||
op, `do_rename_table`: native rename + one-transaction reconciliation of
|
||||
the CSV file and every metadata row naming the table, incl. **rewriting
|
||||
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.)*
|
||||
- [ ] **Q2** Non-standard syntax rejected with a clear message
|
||||
pointing at the supported subset.
|
||||
|
||||
@@ -1608,6 +1608,12 @@ impl App {
|
||||
AlterTableAction::DropConstraint { .. } => {
|
||||
(Operation::DropConstraint, Some(table.as_str()), None)
|
||||
}
|
||||
// `RENAME TO <new>` — the failure concerns the table being
|
||||
// renamed (the old name); the executor authors the
|
||||
// existing-target / same-name refusals (ADR-0035 §6, 4h).
|
||||
AlterTableAction::RenameTable { .. } => {
|
||||
(Operation::RenameTable, Some(table.as_str()), None)
|
||||
}
|
||||
},
|
||||
C::SqlCreateTable { name, .. } => {
|
||||
(Operation::CreateTable, Some(name.as_str()), None)
|
||||
|
||||
@@ -528,6 +528,13 @@ enum Request {
|
||||
source: Option<String>,
|
||||
reply: oneshot::Sender<Result<TableDescription, DbError>>,
|
||||
},
|
||||
/// `ALTER TABLE <table> RENAME TO <new>` (ADR-0035 §6, 4h).
|
||||
RenameTable {
|
||||
table: String,
|
||||
new: String,
|
||||
source: Option<String>,
|
||||
reply: oneshot::Sender<Result<TableDescription, DbError>>,
|
||||
},
|
||||
ChangeColumnType {
|
||||
table: String,
|
||||
column: String,
|
||||
@@ -1209,6 +1216,24 @@ impl Database {
|
||||
recv.await.map_err(|_| DbError::WorkerGone)?
|
||||
}
|
||||
|
||||
/// `ALTER TABLE <table> RENAME TO <new>` (ADR-0035 §6, 4h).
|
||||
pub async fn rename_table(
|
||||
&self,
|
||||
table: String,
|
||||
new: String,
|
||||
source: Option<String>,
|
||||
) -> Result<TableDescription, DbError> {
|
||||
let (reply, recv) = oneshot::channel();
|
||||
self.send(Request::RenameTable {
|
||||
table,
|
||||
new,
|
||||
source,
|
||||
reply,
|
||||
})
|
||||
.await?;
|
||||
recv.await.map_err(|_| DbError::WorkerGone)?
|
||||
}
|
||||
|
||||
pub async fn change_column_type(
|
||||
&self,
|
||||
table: String,
|
||||
@@ -2050,6 +2075,20 @@ fn handle_request(
|
||||
&new,
|
||||
));
|
||||
}
|
||||
Request::RenameTable {
|
||||
table,
|
||||
new,
|
||||
source,
|
||||
reply,
|
||||
} => {
|
||||
snapshot_then(snap, batch, conn, source.as_deref(), reply, || do_rename_table(
|
||||
conn,
|
||||
persistence,
|
||||
source.as_deref(),
|
||||
&table,
|
||||
&new,
|
||||
));
|
||||
}
|
||||
Request::ChangeColumnType {
|
||||
table,
|
||||
column,
|
||||
@@ -4414,6 +4453,188 @@ fn do_rename_column(
|
||||
Ok(description)
|
||||
}
|
||||
|
||||
/// Rename a table (ADR-0035 §6, sub-phase 4h) — the one genuinely new
|
||||
/// low-level op in Phase 4.
|
||||
///
|
||||
/// Uses SQLite's native `ALTER TABLE … RENAME TO` (structure-preserving,
|
||||
/// so no rebuild), which also rewrites references to the old name in
|
||||
/// other tables' FK declarations and inside the renamed table's own
|
||||
/// CHECK / self-FK definitions (the engine's modern, non-legacy
|
||||
/// behaviour). We then reconcile everything the engine does *not* know
|
||||
/// about, all in one transaction (commit-db-last, ADR-0015 §6):
|
||||
///
|
||||
/// 1. every metadata row that names the table — `__rdbms_playground_columns`
|
||||
/// (`table_name`), **both ends** of `__rdbms_playground_relationships`
|
||||
/// (`parent_table` *and* `child_table`, covering a self-referential
|
||||
/// table), and `__rdbms_playground_table_checks` (`table_name`);
|
||||
/// 2. CHECK *text* that qualifies a column with the old table name
|
||||
/// (`T.age` → `U.age`), in both metadata tables — the live schema was
|
||||
/// already rewritten, so the stored text must match or a rebuild fails;
|
||||
/// 3. the CSV file, via the existing persistence rewrite+delete path
|
||||
/// (`rewritten_tables = [new]`, `deleted_tables = [old]`).
|
||||
///
|
||||
/// Auto-named indexes and relationships keep their (now stale but
|
||||
/// functional) names — only the table-name *columns* update (ADR-0035 §6
|
||||
/// scope; user-confirmed). One undo step (the worker's whole-project
|
||||
/// snapshot).
|
||||
fn do_rename_table(
|
||||
conn: &Connection,
|
||||
persistence: Option<&Persistence>,
|
||||
source: Option<&str>,
|
||||
old: &str,
|
||||
new: &str,
|
||||
) -> Result<TableDescription, DbError> {
|
||||
reject_internal_table_name(old)?;
|
||||
reject_internal_table_name(new)?;
|
||||
|
||||
// Existence + collision: `read_schema` does not error on a missing
|
||||
// table (`pragma_table_info` returns no rows), so check explicitly —
|
||||
// and pre-empt the engine's own collision error so the refusal stays
|
||||
// engine-neutral (ADR-0035 §9).
|
||||
let tables = do_list_tables(conn)?;
|
||||
if !tables.iter().any(|t| t == old) {
|
||||
return Err(DbError::Sqlite {
|
||||
message: format!("no such table: {old}"),
|
||||
kind: SqliteErrorKind::NoSuchTable,
|
||||
});
|
||||
}
|
||||
if old == new {
|
||||
return Err(DbError::Unsupported(format!(
|
||||
"rename: new name is identical to the existing one (`{old}`)."
|
||||
)));
|
||||
}
|
||||
// The database matches table names case-insensitively, so collision
|
||||
// checks must be case-insensitive too — otherwise the native rename
|
||||
// surfaces a raw engine collision error (ADR-0035 §9). A target that
|
||||
// differs from the source only in capitalization is a no-op the engine
|
||||
// cannot perform; a target colliding with a *different* table is the
|
||||
// ordinary "already exists" refusal.
|
||||
if old.eq_ignore_ascii_case(new) {
|
||||
return Err(DbError::Unsupported(format!(
|
||||
"`{old}` and `{new}` differ only in capitalization; the database \
|
||||
treats them as the same table, so there is nothing to rename."
|
||||
)));
|
||||
}
|
||||
if tables.iter().any(|t| t.eq_ignore_ascii_case(new)) {
|
||||
return Err(DbError::Unsupported(format!(
|
||||
"table `{new}` already exists; pick a different name."
|
||||
)));
|
||||
}
|
||||
|
||||
let tx = conn
|
||||
.unchecked_transaction()
|
||||
.map_err(DbError::from_rusqlite)?;
|
||||
let ddl = format!(
|
||||
"ALTER TABLE {old_t} RENAME TO {new_t};",
|
||||
old_t = quote_ident(old),
|
||||
new_t = quote_ident(new),
|
||||
);
|
||||
debug!(ddl = %ddl, "rename_table");
|
||||
tx.execute_batch(&ddl).map_err(DbError::from_rusqlite)?;
|
||||
|
||||
// (1) Rename the table name in every metadata row that names it.
|
||||
tx.execute(
|
||||
&format!("UPDATE {META_TABLE} SET table_name = ?1 WHERE table_name = ?2;"),
|
||||
[new, old],
|
||||
)
|
||||
.map_err(DbError::from_rusqlite)?;
|
||||
// Both relationship ends — parent and child — so an FK parent, an FK
|
||||
// child, and a self-referencing table are all covered. The
|
||||
// relationship `name` is left as-is (auto-names embed the old table
|
||||
// name; stale-but-functional per the user decision, like index names).
|
||||
tx.execute(
|
||||
&format!("UPDATE {REL_TABLE} SET parent_table = ?1 WHERE parent_table = ?2;"),
|
||||
[new, old],
|
||||
)
|
||||
.map_err(DbError::from_rusqlite)?;
|
||||
tx.execute(
|
||||
&format!("UPDATE {REL_TABLE} SET child_table = ?1 WHERE child_table = ?2;"),
|
||||
[new, old],
|
||||
)
|
||||
.map_err(DbError::from_rusqlite)?;
|
||||
tx.execute(
|
||||
&format!("UPDATE {CHECK_TABLE} SET table_name = ?1 WHERE table_name = ?2;"),
|
||||
[new, old],
|
||||
)
|
||||
.map_err(DbError::from_rusqlite)?;
|
||||
|
||||
// (2) Reconcile CHECK *text* that qualifies a reference with the old
|
||||
// table name. Read the rows (now keyed by `new`), rewrite, write back
|
||||
// only when changed — the common unqualified CHECK is a no-op.
|
||||
let col_checks: Vec<(String, String)> = {
|
||||
let mut stmt = tx
|
||||
.prepare(&format!(
|
||||
"SELECT column_name, check_expr FROM {META_TABLE} \
|
||||
WHERE table_name = ?1 AND check_expr IS NOT NULL;"
|
||||
))
|
||||
.map_err(DbError::from_rusqlite)?;
|
||||
let rows = stmt
|
||||
.query_map([new], |r| Ok((r.get::<_, String>(0)?, r.get::<_, String>(1)?)))
|
||||
.map_err(DbError::from_rusqlite)?;
|
||||
let mut v = Vec::new();
|
||||
for row in rows {
|
||||
v.push(row.map_err(DbError::from_rusqlite)?);
|
||||
}
|
||||
v
|
||||
};
|
||||
for (column_name, expr) in col_checks {
|
||||
let rewritten = rewrite_check_table_qualifier(&expr, old, new);
|
||||
if rewritten != expr {
|
||||
tx.execute(
|
||||
&format!(
|
||||
"UPDATE {META_TABLE} SET check_expr = ?1 \
|
||||
WHERE table_name = ?2 AND column_name = ?3;"
|
||||
),
|
||||
rusqlite::params![rewritten, new, column_name],
|
||||
)
|
||||
.map_err(DbError::from_rusqlite)?;
|
||||
}
|
||||
}
|
||||
let table_checks: Vec<(i64, String)> = {
|
||||
let mut stmt = tx
|
||||
.prepare(&format!(
|
||||
"SELECT seq, check_expr FROM {CHECK_TABLE} WHERE table_name = ?1;"
|
||||
))
|
||||
.map_err(DbError::from_rusqlite)?;
|
||||
let rows = stmt
|
||||
.query_map([new], |r| Ok((r.get::<_, i64>(0)?, r.get::<_, String>(1)?)))
|
||||
.map_err(DbError::from_rusqlite)?;
|
||||
let mut v = Vec::new();
|
||||
for row in rows {
|
||||
v.push(row.map_err(DbError::from_rusqlite)?);
|
||||
}
|
||||
v
|
||||
};
|
||||
for (seq, expr) in table_checks {
|
||||
let rewritten = rewrite_check_table_qualifier(&expr, old, new);
|
||||
if rewritten != expr {
|
||||
tx.execute(
|
||||
&format!(
|
||||
"UPDATE {CHECK_TABLE} SET check_expr = ?1 \
|
||||
WHERE table_name = ?2 AND seq = ?3;"
|
||||
),
|
||||
rusqlite::params![rewritten, new, seq],
|
||||
)
|
||||
.map_err(DbError::from_rusqlite)?;
|
||||
}
|
||||
}
|
||||
|
||||
let description = do_describe_table(conn, new)?;
|
||||
// (3) CSV follows the table: write `data/<new>.csv` from the renamed
|
||||
// table (read in-tx by its new name) and delete `data/<old>.csv` — the
|
||||
// existing rewrite+delete path; an empty table writes no CSV on either
|
||||
// side. `schema_dirty` rewrites `project.yaml`, which reflects the new
|
||||
// name automatically.
|
||||
let changes = Changes {
|
||||
schema_dirty: true,
|
||||
rewritten_tables: vec![new.to_string()],
|
||||
deleted_tables: vec![old.to_string()],
|
||||
};
|
||||
finalize_persistence(conn, persistence, source, &changes)?;
|
||||
tx.commit().map_err(DbError::from_rusqlite)?;
|
||||
Ok(description)
|
||||
}
|
||||
|
||||
/// Change a column's type.
|
||||
///
|
||||
/// Change a column's user-facing type, per ADR-0017's per-cell
|
||||
@@ -5541,6 +5762,141 @@ mod check_references_column_tests {
|
||||
}
|
||||
}
|
||||
|
||||
/// Rewrite the old table name where it is used as a **qualifier** in a
|
||||
/// CHECK expression (the identifier immediately followed by `.`) to the
|
||||
/// new name — both the bare (`old.col`) and double-quoted (`"old".col`)
|
||||
/// forms, case-insensitively — leaving everything else byte-for-byte
|
||||
/// intact (ADR-0035 §6, sub-phase 4h).
|
||||
///
|
||||
/// **Why this is needed.** A native `ALTER TABLE old RENAME TO new`
|
||||
/// rewrites table-qualified column references inside the renamed table's
|
||||
/// *live* CHECK (`CHECK (T.age>0)` → `CHECK ("U".age>0)`), but our
|
||||
/// *stored* CHECK text would keep the old name and break a later rebuild
|
||||
/// (`schema_to_ddl` would emit `CHECK (T.age>0)` for a table now named
|
||||
/// `U` → "no such table T"). This keeps the stored text in step with the
|
||||
/// live schema. **Bounded problem:** a CHECK may reference only its own
|
||||
/// table's columns (SQLite forbids subqueries / other tables in a CHECK),
|
||||
/// so the only table qualifier that can appear is the renamed table's
|
||||
/// name — the rewrite target is unambiguous.
|
||||
///
|
||||
/// Extends the [`check_references_column`] tokenizer: skips single-quoted
|
||||
/// string literals (a literal containing the old name is untouched), and
|
||||
/// a bare identifier equal to the old name but **not** used as a qualifier
|
||||
/// (not followed by `.` — e.g. a column literally named like the table)
|
||||
/// is left alone, so the common unqualified CHECK (`age > 0`) is a no-op.
|
||||
fn rewrite_check_table_qualifier(check_expr: &str, old: &str, new: &str) -> String {
|
||||
use crate::dsl::walker::lex_helpers::{consume_ident, consume_string_literal};
|
||||
let bytes = check_expr.as_bytes();
|
||||
// Whether the next byte at `pos` begins a `.` qualifier separator.
|
||||
let dot_follows = |pos: usize| bytes.get(pos) == Some(&b'.');
|
||||
let mut out = String::with_capacity(check_expr.len());
|
||||
let mut i = 0;
|
||||
while i < check_expr.len() {
|
||||
// Single-quoted string literal — copy verbatim, never rewrite.
|
||||
if let Some(((start, end), _)) = consume_string_literal(check_expr, i) {
|
||||
out.push_str(&check_expr[start..end]);
|
||||
i = end;
|
||||
continue;
|
||||
}
|
||||
// Double-quoted identifier — scan to the closing quote (`""` is an
|
||||
// escaped quote). Rewrite when it is the old name used as a
|
||||
// qualifier; table names never contain quotes, so the raw inner
|
||||
// text suffices for comparison.
|
||||
if bytes[i] == b'"' {
|
||||
let mut j = i + 1;
|
||||
while j < check_expr.len() {
|
||||
if bytes[j] == b'"' {
|
||||
if bytes.get(j + 1) == Some(&b'"') {
|
||||
j += 2; // escaped quote inside the identifier
|
||||
continue;
|
||||
}
|
||||
break; // closing quote
|
||||
}
|
||||
j += 1;
|
||||
}
|
||||
let end = (j + 1).min(check_expr.len()); // byte after closing `"`
|
||||
let inner = &check_expr[i + 1..j.min(check_expr.len())];
|
||||
if inner.eq_ignore_ascii_case(old) && dot_follows(end) {
|
||||
out.push('"');
|
||||
out.push_str(new);
|
||||
out.push('"');
|
||||
} else {
|
||||
out.push_str(&check_expr[i..end]);
|
||||
}
|
||||
i = end;
|
||||
continue;
|
||||
}
|
||||
// Bare identifier.
|
||||
if let Some((start, end)) = consume_ident(check_expr, i) {
|
||||
if check_expr[start..end].eq_ignore_ascii_case(old) && dot_follows(end) {
|
||||
out.push_str(new);
|
||||
} else {
|
||||
out.push_str(&check_expr[start..end]);
|
||||
}
|
||||
i = end;
|
||||
continue;
|
||||
}
|
||||
// Operator / paren / number / punctuation — copy one char.
|
||||
let ch_len = check_expr[i..].chars().next().map_or(1, char::len_utf8);
|
||||
out.push_str(&check_expr[i..i + ch_len]);
|
||||
i += ch_len;
|
||||
}
|
||||
out
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod rewrite_check_table_qualifier_tests {
|
||||
use super::rewrite_check_table_qualifier;
|
||||
|
||||
#[test]
|
||||
fn rewrites_a_bare_qualifier() {
|
||||
assert_eq!(rewrite_check_table_qualifier("T.age > 0", "T", "U"), "U.age > 0");
|
||||
// multiple occurrences, table-level CHECK shape
|
||||
assert_eq!(rewrite_check_table_qualifier("T.a <> T.b", "T", "U"), "U.a <> U.b");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn is_case_insensitive_on_the_qualifier() {
|
||||
assert_eq!(rewrite_check_table_qualifier("t.age > 0", "T", "U"), "U.age > 0");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rewrites_a_quoted_qualifier() {
|
||||
assert_eq!(
|
||||
rewrite_check_table_qualifier("\"T\".age > 0", "T", "U"),
|
||||
"\"U\".age > 0"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn leaves_string_literals_untouched() {
|
||||
assert_eq!(
|
||||
rewrite_check_table_qualifier("note <> 'T.x' AND T.age > 0", "T", "U"),
|
||||
"note <> 'T.x' AND U.age > 0"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn leaves_a_bare_name_that_is_not_a_qualifier() {
|
||||
// A column literally named like the table, not followed by `.`.
|
||||
assert_eq!(rewrite_check_table_qualifier("T > 0", "T", "U"), "T > 0");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unqualified_check_is_a_no_op() {
|
||||
assert_eq!(rewrite_check_table_qualifier("age > 0", "T", "U"), "age > 0");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn does_not_match_a_longer_identifier() {
|
||||
// `Total` merely starts with `T`; not the qualifier `T`.
|
||||
assert_eq!(
|
||||
rewrite_check_table_qualifier("Total.x > 0", "T", "U"),
|
||||
"Total.x > 0"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// Whether any CHECK constraint on `table` references `column` — both the
|
||||
/// table-level CHECKs (`read_table_checks`) and the column-level CHECKs
|
||||
/// (`schema.columns[].check`), the guard for drop/rename column
|
||||
|
||||
@@ -753,6 +753,12 @@ pub enum AlterTableAction {
|
||||
/// `DROP CONSTRAINT <name>` — drops a named table-level CHECK or a
|
||||
/// named FK (relationship), resolved by name (ADR-0035 §4g).
|
||||
DropConstraint { name: String },
|
||||
/// `RENAME TO <new>` — rename the table (ADR-0035 §6, sub-phase 4h).
|
||||
/// The one genuinely new low-level op in Phase 4: a native table
|
||||
/// rename plus reconciliation of the CSV file name and every metadata
|
||||
/// row that names the table (columns, both relationship ends, table
|
||||
/// CHECKs, and any table-qualified CHECK *text*). Advanced-mode only.
|
||||
RenameTable { new: String },
|
||||
}
|
||||
|
||||
/// A table-level constraint added via `ALTER TABLE … ADD [CONSTRAINT
|
||||
|
||||
+88
-5
@@ -1904,14 +1904,49 @@ const AT_ADD_COLUMN_TAIL: Node = Node::Seq(AT_ADD_COLUMN_TAIL_NODES);
|
||||
static AT_DROP_COLUMN_TAIL_NODES: &[Node] = &[Node::Word(Word::keyword("column")), COLUMN_NAME];
|
||||
const AT_DROP_COLUMN_TAIL: Node = Node::Seq(AT_DROP_COLUMN_TAIL_NODES);
|
||||
|
||||
static AT_RENAME_COLUMN_NODES: &[Node] = &[
|
||||
Node::Word(Word::keyword("rename")),
|
||||
// New-table-name slot for `RENAME TO <new>` (ADR-0035 §6, sub-phase 4h).
|
||||
// Mirrors the `CREATE TABLE` name slot: `IdentSource::NewName` (a name
|
||||
// being introduced, not completed from existing tables) + the same
|
||||
// `reject_internal_table` parse-time validator, so an `__rdbms_*` target
|
||||
// is refused before submit. Wrapped in `NEW_NAME_HINT` like
|
||||
// `NEW_COLUMN_NAME`. `writes_table: false` — nothing downstream of
|
||||
// `rename to <new>` references the schema cache.
|
||||
const NEW_TABLE_NAME_IDENT: Node = Node::Ident {
|
||||
source: IdentSource::NewName,
|
||||
role: "new_table_name",
|
||||
validator: Some(super::sql_select::reject_internal_table),
|
||||
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 NEW_TABLE_NAME: Node = Node::Hinted {
|
||||
mode: NEW_NAME_HINT,
|
||||
inner: &NEW_TABLE_NAME_IDENT,
|
||||
};
|
||||
|
||||
// The `rename` verb fans out (like `add`/`drop`, §6.1) to an inner
|
||||
// `Choice` whose two tails lead on DISTINCT second keywords: `column`
|
||||
// (rename column) and `to` (rename table — 4h). The walker `Choice`
|
||||
// selects by the leading token and never backtracks between branches, so
|
||||
// the distinct keywords keep them apart.
|
||||
static AT_RENAME_COLUMN_TAIL_NODES: &[Node] = &[
|
||||
Node::Word(Word::keyword("column")),
|
||||
COLUMN_NAME,
|
||||
Node::Word(Word::keyword("to")),
|
||||
NEW_COLUMN_NAME,
|
||||
];
|
||||
const AT_RENAME_COLUMN: Node = Node::Seq(AT_RENAME_COLUMN_NODES);
|
||||
const AT_RENAME_COLUMN_TAIL: Node = Node::Seq(AT_RENAME_COLUMN_TAIL_NODES);
|
||||
static AT_RENAME_TABLE_TAIL_NODES: &[Node] =
|
||||
&[Node::Word(Word::keyword("to")), NEW_TABLE_NAME];
|
||||
const AT_RENAME_TABLE_TAIL: Node = Node::Seq(AT_RENAME_TABLE_TAIL_NODES);
|
||||
static AT_RENAME_TAIL_CHOICES: &[Node] = &[AT_RENAME_COLUMN_TAIL, AT_RENAME_TABLE_TAIL];
|
||||
const AT_RENAME_TAIL: Node = Node::Choice(AT_RENAME_TAIL_CHOICES);
|
||||
static AT_RENAME_NODES: &[Node] = &[Node::Word(Word::keyword("rename")), AT_RENAME_TAIL];
|
||||
const AT_RENAME: Node = Node::Seq(AT_RENAME_NODES);
|
||||
|
||||
// `ALTER COLUMN <col> TYPE <type>` (ADR-0035 §4f). The type slot reuses
|
||||
// SQL_TYPE (the same alias map + `double precision` pair the CREATE
|
||||
@@ -1995,7 +2030,7 @@ const AT_DROP: Node = Node::Seq(AT_DROP_NODES);
|
||||
// concrete keywords, trap-safe. (The branch's `alter` is the action
|
||||
// word; the entry-word `alter` was already consumed by dispatch.) The
|
||||
// second-keyword fan-out happens in `AT_ADD` / `AT_DROP`'s inner Choice.
|
||||
static AT_ACTION_CHOICES: &[Node] = &[AT_ADD, AT_DROP, AT_RENAME_COLUMN, AT_ALTER_COLUMN];
|
||||
static AT_ACTION_CHOICES: &[Node] = &[AT_ADD, AT_DROP, AT_RENAME, AT_ALTER_COLUMN];
|
||||
const AT_ACTION: Node = Node::Choice(AT_ACTION_CHOICES);
|
||||
|
||||
static SQL_ALTER_TABLE_SHAPE_NODES: &[Node] = &[
|
||||
@@ -2127,7 +2162,12 @@ fn build_alter_column_type(path: &MatchedPath) -> Result<AlterTableAction, Valid
|
||||
/// still routes to AddColumn.
|
||||
/// 3. **`add`** — a table-level constraint (CHECK / UNIQUE / FK / the
|
||||
/// refused PRIMARY KEY).
|
||||
/// 4. else **`drop`** — `drop constraint <name>`.
|
||||
/// 4. **`rename`** — `rename to <new>` (table rename, 4h). Reached only
|
||||
/// when `column` is absent (caught by step 2), so a lone `rename`
|
||||
/// means the table form. The new name binds a *distinct* role
|
||||
/// (`new_table_name`), so it never collides with the `table_name`
|
||||
/// target slot.
|
||||
/// 5. else **`drop`** — `drop constraint <name>`.
|
||||
fn build_sql_alter_table(path: &MatchedPath, source: &str) -> Result<Command, ValidationError> {
|
||||
let table = require_ident(path, "table_name")?;
|
||||
let action = if path.contains_word("type") {
|
||||
@@ -2147,6 +2187,10 @@ fn build_sql_alter_table(path: &MatchedPath, source: &str) -> Result<Command, Va
|
||||
}
|
||||
} else if path.contains_word("add") {
|
||||
build_alter_add_table_constraint(path, source)?
|
||||
} else if path.contains_word("rename") {
|
||||
AlterTableAction::RenameTable {
|
||||
new: require_ident(path, "new_table_name")?,
|
||||
}
|
||||
} else {
|
||||
AlterTableAction::DropConstraint {
|
||||
name: require_ident(path, "constraint_name")?,
|
||||
@@ -2791,6 +2835,45 @@ mod sql_alter_table_tests {
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rename_table() {
|
||||
// ADR-0035 §6 / 4h: `rename to <new>` — the `rename` verb fans out
|
||||
// on a distinct second keyword (`to` vs `column`).
|
||||
let (table, action) = alter("alter table Orders rename to Purchases");
|
||||
assert_eq!(table, "Orders");
|
||||
match action {
|
||||
AlterTableAction::RenameTable { new } => assert_eq!(new, "Purchases"),
|
||||
other => panic!("expected RenameTable, got {other:?}"),
|
||||
}
|
||||
// trailing semicolon tolerated
|
||||
assert!(matches!(
|
||||
alter("alter table Orders rename to Purchases;").1,
|
||||
AlterTableAction::RenameTable { .. }
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rename_table_does_not_steal_rename_column() {
|
||||
// The two `rename` tails coexist: `rename to` → table,
|
||||
// `rename column … to …` → column. Neither misroutes.
|
||||
assert!(matches!(
|
||||
alter("alter table T rename to U").1,
|
||||
AlterTableAction::RenameTable { .. }
|
||||
));
|
||||
assert!(matches!(
|
||||
alter("alter table T rename column a to b").1,
|
||||
AlterTableAction::RenameColumn { .. }
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rename_to_internal_target_refused_at_parse() {
|
||||
// The target slot carries the `reject_internal_table` validator
|
||||
// (mirroring CREATE TABLE), so an `__rdbms_*` target is refused
|
||||
// before submit — engine-neutral, not a raw engine error.
|
||||
assert!(parse_command_in_mode("alter table T rename to __rdbms_evil", Mode::Advanced).is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn alter_column_type_parses() {
|
||||
// ADR-0035 §4f: the fourth action, discriminated by the `type`
|
||||
|
||||
@@ -274,6 +274,7 @@ help:
|
||||
alter table <T> add column <col> <type> [not null] [unique] [default …] [check …]
|
||||
alter table <T> drop column <col>
|
||||
alter table <T> rename column <old> to <new>
|
||||
alter table <T> rename to <new>
|
||||
alter table <T> alter column <col> type <type>
|
||||
alter table <T> add [constraint <name>] check (<expr>) | unique (<col>, …) | foreign key (<col>) references <P>[(<col>)]
|
||||
alter table <T> drop constraint <name> — evolve a table's columns and constraints (advanced SQL)
|
||||
@@ -482,6 +483,7 @@ parse:
|
||||
alter table <Table> add column <Name> <Type> [not null] [unique] [default <expr>] [check (<expr>)]
|
||||
alter table <Table> drop column <Name>
|
||||
alter table <Table> rename column <Old> to <New>
|
||||
alter table <Table> rename to <NewName>
|
||||
alter table <Table> alter column <Name> type <Type>
|
||||
alter table <Table> add [constraint <Name>] check (<expr>) | unique (<col>, ...) | foreign key (<col>) references <Parent>[(<col>)]
|
||||
alter table <Table> drop constraint <Name>
|
||||
|
||||
@@ -62,6 +62,7 @@ pub enum Operation {
|
||||
AddColumn,
|
||||
DropColumn,
|
||||
RenameColumn,
|
||||
RenameTable,
|
||||
ChangeColumnType,
|
||||
AddRelationship,
|
||||
DropRelationship,
|
||||
@@ -93,6 +94,7 @@ impl Operation {
|
||||
Self::AddColumn => "add column",
|
||||
Self::DropColumn => "drop column",
|
||||
Self::RenameColumn => "rename column",
|
||||
Self::RenameTable => "rename table",
|
||||
Self::ChangeColumnType => "change column",
|
||||
Self::AddRelationship => "add relationship",
|
||||
Self::DropRelationship => "drop relationship",
|
||||
|
||||
@@ -2149,6 +2149,13 @@ async fn execute_command_typed(
|
||||
.alter_drop_constraint(table, name, src)
|
||||
.await
|
||||
.map(CommandOutcome::Schema),
|
||||
// `RENAME TO <new>` — the one genuinely new low-level op
|
||||
// (ADR-0035 §6, 4h): native table rename + CSV + metadata
|
||||
// reconciliation, one undo step.
|
||||
AlterTableAction::RenameTable { new } => database
|
||||
.rename_table(table, new, src)
|
||||
.await
|
||||
.map(|d| CommandOutcome::Schema(Some(d))),
|
||||
},
|
||||
Command::AddConstraint {
|
||||
table,
|
||||
|
||||
@@ -652,3 +652,426 @@ fn e2e_named_check_metadata_survives_a_fresh_rebuild() {
|
||||
))
|
||||
.expect("DROP CONSTRAINT after a fresh rebuild — the CHECK metadata was reconstructed");
|
||||
}
|
||||
|
||||
// --- 4h: ALTER TABLE … RENAME TO (ADR-0035 §6) --------------------------
|
||||
|
||||
/// Path to a table's CSV in the project data dir.
|
||||
fn csv_path(project: &project::Project, table: &str) -> std::path::PathBuf {
|
||||
project
|
||||
.path()
|
||||
.join(project::DATA_DIR)
|
||||
.join(format!("{table}.csv"))
|
||||
}
|
||||
|
||||
/// Drop the current db handle, delete the `.db`, reopen, and rebuild from
|
||||
/// the text artifacts (`project.yaml` + CSVs) only — the FRESH rebuild
|
||||
/// that re-emits DDL from stored metadata via `schema_to_ddl`. This is
|
||||
/// where the CHECK-text drift (Finding-1) and the FK / metadata
|
||||
/// reconciliation actually round-trip, unlike an in-place rebuild whose
|
||||
/// wipe leaves the user tables untouched.
|
||||
fn fresh_rebuild(
|
||||
old: Database,
|
||||
project: &project::Project,
|
||||
r: &tokio::runtime::Runtime,
|
||||
) -> Database {
|
||||
use rdbms_playground::project::PLAYGROUND_DB;
|
||||
// Drop only the db handle (release the .db file) and reuse the live
|
||||
// `project` — re-opening the Project would re-acquire its lock file,
|
||||
// which the still-alive `project` already holds. The Database does not
|
||||
// hold the project lock; only the Project does.
|
||||
drop(old);
|
||||
std::fs::remove_file(project.path().join(PLAYGROUND_DB)).expect("remove db");
|
||||
let db = Database::open_with_persistence(
|
||||
project.db_path(),
|
||||
Persistence::new(project.path().to_path_buf()),
|
||||
)
|
||||
.expect("db");
|
||||
r.block_on(db.rebuild_from_text(project.path().to_path_buf(), None))
|
||||
.expect("rebuild");
|
||||
db
|
||||
}
|
||||
|
||||
fn table_names(db: &Database, r: &tokio::runtime::Runtime) -> Vec<String> {
|
||||
r.block_on(db.list_tables()).expect("list_tables")
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn e2e_rename_table_with_rows_csv_follows_and_survives_rebuild() {
|
||||
// The CSV file follows the rename (data/<new>.csv written, <old>.csv
|
||||
// removed), rows are intact including a NULL (NULL-vs-empty fidelity),
|
||||
// and the renamed table round-trips through a FRESH rebuild.
|
||||
let (project, db, _d) = open();
|
||||
let r = rt();
|
||||
std::fs::write(
|
||||
project.path().join("rn.commands"),
|
||||
"create table Orders with pk id(int)\n\
|
||||
add column Orders: note (text)\n\
|
||||
insert into Orders (id, note) values (1, 'first')\n\
|
||||
insert into Orders (id) values (2)\n\
|
||||
alter table Orders rename to Purchases\n",
|
||||
)
|
||||
.expect("write script");
|
||||
let events = r.block_on(run_replay(&db, project.path(), "rn.commands"));
|
||||
match events.last().expect("event") {
|
||||
AppEvent::ReplayCompleted { count, .. } => {
|
||||
assert_eq!(*count, 5, "all five lines replayed; events: {events:?}");
|
||||
}
|
||||
other => panic!("expected ReplayCompleted, got {other:?} (events: {events:?})"),
|
||||
}
|
||||
|
||||
let tables = table_names(&db, &r);
|
||||
assert!(
|
||||
tables.contains(&"Purchases".to_string()) && !tables.contains(&"Orders".to_string()),
|
||||
"the table is now Purchases, not Orders: {tables:?}"
|
||||
);
|
||||
assert!(csv_path(&project, "Purchases").exists(), "data/Purchases.csv written");
|
||||
assert!(!csv_path(&project, "Orders").exists(), "data/Orders.csv removed");
|
||||
|
||||
let rows = r
|
||||
.block_on(db.query_data("Purchases".to_string(), None, None, None))
|
||||
.expect("query")
|
||||
.rows;
|
||||
assert_eq!(rows.len(), 2);
|
||||
assert_eq!(rows[0][1].as_deref(), Some("first"));
|
||||
assert_eq!(rows[1][1], None, "the NULL note survived the rename");
|
||||
|
||||
// FRESH rebuild — the renamed table + its rows reconstruct from text.
|
||||
let db = fresh_rebuild(db, &project, &r);
|
||||
let tables = table_names(&db, &r);
|
||||
assert!(
|
||||
tables.contains(&"Purchases".to_string()) && !tables.contains(&"Orders".to_string()),
|
||||
"Purchases round-tripped through a fresh rebuild: {tables:?}"
|
||||
);
|
||||
let rows = r
|
||||
.block_on(db.query_data("Purchases".to_string(), None, None, None))
|
||||
.expect("query")
|
||||
.rows;
|
||||
assert_eq!(rows.len(), 2);
|
||||
assert_eq!(rows[1][1], None, "NULL preserved across the rebuild");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn e2e_rename_table_with_table_qualified_check_survives_fresh_rebuild() {
|
||||
// Finding-1 regression. A CHECK that qualifies a column with the table
|
||||
// name (`T.age`, and a table-level `T.lo < T.hi`) drifts on rename:
|
||||
// the engine rewrites the LIVE CHECK, but the STORED text would stay
|
||||
// `T.…` and break a FRESH rebuild (`schema_to_ddl` → "no such table
|
||||
// T"). The §2.9 rewrite keeps the stored text in step.
|
||||
let (project, db, _d) = open();
|
||||
let r = rt();
|
||||
std::fs::write(
|
||||
project.path().join("rn.commands"),
|
||||
"create table T (id integer primary key, age integer check (T.age > 0), lo integer, hi integer, check (T.lo < T.hi))\n\
|
||||
insert into T (id, age, lo, hi) values (1, 5, 1, 9)\n\
|
||||
alter table T rename to U\n",
|
||||
)
|
||||
.expect("write script");
|
||||
let events = r.block_on(run_replay(&db, project.path(), "rn.commands"));
|
||||
assert!(
|
||||
matches!(events.last(), Some(AppEvent::ReplayCompleted { .. })),
|
||||
"create with table-qualified CHECKs + rename replayed; events: {events:?}"
|
||||
);
|
||||
|
||||
// The live CHECKs still enforce under the new name.
|
||||
let bad_age = r.block_on(db.insert(
|
||||
"U".to_string(),
|
||||
Some(vec!["id".into(), "age".into(), "lo".into(), "hi".into()]),
|
||||
vec![
|
||||
Value::Number("2".into()),
|
||||
Value::Number("0".into()),
|
||||
Value::Number("1".into()),
|
||||
Value::Number("9".into()),
|
||||
],
|
||||
Some("i".into()),
|
||||
));
|
||||
assert!(bad_age.is_err(), "age > 0 still enforced after rename");
|
||||
|
||||
// The headline: a FRESH rebuild reconstructs from the stored CHECK
|
||||
// text — which must now reference U, not T — and still enforces.
|
||||
let db = fresh_rebuild(db, &project, &r);
|
||||
assert!(
|
||||
table_names(&db, &r).contains(&"U".to_string()),
|
||||
"U rebuilt from the text artifacts (would fail on 'no such table T' without the rewrite)"
|
||||
);
|
||||
let bad_after = r.block_on(db.insert(
|
||||
"U".to_string(),
|
||||
Some(vec!["id".into(), "age".into(), "lo".into(), "hi".into()]),
|
||||
vec![
|
||||
Value::Number("3".into()),
|
||||
Value::Number("-1".into()),
|
||||
Value::Number("1".into()),
|
||||
Value::Number("9".into()),
|
||||
],
|
||||
Some("i".into()),
|
||||
));
|
||||
assert!(bad_after.is_err(), "the rewritten CHECK enforces after a fresh rebuild");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn e2e_rename_fk_parent_updates_metadata_and_still_enforces() {
|
||||
// Renaming an FK *parent* updates the relationship's parent end; the
|
||||
// child FK still enforces, and the metadata is consistent enough that
|
||||
// a fresh rebuild (which re-emits the FK DDL from metadata) succeeds.
|
||||
let (project, db, _d) = open();
|
||||
let r = rt();
|
||||
std::fs::write(
|
||||
project.path().join("rn.commands"),
|
||||
"create table P with pk id(int)\n\
|
||||
create table C (id integer primary key, p_id integer references P(id))\n\
|
||||
insert into P (id) values (1)\n\
|
||||
alter table P rename to Parent\n",
|
||||
)
|
||||
.expect("write script");
|
||||
let events = r.block_on(run_replay(&db, project.path(), "rn.commands"));
|
||||
assert!(
|
||||
matches!(events.last(), Some(AppEvent::ReplayCompleted { .. })),
|
||||
"events: {events:?}"
|
||||
);
|
||||
|
||||
// The child's outbound relationship now points at the new parent name.
|
||||
let c = r.block_on(db.describe_table("C".to_string(), None)).expect("describe C");
|
||||
assert_eq!(c.outbound_relationships.len(), 1);
|
||||
assert_eq!(c.outbound_relationships[0].other_table, "Parent");
|
||||
|
||||
// FK still enforces: a child row referencing a missing parent fails.
|
||||
assert!(
|
||||
r.block_on(db.insert(
|
||||
"C".to_string(),
|
||||
Some(vec!["id".into(), "p_id".into()]),
|
||||
vec![Value::Number("9".into()), Value::Number("99".into())],
|
||||
Some("i".into()),
|
||||
))
|
||||
.is_err(),
|
||||
"FK to the renamed parent still enforces"
|
||||
);
|
||||
|
||||
// Fresh rebuild re-emits the FK from metadata (parent_table = Parent).
|
||||
let db = fresh_rebuild(db, &project, &r);
|
||||
let tables = table_names(&db, &r);
|
||||
assert!(tables.contains(&"Parent".to_string()) && tables.contains(&"C".to_string()));
|
||||
assert!(
|
||||
r.block_on(db.insert(
|
||||
"C".to_string(),
|
||||
Some(vec!["id".into(), "p_id".into()]),
|
||||
vec![Value::Number("8".into()), Value::Number("77".into())],
|
||||
Some("i".into()),
|
||||
))
|
||||
.is_err(),
|
||||
"FK still enforces after a fresh rebuild"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn e2e_rename_fk_child_updates_metadata_and_still_enforces() {
|
||||
// Renaming an FK *child* updates the relationship's child end.
|
||||
let (project, db, _d) = open();
|
||||
let r = rt();
|
||||
std::fs::write(
|
||||
project.path().join("rn.commands"),
|
||||
"create table P with pk id(int)\n\
|
||||
create table C (id integer primary key, p_id integer references P(id))\n\
|
||||
insert into P (id) values (1)\n\
|
||||
alter table C rename to Child\n",
|
||||
)
|
||||
.expect("write script");
|
||||
let events = r.block_on(run_replay(&db, project.path(), "rn.commands"));
|
||||
assert!(
|
||||
matches!(events.last(), Some(AppEvent::ReplayCompleted { .. })),
|
||||
"events: {events:?}"
|
||||
);
|
||||
|
||||
// The parent's inbound relationship now names the renamed child.
|
||||
let p = r.block_on(db.describe_table("P".to_string(), None)).expect("describe P");
|
||||
assert_eq!(p.inbound_relationships.len(), 1);
|
||||
assert_eq!(p.inbound_relationships[0].other_table, "Child");
|
||||
|
||||
// FK still enforces under the new child name; survives a fresh rebuild.
|
||||
assert!(
|
||||
r.block_on(db.insert(
|
||||
"Child".to_string(),
|
||||
Some(vec!["id".into(), "p_id".into()]),
|
||||
vec![Value::Number("9".into()), Value::Number("99".into())],
|
||||
Some("i".into()),
|
||||
))
|
||||
.is_err(),
|
||||
"FK from the renamed child still enforces"
|
||||
);
|
||||
let db = fresh_rebuild(db, &project, &r);
|
||||
assert!(table_names(&db, &r).contains(&"Child".to_string()));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn e2e_rename_self_referential_table_updates_both_ends() {
|
||||
// A self-referential FK has parent_table == child_table; both ends
|
||||
// must update on rename without a relationship-metadata PK conflict.
|
||||
let (project, db, _d) = open();
|
||||
let r = rt();
|
||||
std::fs::write(
|
||||
project.path().join("rn.commands"),
|
||||
"create table N (id integer primary key, parent_id integer references N(id))\n\
|
||||
insert into N (id, parent_id) values (1, null)\n\
|
||||
alter table N rename to Tree\n",
|
||||
)
|
||||
.expect("write script");
|
||||
let events = r.block_on(run_replay(&db, project.path(), "rn.commands"));
|
||||
assert!(
|
||||
matches!(events.last(), Some(AppEvent::ReplayCompleted { .. })),
|
||||
"events: {events:?}"
|
||||
);
|
||||
|
||||
// Both ends of the self-reference now name `Tree`.
|
||||
let t = r.block_on(db.describe_table("Tree".to_string(), None)).expect("describe Tree");
|
||||
assert_eq!(t.outbound_relationships[0].other_table, "Tree");
|
||||
assert_eq!(t.inbound_relationships[0].other_table, "Tree");
|
||||
|
||||
// The self-FK still enforces and survives a fresh rebuild.
|
||||
assert!(
|
||||
r.block_on(db.insert(
|
||||
"Tree".to_string(),
|
||||
Some(vec!["id".into(), "parent_id".into()]),
|
||||
vec![Value::Number("2".into()), Value::Number("99".into())],
|
||||
Some("i".into()),
|
||||
))
|
||||
.is_err(),
|
||||
"self-FK to a missing parent row is rejected"
|
||||
);
|
||||
let db = fresh_rebuild(db, &project, &r);
|
||||
assert!(table_names(&db, &r).contains(&"Tree".to_string()));
|
||||
// A valid self-reference (parent_id = 1, which exists) is accepted.
|
||||
r.block_on(db.insert(
|
||||
"Tree".to_string(),
|
||||
Some(vec!["id".into(), "parent_id".into()]),
|
||||
vec![Value::Number("3".into()), Value::Number("1".into())],
|
||||
Some("i".into()),
|
||||
))
|
||||
.expect("valid self-reference accepted after rebuild");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn e2e_rename_table_keeps_its_index_with_a_stale_name() {
|
||||
// Auto-named indexes embed the old table name and are left STALE on
|
||||
// rename (user decision); the index stays functional and survives a
|
||||
// fresh rebuild.
|
||||
let (project, db, _d) = open();
|
||||
let r = rt();
|
||||
std::fs::write(
|
||||
project.path().join("rn.commands"),
|
||||
"create table T with pk id(int)\n\
|
||||
add column T: email (text)\n\
|
||||
create index on T (email)\n\
|
||||
alter table T rename to Users\n",
|
||||
)
|
||||
.expect("write script");
|
||||
let events = r.block_on(run_replay(&db, project.path(), "rn.commands"));
|
||||
assert!(
|
||||
matches!(events.last(), Some(AppEvent::ReplayCompleted { .. })),
|
||||
"events: {events:?}"
|
||||
);
|
||||
|
||||
let u = r.block_on(db.describe_table("Users".to_string(), None)).expect("describe Users");
|
||||
assert_eq!(u.indexes.len(), 1, "the index followed the rename");
|
||||
assert_eq!(
|
||||
u.indexes[0].name, "T_email_idx",
|
||||
"the auto-name is left stale (embeds the old table name) per the user decision"
|
||||
);
|
||||
assert_eq!(u.indexes[0].columns, vec!["email".to_string()]);
|
||||
|
||||
// Survives a fresh rebuild (recreated from IndexSchema on table Users).
|
||||
let db = fresh_rebuild(db, &project, &r);
|
||||
let u = r.block_on(db.describe_table("Users".to_string(), None)).expect("describe Users");
|
||||
assert_eq!(u.indexes.len(), 1);
|
||||
assert_eq!(u.indexes[0].name, "T_email_idx");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn e2e_rename_table_is_one_undo_step() {
|
||||
// The rename is one user mutation = one whole-project snapshot = one
|
||||
// undo step. Undo restores the old name and its rows; redo reapplies.
|
||||
let (project, db, _d) = open_with_undo();
|
||||
let r = rt();
|
||||
std::fs::write(
|
||||
project.path().join("rn.commands"),
|
||||
"create table Orders with pk id(int)\n\
|
||||
insert into Orders (id) values (1)\n\
|
||||
alter table Orders rename to Purchases\n",
|
||||
)
|
||||
.expect("write script");
|
||||
r.block_on(run_replay(&db, project.path(), "rn.commands"));
|
||||
assert!(table_names(&db, &r).contains(&"Purchases".to_string()));
|
||||
|
||||
// One undo reverts the rename.
|
||||
assert!(r.block_on(db.undo()).expect("undo").is_some(), "rename was one undo step");
|
||||
let tables = table_names(&db, &r);
|
||||
assert!(
|
||||
tables.contains(&"Orders".to_string()) && !tables.contains(&"Purchases".to_string()),
|
||||
"undo restored the old table name: {tables:?}"
|
||||
);
|
||||
assert_eq!(
|
||||
r.block_on(db.query_data("Orders".to_string(), None, None, None)).expect("query").rows.len(),
|
||||
1,
|
||||
"the row is back under the old name"
|
||||
);
|
||||
|
||||
// Redo reapplies the rename.
|
||||
assert!(r.block_on(db.redo()).expect("redo").is_some());
|
||||
let tables = table_names(&db, &r);
|
||||
assert!(
|
||||
tables.contains(&"Purchases".to_string()) && !tables.contains(&"Orders".to_string()),
|
||||
"redo reapplied the rename: {tables:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn e2e_rename_table_refusals() {
|
||||
// The executor's guards: existing-target, same-name, non-existent
|
||||
// source, and an internal `__rdbms_*` target (defense in depth — the
|
||||
// parse validator also refuses it, but a synthesised command reaches
|
||||
// the worker directly).
|
||||
let (project, db, _d) = open();
|
||||
let r = rt();
|
||||
std::fs::write(
|
||||
project.path().join("setup.commands"),
|
||||
"create table T with pk id(int)\n\
|
||||
create table X with pk id(int)\n",
|
||||
)
|
||||
.expect("write");
|
||||
r.block_on(run_replay(&db, project.path(), "setup.commands"));
|
||||
|
||||
assert!(
|
||||
r.block_on(db.rename_table("T".into(), "X".into(), Some("rn".into()))).is_err(),
|
||||
"rename to an existing other table is refused"
|
||||
);
|
||||
assert!(
|
||||
r.block_on(db.rename_table("T".into(), "T".into(), Some("rn".into()))).is_err(),
|
||||
"rename to the same name is refused"
|
||||
);
|
||||
assert!(
|
||||
r.block_on(db.rename_table("Ghost".into(), "G".into(), Some("rn".into()))).is_err(),
|
||||
"rename of a non-existent table is refused"
|
||||
);
|
||||
assert!(
|
||||
r.block_on(db.rename_table("T".into(), "__rdbms_evil".into(), Some("rn".into()))).is_err(),
|
||||
"rename to an internal table name is refused at the executor"
|
||||
);
|
||||
|
||||
// Case-insensitive collisions are refused with engine-neutral wording
|
||||
// (not the raw engine "already another table" error) — the database
|
||||
// matches names case-insensitively (ADR-0035 §9).
|
||||
let case_only = r.block_on(db.rename_table("T".into(), "t".into(), Some("rn".into())));
|
||||
assert!(case_only.is_err(), "a case-only rename is refused");
|
||||
if let Err(e) = case_only {
|
||||
let msg = e.to_string();
|
||||
assert!(
|
||||
!msg.to_lowercase().contains("another table") && !msg.to_lowercase().contains("index"),
|
||||
"the refusal is engine-neutral, not a raw engine collision error: {msg}"
|
||||
);
|
||||
}
|
||||
assert!(
|
||||
r.block_on(db.rename_table("T".into(), "x".into(), Some("rn".into()))).is_err(),
|
||||
"rename to a name colliding case-insensitively with another table (X) is refused"
|
||||
);
|
||||
|
||||
// The failed renames left the schema untouched.
|
||||
let tables = table_names(&db, &r);
|
||||
assert!(tables.contains(&"T".to_string()) && tables.contains(&"X".to_string()));
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user