docs: session handoff 41 — ADR-0035 4h + case-insensitive table-name fix; 4i next
This commit is contained in:
@@ -0,0 +1,215 @@
|
||||
# Session handoff — 2026-05-26 (41)
|
||||
|
||||
Forty-first handover. This session **shipped ADR-0035 Phase-4 sub-phase
|
||||
4h — `ALTER TABLE … RENAME TO`** (planned Phase 1→3 with a planning
|
||||
`/runda` and a finished-slice `/runda`, both of which found real bugs)
|
||||
**plus a follow-up cross-cutting fix: case-insensitive table-name
|
||||
resolution across all executors** (a drift / silent-data-loss bug the
|
||||
user asked to make safe). The next job is **4i — the verification
|
||||
sweep**, which **completes ADR-0035 Phase 4**.
|
||||
|
||||
## §1. State at handoff
|
||||
|
||||
**Branch:** `main`. **HEAD `a95c807`.** **Tests: 1909 passing, 0 failing,
|
||||
0 skipped, 1 ignored** (the unchanged `friendly/mod.rs` ` ```ignore `
|
||||
doctest). **Clippy:** clean (`cargo clippy --all-targets -- -D warnings`).
|
||||
|
||||
**This session's commits** (oldest → newest, local):
|
||||
|
||||
```
|
||||
f7e77a8 feat: ADR-0035 4h — ALTER TABLE … RENAME TO
|
||||
a95c807 fix: resolve table names case-insensitively across all executors
|
||||
```
|
||||
|
||||
**Git state.** `6112859` (handoff-40 docs), `f7e77a8` (4h), `a95c807`
|
||||
(case-safety) are **unpushed** — a normal working state. Push is the
|
||||
user's step; nothing blocks. Working tree clean.
|
||||
|
||||
## §2. What shipped this session
|
||||
|
||||
- **4h — `ALTER TABLE <old> RENAME TO <new>`** (`f7e77a8`). The one
|
||||
genuinely new low-level op in Phase 4: a new `do_rename_table` — native
|
||||
engine `RENAME TO` (structure-preserving, no rebuild) + one-transaction
|
||||
(commit-db-last) reconciliation of everything the engine doesn't track:
|
||||
every metadata row naming the table (`__rdbms_playground_columns`,
|
||||
**both ends** of `__rdbms_playground_relationships`,
|
||||
`__rdbms_playground_table_checks`), the CSV file (via the existing
|
||||
`finalize_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 — `rewrite_check_table_qualifier`,
|
||||
extending the 4e tokenizer). 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 §6.1 trap-safe pattern); the new-name slot
|
||||
mirrors the `CREATE TABLE` name slot (`IdentSource::NewName` +
|
||||
`reject_internal_table` validator). Refusals (engine-neutral,
|
||||
case-insensitive): same-name, case-only, existing-target, `__rdbms_*`,
|
||||
non-existent. Auto-named indexes *and* relationships keep their stale
|
||||
names (only table-name *columns* update — §6 scope; user-confirmed).
|
||||
One undo step; advanced-mode only; closes the rename half of `C1`. Plan:
|
||||
`docs/plans/20260526-adr-0035-sql-ddl-4h.md`.
|
||||
|
||||
**The two `/runda` passes both paid off** (probe, don't reason):
|
||||
- *Planning pass (blocking):* the CHECK-text drift — the engine rewrites
|
||||
a table-qualified column ref in the *live* CHECK on rename, but the
|
||||
*stored* text would stay `T.age` and break a fresh rebuild. Confirmed
|
||||
empirically (SQLite 3.48); resolved by the rewrite (§2.9 of the plan).
|
||||
Headline test: `e2e_rename_table_with_table_qualified_check_survives_
|
||||
fresh_rebuild`.
|
||||
- *Finished-slice pass (engine-neutrality leak):* byte-exact collision
|
||||
guards let a case-only rename / case-insensitive clash surface the raw
|
||||
engine "there is already another table" error; fixed with
|
||||
case-insensitive pre-checks + a dedicated case-only message.
|
||||
|
||||
- **Case-insensitive table-name resolution** (`a95c807`). SQL identifiers
|
||||
are case-insensitive (the engine resolves any capitalization), but the
|
||||
metadata tables (`table_name` / `parent_table` / `child_table`) and the
|
||||
`data/<table>.csv` files used case-sensitive TEXT `=` — so an op naming a
|
||||
table in a different case **drifted**: schema ops orphaned metadata, and
|
||||
a wrong-case `insert`/`update`/`delete` **silently skipped the CSV write
|
||||
→ the change was lost on the next reload/rebuild**. This contradicted
|
||||
ADR-0009's stated rule (case-insensitive resolution, case-preserving
|
||||
display) — a bug, not a new decision. Fix: a `canonical_table_name(conn,
|
||||
name)` helper (resolve to stored case via `COLLATE NOCASE`, excluding
|
||||
`sqlite_*` **and** `__rdbms_*`) applied at the entry of **every
|
||||
table-naming executor** (drop table, add/drop/rename column, change
|
||||
column type, add/drop constraint, add relationship, add index, rename
|
||||
table, insert/update/delete, and the advanced SQL DML), shadowing the
|
||||
table name with its canonical form so metadata + native SQL + CSV stay
|
||||
in step. The internal-`__rdbms_*` guard is folded into the same lookup
|
||||
(executors that previously lacked it now refuse internal/`sqlite_*` as
|
||||
"no such table"). `do_rename_table` now accepts a case-variant source
|
||||
too. **Columns stay strict** (wrong-case column → "no such column" —
|
||||
safe, never drifting; user-confirmed scope). Tests:
|
||||
`tests/case_insensitive_names.rs` (6, all with fresh-rebuild proofs).
|
||||
ADR-0009 gained a clarifying sentence (the canonicalization mechanism).
|
||||
|
||||
ADR-0035 stays **Accepted**; README + `requirements.md` Q1/C1 + ADR §13
|
||||
(4h) + Status updated. ADR-0009 clarified.
|
||||
|
||||
## §3. The NEXT job — 4i (the verification sweep, completes Phase 4)
|
||||
|
||||
Canonical list: **ADR-0035 §13 4i**. With 4h's own help/usage now in
|
||||
place, the running deferral tail is:
|
||||
|
||||
- **(a) Refresh the `CREATE TABLE` help/usage skeleton** for the 4a.2
|
||||
`DEFAULT`/`CHECK`/composite-`UNIQUE`, 4a.3 table-`CHECK`, and 4b FK forms
|
||||
(deferred from each). The 4d/4e/4f/4g/4h forms already carry their own
|
||||
help/usage — so the *CREATE TABLE* skeleton is the only help/usage debt.
|
||||
- **(b) `describe` display of table-level constraints** — composite
|
||||
`UNIQUE` + table `CHECK` (incl. **named** CHECKs from 4g). The
|
||||
unique-*index* `[unique]` marker shipped in 4d; only the table-level
|
||||
*constraint* display remains.
|
||||
- **(c) 4b self-ref FK pre-submit indicator** — a `CREATE TABLE` with a
|
||||
self-referencing FK (`references <self>`) parses + executes fine, but the
|
||||
pre-submit schema-existence diagnostic false-flags the not-yet-created
|
||||
self table as unknown. Treat a FK parent equal to the `CREATE TABLE`
|
||||
target as valid.
|
||||
- **(d) shared-entry-word completion merge** — in advanced mode a shared
|
||||
entry word (`create`/`drop`) surfaces only the SQL node's continuations,
|
||||
so `drop ` offers only `table` (not the DSL `column`/`relationship`/
|
||||
`index`/`constraint`) and a partial like `drop rel` returns empty. Merge
|
||||
the expected sets of all candidate nodes. 4d widened this (two advanced
|
||||
nodes each for `create`/`drop`).
|
||||
- **(e) user discussion flag (2026-05-25):** before/with (d), discuss
|
||||
**visually distinguishing simple- vs advanced-mode completions in the
|
||||
hint UI** (likely colour) — a UX design conversation.
|
||||
- Plus the §4i staples: typing-surface + matrix coverage, engine-neutral
|
||||
error pass, undo-parity (one step per statement).
|
||||
|
||||
**Sequencing note:** (a)/(b)/(c) are mechanical and well-scoped; (d)/(e)
|
||||
involve a UX design conversation with the user (escalate before building).
|
||||
A finished-slice `/runda` covering 4i is worth budgeting (the last two
|
||||
both found real bugs).
|
||||
|
||||
## §4. Tracked follow-ups (not lost)
|
||||
|
||||
**New this session:**
|
||||
- **Case-only table rename refusal (4h, autonomous decision).** `alter
|
||||
table T rename to t` is refused ("differ only in capitalization …
|
||||
nothing to rename"), decided as an extension of the user's same-name
|
||||
refusal. The engine can't do a case-only rename via `RENAME TO`
|
||||
directly; supporting it would need a two-step temp-name dance. Flagged
|
||||
to the user; left as refuse unless they ask otherwise.
|
||||
- **Column-name case strictness (case-safety fix).** Table names are now
|
||||
resolved case-insensitively; **column names are still matched
|
||||
case-sensitively** (wrong case → "no such column"). Safe (never
|
||||
drifts), but not SQL-case-insensitive like tables. A separate
|
||||
UX-consistency change if the user wants it (would need column-name
|
||||
canonicalization across the column ops).
|
||||
|
||||
**Carried:**
|
||||
- **H1 wording — `--create-fk` leak (4g).** `ALTER … ADD FOREIGN KEY` on
|
||||
a missing child column reuses `do_add_relationship`'s error, which
|
||||
mentions the DSL flag `--create-fk` — off-key in SQL. Fix the wording
|
||||
for the SQL path. H1.
|
||||
- **Coverage gap — pre-4g 3-column `CHECK_TABLE` migration.** The
|
||||
rebuild-time `ALTER … ADD COLUMN name` migration + the named-CHECK-on-
|
||||
old-project refusal are sound + guarded but the genuinely-old 3-column
|
||||
*in-place* path is untested (can't fabricate a 3-col table through the
|
||||
public API). The common case (old project → fresh rebuild → 4-col) IS
|
||||
tested. A raw-SQL test helper or fixture `.db` would close it.
|
||||
- **H1 friendly wording** for the 4e CHECK-guard refusals and the 4f
|
||||
type-conversion diagnostics (engine-neutral but terse).
|
||||
- The §8 items from handoff-39: app-lifecycle-command runtime-failure
|
||||
journalling; M4 execution-time mode side-channel; `blob` value literal;
|
||||
CI/TT5; DSL→SQL teaching echo (ADR-0030 Phase 5).
|
||||
|
||||
## §5. Patterns the implementer must not forget
|
||||
|
||||
1. **Walker `Choice` does not backtrack between same-leading-keyword
|
||||
branches.** Keep **one branch per leading verb**, fan out on a
|
||||
**distinct** second token via an inner `Choice` (4g `add`/`drop`, 4h
|
||||
`rename`). Bit 4g and shaped 4h.
|
||||
2. **`do_rebuild_from_text` reconstructs ALL metadata from yaml.** A new
|
||||
metadata table/column MUST be added to both the wipe and the
|
||||
repopulation, or a *fresh* rebuild silently drops it. Probe the
|
||||
*fresh*-rebuild path, not just in-place (the `50a889e` bug, and 4h's
|
||||
CHECK-text drift, both lived there).
|
||||
3. **Canonicalize a user-supplied table name to its stored case** at the
|
||||
entry of any executor that touches metadata or CSV by name
|
||||
(`canonical_table_name`, `db.rs`). The engine is case-insensitive; our
|
||||
metadata/CSV are case-sensitive. Any NEW table-naming executor must do
|
||||
this or it will drift (the exact `a95c807` bug class).
|
||||
4. **Exhaustive matches:** a new `AlterTableAction` variant forces arms in
|
||||
`runtime.rs`, `app.rs` (`build_translate_context`), and a new
|
||||
`Operation` variant forces `Operation::keyword()`. The compiler finds
|
||||
them.
|
||||
5. **Two DDL generators stay in sync** (`do_create_table` /
|
||||
`schema_to_ddl`); **catalog lockstep + vocab audit** — keep new
|
||||
help/usage engine-neutral (`engine_vocabulary_audit` enforces it).
|
||||
6. **Probe, don't reason.** Both 4h `/runda` passes found real bugs by
|
||||
testing the engine's actual behaviour (the CHECK rewrite; the
|
||||
case-collision leak) rather than reasoning about it.
|
||||
|
||||
## §6. Process pins (unchanged, still binding)
|
||||
|
||||
- **Confirm every commit.** Propose the message; wait for the go-ahead.
|
||||
No AI attribution. (Both this session's commits were user-approved.)
|
||||
- **Push is the user's step.** Never push. Unpushed commits are normal.
|
||||
- **Escalate ambiguity / scope.** 4h's open calls (same-name, index/rel
|
||||
staleness, CHECK-drift handling) and the case-safety scope (all paths;
|
||||
table-only vs +columns) were all surfaced + user-decided before
|
||||
building. Do the same for 4i's (d)/(e).
|
||||
- **DA hat each slice; `/runda` at the planning exit AND on the finished
|
||||
slice.** Both paid off this session. Budget at least one finished-slice
|
||||
`/runda` for 4i.
|
||||
- **Keep docs lockstep** — ADR status/§13 + README + `requirements.md` in
|
||||
the same edit.
|
||||
|
||||
## §7. How to take over
|
||||
|
||||
1. **Read, in order:** this file →
|
||||
`docs/adr/0035-advanced-mode-sql-ddl.md` (§13 4i is the spec) →
|
||||
`CLAUDE.md` → `docs/requirements.md` (`Q1`, `C1`). The 4h plan
|
||||
(`docs/plans/20260526-adr-0035-sql-ddl-4h.md`) is a good model.
|
||||
2. **Baseline:**
|
||||
```
|
||||
cargo test # expect 1909 passing / 0 failing / 0 skipped / 1 ignored
|
||||
cargo clippy --all-targets -- -D warnings # clean
|
||||
```
|
||||
3. **4i** (§3): start with the mechanical, well-scoped items (a)/(b)/(c)
|
||||
test-first; **escalate the (d)/(e) completion-UX conversation to the
|
||||
user before building**; finish with the §4i staples + a finished-slice
|
||||
`/runda`. 4i completes ADR-0035 Phase 4.
|
||||
Reference in New Issue
Block a user