feat: ADR-0035 4d — CREATE [UNIQUE] INDEX / DROP INDEX
Advanced-mode SQL CREATE [UNIQUE] INDEX [IF NOT EXISTS] [<name>] ON <T> (cols) -> SqlCreateIndex and DROP INDEX [IF EXISTS] <name> -> SqlDropIndex, both reusing the ADR-0025 executors (do_add_index / do_drop_index), like 4c reused do_drop_table. - CREATE UNIQUE INDEX admitted in advanced mode (ADR-0025 Amendment 1): ADR-0025 deferred UNIQUE indexes for the simple-mode DSL, but advanced mode trusts the user like SQL does. Adds an additive IndexSchema.unique flag (project.yaml, serde-default, version stays 1); rebuild re-emits CREATE UNIQUE INDEX; the redundant-set guard keys on (columns, unique). Simple-mode `add unique index` stays deferred. - IF [NOT] EXISTS on both forms reuses the 4c no-op-with-note skip (journalled, not snapshotted) via CreateIndexOutcome / DropIndexOutcome. - Unnamed CREATE INDEX auto-named (ADR-0025 convention); the [UNIQUE] prefix is a concrete-keyword Choice and the optional name an on-led-first selector (the drop-index selector precedent) — trap-safe. - create/drop each gain a second advanced node; the existing all-candidates dispatch handles it (locked by parse tests). - Unique indexes marked [unique] in the structure view and items panel. - do_add_index refuses internal __rdbms_* tables as "no such table", closing a latent exposure on both the simple `add index` and the new SQL CREATE INDEX surfaces (ADR-0025 Amendment 1). Docs: ADR-0035 status + §13 4d + 4i; ADR-0025 Amendment 1; ADR README; requirements.md Q1/C3. Plan: docs/plans/20260525-adr-0035-sql-ddl-4d.md. Tests: 1834 passing / 0 failing / 0 skipped / 1 ignored; clippy clean.
This commit is contained in:
claude@clouddev1
@@ -0,0 +1,439 @@
|
||||
# Plan: ADR-0035 Phase 4, sub-phase 4d — `CREATE [UNIQUE] INDEX` / `DROP INDEX`
|
||||
|
||||
Add advanced-mode SQL `CREATE [UNIQUE] INDEX [IF NOT EXISTS] [<name>] ON
|
||||
<table> (<col>, …)` → `SqlCreateIndex` and `DROP INDEX [IF EXISTS]
|
||||
<name>` → `SqlDropIndex` (ADR-0035 §1/§4/§13). Both reuse the existing
|
||||
ADR-0025 low-level executors (`do_add_index` / `do_drop_index`), like 4c
|
||||
reused `do_drop_table`. Two genuinely new things beyond 4c's pattern:
|
||||
|
||||
1. **`UNIQUE` index → an `IndexSchema.unique` model extension** (the
|
||||
escalation; user-approved 2026-05-25). ADR-0025 deferred UNIQUE
|
||||
indexes for the *simple-mode DSL* (`add unique index`); ADR-0035 §4
|
||||
supersedes that for the *advanced* SQL surface. Simple-mode
|
||||
`add unique index` stays deferred. An ADR-0025 **Amendment** records
|
||||
this.
|
||||
2. **A second advanced node per entry word.** `create` gains
|
||||
`SQL_CREATE_INDEX` alongside `SQL_CREATE_TABLE`; `drop` gains
|
||||
`SQL_DROP_INDEX` alongside `SQL_DROP_TABLE`. The dispatcher already
|
||||
tries *all* advanced candidates (`decide`: `advanced.chain(simple)` +
|
||||
the first-full-match loop), so no dispatch change is needed — but a
|
||||
parse test locks it.
|
||||
|
||||
`IF [NOT] EXISTS` on **both** forms (user-approved 2026-05-25), reusing
|
||||
the 4c no-op-with-note skip path.
|
||||
|
||||
## 1. Baseline
|
||||
|
||||
- Tests: **1805 passing, 0 failing, 0 skipped, 1 ignored** (the
|
||||
`friendly/mod.rs` ```ignore``` doctest); clippy clean. Branch `main`,
|
||||
HEAD `e52e90c` (4c), 3 local-only commits since `origin/main`. 4d
|
||||
starts here.
|
||||
|
||||
## 2. Decisions (settled — escalated + user-confirmed 2026-05-25)
|
||||
|
||||
1. **`CREATE UNIQUE INDEX` is in; the model extension lands now.**
|
||||
`IndexSchema` gains `unique: bool`. ADR-0025's UNIQUE-index exclusion
|
||||
was a statement about the *simple-mode DSL teaching surface* (the only
|
||||
surface that existed on 2026-05-16); advanced mode "trusts the user
|
||||
like SQL does" (ADR-0035 §7), so the concept-separation argument does
|
||||
not transfer. The constraint track it deferred *to* (ADR-0018 → 0029 →
|
||||
0035 4a.2) has since shipped. ADR-0025 gets an Amendment; **simple-mode
|
||||
`add unique index` stays deferred** (not in this slice).
|
||||
2. **`IF [NOT] EXISTS` on both forms** — universal cross-vendor idiom
|
||||
(ADR-0035 §4 reclassification), reusing the 4c
|
||||
`CreateOutcome`/`DropOutcome`-style no-op-with-note. Skip is
|
||||
**journalled, not snapshotted**.
|
||||
3. **Reuse the ADR-0025 executors.** `do_add_index` / `do_drop_index` are
|
||||
the single executors; the SQL path differs only at the `unique` flag
|
||||
and the `IF [NOT] EXISTS` skip branch. No fork.
|
||||
4. **`DROP INDEX <name>` is name-only.** SQL has no positional
|
||||
`on T (cols)` drop form (that is the simple DSL `drop index on …`,
|
||||
which keeps falling back to the simple `drop` node). `SqlDropIndex`
|
||||
carries `{ name, if_exists }`; the worker builds
|
||||
`IndexSelector::Named { name }` for `do_drop_index`.
|
||||
5. **Unnamed `CREATE INDEX` is supported and auto-named** (ADR-0035 §4
|
||||
`[<name>]`; the ADR-0025 `<T>_<cols>_idx` convention). A playground
|
||||
convenience over SQLite (which requires a name). See §4.1 for the
|
||||
grammar-disambiguation risk this carries.
|
||||
|
||||
## 3. Phase 1 — Requirements checklist (4d)
|
||||
|
||||
Grammar / parse:
|
||||
- [ ] `CREATE INDEX <name> ON <T> (<cols>)` parses in advanced mode →
|
||||
`SqlCreateIndex { unique: false, if_not_exists: false }`.
|
||||
- [ ] `CREATE UNIQUE INDEX <name> ON …` sets `unique: true`.
|
||||
- [ ] `CREATE INDEX ON <T> (<cols>)` (no name) parses → `name: None`
|
||||
(auto-named at execution). The optional name must **not** swallow `on`.
|
||||
- [ ] `CREATE [UNIQUE] INDEX IF NOT EXISTS <name> ON …` sets
|
||||
`if_not_exists: true`.
|
||||
- [ ] `DROP INDEX <name>` → `SqlDropIndex { if_exists: false }`;
|
||||
`DROP INDEX IF EXISTS <name>` → `if_exists: true`.
|
||||
- [ ] Trailing `;` tolerated on both (ADR-0035 §12).
|
||||
- [ ] **Dispatch (second advanced node):** in advanced mode
|
||||
`create table …` → `SqlCreateTable`, `create [unique] index …` →
|
||||
`SqlCreateIndex`; `drop table …` → `SqlDropTable`, `drop index …` →
|
||||
`SqlDropIndex`. Simple `add index` / `drop index on T(…)` /
|
||||
`drop column` etc. still parse (fallback).
|
||||
|
||||
Execution / model:
|
||||
- [ ] `CREATE INDEX` creates a plain index (parity with `add index`);
|
||||
one undo step; `undo` removes it.
|
||||
- [ ] `CREATE UNIQUE INDEX` creates a unique index; it round-trips
|
||||
through `project.yaml` (the `unique` flag) and survives `rebuild`
|
||||
(re-emitted as `CREATE UNIQUE INDEX`); enforces uniqueness (a duplicate
|
||||
insert is refused by the engine).
|
||||
- [ ] `DROP INDEX` removes the index; one undo step; `undo` restores it;
|
||||
the affected table's structure is auto-shown (ADR-0014).
|
||||
- [ ] `IF NOT EXISTS` on an existing index name → success + note, no
|
||||
error, no snapshot, journalled. `IF EXISTS` on an absent index →
|
||||
ditto.
|
||||
- [ ] Plain `CREATE INDEX` on a duplicate name / `DROP INDEX` on an
|
||||
unknown name → the existing friendly errors (unchanged).
|
||||
- [ ] Redundant-column-set guard refined for uniqueness: a plain and a
|
||||
unique index over the same columns are **not** mutual duplicates
|
||||
(different semantics); an exact duplicate (same columns **and** same
|
||||
uniqueness) is still refused. *(Test must use **explicit distinct
|
||||
names** for the plain+unique pair — the auto-name `<T>_<cols>_idx` is
|
||||
identical for both, so an unnamed second would collide on the name
|
||||
guard regardless. DA finding D.)*
|
||||
- [ ] `CREATE UNIQUE INDEX` on a column that **already holds duplicate
|
||||
values** is refused by the engine at creation; the failure routes
|
||||
through the friendly layer, engine-neutral. *(Test.)*
|
||||
- [ ] `IF NOT EXISTS` only short-circuits a **name** collision into a
|
||||
skip; a *different*-named but redundant-column-set create still hits
|
||||
the ADR-0025 redundant-set refusal (the playground's pedagogical guard,
|
||||
not raw-SQL semantics). *(Test + one-line doc note. DA finding H.)*
|
||||
- [ ] Errors/notes engine-neutral.
|
||||
|
||||
Persistence round-trip:
|
||||
- [ ] `IndexSchema.unique` round-trips: save → YAML `unique: true` →
|
||||
load → identical snapshot. Older project files (no `unique` field)
|
||||
default to `false` (`#[serde(default)]`); `version` stays `1`.
|
||||
- [ ] The rebuild-table primitive preserves a unique index's uniqueness
|
||||
(it re-creates captured indexes; `IndexInfo.unique` already read —
|
||||
verify the re-emit honours it).
|
||||
|
||||
Display (unique marker — user-confirmed for 4d):
|
||||
- [ ] Structure view marks a unique index — `… (cols) [unique]` — and
|
||||
leaves a plain index unmarked.
|
||||
- [ ] Items list (left panel) marks a unique index name.
|
||||
|
||||
### Testing
|
||||
- [ ] **Tier 1** (in-crate `sql_create_index_tests` / `sql_drop_index_tests`
|
||||
in `ddl.rs`, mirroring `sql_drop_table_tests`): the parse + flag cases
|
||||
above + the dispatch (second-advanced-node) cases.
|
||||
- [ ] **Tier 3** (`tests/sql_create_index.rs`, `tests/sql_drop_index.rs`):
|
||||
create plain/unique, the YAML round-trip + rebuild survival of a unique
|
||||
index, uniqueness enforcement, `IF [NOT] EXISTS` skip + journal, plain
|
||||
duplicate/unknown errors, one undo step + restore, drop auto-show.
|
||||
- [ ] **Tier 2** (insta): a `project.yaml` snapshot carrying a
|
||||
`unique: true` index, if an existing yaml snapshot test covers indexes
|
||||
(extend it; else a focused new one).
|
||||
- [ ] **Catalog** lockstep + vocab audit for the new note + help/usage
|
||||
keys.
|
||||
|
||||
## 4. Architecture & design
|
||||
|
||||
### 4.1 Grammar (`src/dsl/grammar/ddl.rs`)
|
||||
|
||||
**`DROP INDEX` (easy, mirrors `SQL_DROP_TABLE`).**
|
||||
- `SQL_DROP_INDEX_SHAPE = Seq[ Word("index"), SQL_DROP_IF_EXISTS_OPT,
|
||||
INDEX_NAME_EXISTING, Optional(Punct(';')) ]`. Reuse the existing
|
||||
`SQL_DROP_IF_EXISTS_OPT` and `INDEX_NAME_EXISTING` nodes.
|
||||
- `pub static SQL_DROP_INDEX: CommandNode { entry: "drop", shape,
|
||||
ast_builder: build_sql_drop_index, help_id, usage_ids }`.
|
||||
- `build_sql_drop_index`: `name = require_ident("index_name")`,
|
||||
`if_exists = path.contains_word("if")`.
|
||||
|
||||
**`CREATE INDEX`.** The shape, after the `create` entry word, must lead
|
||||
with a concrete keyword (the leading-`Optional` trap, handoff §3 /
|
||||
pattern 5). Both sub-problems are **settled by existing precedent** (DA
|
||||
pass verified against the walker code, not reasoned):
|
||||
|
||||
- *`[UNIQUE]` optional prefix.* Lead with a `Choice` whose every branch
|
||||
starts on a concrete keyword: `UNIQUE_INDEX_LEAD = Choice[
|
||||
Seq[Word("unique"), Word("index")], Word("index") ]`. A leading
|
||||
**`Choice`** of concrete-keyword branches is exactly the sanctioned
|
||||
pattern (the §3 rule forbids a leading *`Optional`*, not a `Choice`);
|
||||
`unique` presence is read in the builder via
|
||||
`path.contains_word("unique")`. Smoke-probe only.
|
||||
- *Optional `[<name>]` before `on` — use the `DI_SELECTOR` precedent, not
|
||||
a bare `Optional`.* **Verified:** `walk_ident` → `consume_ident` does
|
||||
**not** reject reserved keywords, so a bare `Optional(<name ident>)`
|
||||
*would* greedily eat the `on` keyword and break the unnamed form. The
|
||||
drop-index selector already solves this exact shape: a `Choice` with
|
||||
the **`on`-led branch first**, relying on `Choice` backtracking. Mirror
|
||||
it:
|
||||
- `CI_UNNAMED = Seq[ Word("on"), TABLE_NAME_EXISTING, Punct('('),
|
||||
INDEX_COLUMN_LIST, Punct(')') ]`
|
||||
- `CI_NAMED = Seq[ INDEX_NAME_NEW, Word("on"), TABLE_NAME_EXISTING,
|
||||
Punct('('), INDEX_COLUMN_LIST, Punct(')') ]`
|
||||
- `CI_SELECTOR = Choice[ CI_UNNAMED, CI_NAMED ]` (unnamed first, like
|
||||
`DI_SELECTOR`'s `DI_POSITIONAL`-first). `create index on T (c)` →
|
||||
`CI_UNNAMED` (no name-slot to eat `on`); `create index ix on T (c)` →
|
||||
`CI_UNNAMED` fails at `Word(on)` vs `ix`, backtracks to `CI_NAMED`.
|
||||
- Full shape: `Seq[ UNIQUE_INDEX_LEAD,
|
||||
SQL_CREATE_INDEX_IF_NOT_EXISTS_OPT, CI_SELECTOR, Optional(Punct(';')) ]`.
|
||||
The `IF NOT EXISTS` opt is mid-`Seq` (not leading) — exactly like 4c's
|
||||
`SQL_DROP_TABLE` `IF EXISTS` opt, so it is trap-safe.
|
||||
- `build_sql_create_index`: `unique = contains_word("unique")`,
|
||||
`if_not_exists = contains_word("if")`, `name = optional_ident(
|
||||
"index_name")` (present iff the `CI_NAMED` branch matched),
|
||||
`table = require_ident("table_name")`, `columns` from the list.
|
||||
|
||||
The single-node form is now the *primary* approach (no probe gamble): the
|
||||
two-node `TABLE_FK`-style split is **not needed** — kept only as a
|
||||
contingency if the smoke-probe of the leading `Choice` surprises.
|
||||
|
||||
**REGISTRY (`mod.rs`):** add `(&ddl::SQL_CREATE_INDEX,
|
||||
CommandCategory::Advanced)` and `(&ddl::SQL_DROP_INDEX,
|
||||
CommandCategory::Advanced)`. Verify `decide` tries all advanced
|
||||
candidates (it does — locked by a parse test).
|
||||
|
||||
### 4.2 Command (`src/dsl/command.rs`)
|
||||
- `SqlCreateIndex { name: Option<String>, table: String, columns:
|
||||
Vec<String>, unique: bool, if_not_exists: bool }` — verb
|
||||
`"create index"`, `target_table()` → `table`.
|
||||
- `SqlDropIndex { name: String, if_exists: bool }` — verb
|
||||
`"drop index"`, `target_table()` → `name` (the index name is the
|
||||
identifying thing for logging; mirrors `DropIndex`/`SqlDropTable`).
|
||||
- Exhaustive-match fallout (compiler-found): `verb`, `target_table` in
|
||||
`command.rs`; `app.rs` failure-translate; `runtime.rs` dispatch +
|
||||
outcome→event; `tests/typing_surface/mod.rs` `command_label` arms
|
||||
(`SqlCreateIndex`/`SqlDropIndex`, alongside the existing
|
||||
`SqlCreateTable`/`SqlDropTable`). Add the SQL index forms to the
|
||||
`tests/typing_surface/index_ops.rs` matrix (+ insta snaps) next to the
|
||||
DSL `add index`/`drop index` cases.
|
||||
- **`IndexSchema` struct-literal fallout** (compiler-found, the new
|
||||
`unique` field): db.rs:2414, yaml.rs:299, the yaml round-trip test
|
||||
(~478) — each adds `unique: …`.
|
||||
|
||||
### 4.3 Model extension — `IndexSchema.unique` (`src/persistence/`, `src/db.rs`)
|
||||
- `persistence/mod.rs`: `IndexSchema` gains `pub unique: bool` (doc-note
|
||||
mirroring `TableSchema.unique_constraints`: additive, defaults `false`
|
||||
for older files, `version` stays `1`).
|
||||
- `db.rs read_schema_snapshot` (~2414): carry `idx.unique` (already read
|
||||
into `IndexInfo` by `read_table_indexes`).
|
||||
- `yaml.rs`: the raw deserialize struct gains `#[serde(default)] unique:
|
||||
bool`; the reader (~299) carries it; `write_index` (~66) emits
|
||||
`unique: true` **only when true** (omit when false — keeps existing
|
||||
project files byte-stable and matches the minimal-noise house style;
|
||||
confirm against any golden-yaml test).
|
||||
- `db.rs rebuild_from_text` (~7814): emit `CREATE UNIQUE INDEX` when
|
||||
`index.unique`, else `CREATE INDEX`.
|
||||
- `do_add_index` gains `unique: bool`: emit `CREATE UNIQUE INDEX` when
|
||||
set; **refine the redundant-set guard** to compare `(columns, unique)`
|
||||
so a plain vs unique index over the same columns is not a false
|
||||
duplicate. Simple `add index` passes `unique: false` (call-site
|
||||
update in the worker arm).
|
||||
|
||||
### 4.4 Worker (`src/db.rs`)
|
||||
- `Request::SqlCreateIndex { name, table, columns, unique, if_not_exists,
|
||||
source, reply: oneshot<Result<CreateIndexOutcome>> }` +
|
||||
`db.sql_create_index(...)`.
|
||||
- Skip branch: resolve the index name (given, or the `<T>_<cols>_idx`
|
||||
auto-name — extract a tiny `resolve_index_name` helper shared with
|
||||
`do_add_index`), and if `if_not_exists && index_name_exists(resolved)`
|
||||
→ journal the line (no snapshot), reply `CreateIndexOutcome::Skipped(
|
||||
resolved)`. Else `snapshot_then(do_add_index(..., unique) →
|
||||
Created(desc))`.
|
||||
- `Request::SqlDropIndex { name, if_exists, source, reply:
|
||||
oneshot<Result<DropIndexOutcome>> }` + `db.sql_drop_index(...)`.
|
||||
- Skip branch: if `if_exists && !index_name_exists(name)` → journal (no
|
||||
snapshot), reply `Skipped`. Else `snapshot_then(do_drop_index(
|
||||
IndexSelector::Named { name }) → Dropped(desc))`.
|
||||
- New outcome enums (the index peers of `CreateOutcome`/`DropOutcome`):
|
||||
- `CreateIndexOutcome { Created(TableDescription), Skipped(String) }`
|
||||
(skip carries the **resolved** name — the auto-name is unknown to the
|
||||
command for the unnamed form).
|
||||
- `DropIndexOutcome { Dropped(TableDescription), Skipped }` (drop-skip
|
||||
note uses the command's `name` directly).
|
||||
- `index_name_exists(conn, name)` helper (the `sqlite_master WHERE
|
||||
type='index' AND name=?` lookup `do_add_index` step 5 already does).
|
||||
|
||||
### 4.5 Runtime + event + app
|
||||
- `runtime.rs`: `SqlCreateIndex` → `db.sql_create_index` → `Created(d)` →
|
||||
`CommandOutcome::Schema(Some(d))`, `Skipped(name)` → new
|
||||
`CommandOutcome::SchemaCreateIndexSkipped(name)`. `SqlDropIndex` →
|
||||
`Dropped(d)` → `Schema(Some(d))` (auto-show the de-indexed table),
|
||||
`Skipped` → new `CommandOutcome::SchemaDropIndexSkipped`.
|
||||
- `event.rs`: `DslCreateIndexSkipped { command, name }`,
|
||||
`DslDropIndexSkipped { command }`.
|
||||
- `app.rs`: note `ddl.create_index_skipped_exists` (name from the event)
|
||||
/ `ddl.drop_index_skipped_absent` (name from `command.target_table()`).
|
||||
Both **note-only, no structure** (mirrors the drop-table skip — the
|
||||
index already exists / never existed; showing the whole table is
|
||||
noise). Add the failure-translate arms for the two new commands.
|
||||
|
||||
### 4.7 Unique-index display (user-confirmed for 4d, 2026-05-25)
|
||||
A learner must be able to tell a UNIQUE index from a plain one.
|
||||
- **Structure view (cheap — `IndexInfo.unique` already populated):**
|
||||
`output_render.rs:143` appends a ` [unique]` marker when
|
||||
`index.unique`, e.g. `Customers_email_idx (Email) [unique]`. A render
|
||||
test + any insta snapshot update. No new threading.
|
||||
- **Items list (left panel):** `SchemaCache.table_indexes` is
|
||||
`HashMap<String, Vec<String>>` (names only). Carry uniqueness — change
|
||||
the value to a small `{ name, unique }` entry (or `Vec<(String,
|
||||
bool)>`), populate it from the schema-cache refresh (the `unique` bit
|
||||
is on the read), and render `name [unique]` in `ui.rs:516`. Update the
|
||||
`items_panel_nests_indexes_under_their_table` test + any callers
|
||||
constructing `table_indexes`.
|
||||
- Marker wording engine-neutral (no PRAGMA/SQLite leakage) — `[unique]`
|
||||
is a plain English adjective, fine.
|
||||
|
||||
### 4.6 Friendly catalog (`keys.rs` + `en-US.yaml`)
|
||||
New keys (engine-neutral; lockstep + vocab audit enforce both):
|
||||
- `ddl.create_index_skipped_exists` → `"index '{name}' already exists —
|
||||
skipped (no changes made)"` (`&["name"]`).
|
||||
- `ddl.drop_index_skipped_absent` → `"index '{name}' doesn't exist —
|
||||
skipped (no changes made)"` (`&["name"]`).
|
||||
- `help.ddl.sql_create_index` + `parse.usage.sql_create_index`,
|
||||
`help.ddl.sql_drop_index` + `parse.usage.sql_drop_index` (fresh nodes;
|
||||
their keys must exist now — unlike the deferred *refresh* of the
|
||||
create-table skeleton in 4i(a)).
|
||||
|
||||
## 5. Phase 2 — Candidate approaches
|
||||
|
||||
**(C1) CREATE INDEX grammar — single node w/ leading `Choice`** *(lead)*.
|
||||
One `SQL_CREATE_INDEX` node; `[UNIQUE]` via a leading `Choice` of
|
||||
concrete-keyword-led branches; `unique` read in the builder.
|
||||
- *Good:* one node, one builder, one REGISTRY entry; matches how a single
|
||||
statement maps to a single command.
|
||||
- *Risk:* a leading `Choice` in a top-level shape is untested here; the
|
||||
unnamed-form `on` disambiguation. Both are **probe-or-fall-back**, not
|
||||
blockers.
|
||||
|
||||
**(C2) CREATE INDEX grammar — two nodes split on `unique`/`index`**
|
||||
(the 4c `TABLE_FK` split). `SQL_CREATE_INDEX` (`index`-led) +
|
||||
`SQL_CREATE_INDEX_UNIQUE` (`unique`-led).
|
||||
- *Good:* each node leads on a concrete keyword — guaranteed safe past the
|
||||
trap; `create` then has three advanced nodes, exercising the
|
||||
all-candidates dispatch even harder (a feature for the test).
|
||||
- *Bad:* duplicated shape/builder; more REGISTRY noise. Adopt **only if
|
||||
C1's leading `Choice` probes badly.**
|
||||
|
||||
**(M1) Model extension — `IndexSchema.unique` flag** *(lead; the only
|
||||
real option)*. Mirrors `TableSchema.unique_constraints` exactly (additive,
|
||||
serde-default, version 1). The read side is already half-done
|
||||
(`IndexInfo.unique`).
|
||||
**(M2) Separate `__rdbms_playground_indexes` metadata table** —
|
||||
*rejected.* ADR-0025 deliberately stores nothing app-specific for indexes
|
||||
(SQLite owns the index namespace, incl. uniqueness via
|
||||
`pragma_index_list`). A metadata table would duplicate engine-owned state
|
||||
and create a consistency hazard. The flag is read straight from the
|
||||
pragma.
|
||||
|
||||
**(S1) Skip plumbing — dedicated index skip outcomes/events** *(lead)*.
|
||||
New `CreateIndexOutcome`/`DropIndexOutcome` + `DslCreate/DropIndexSkipped`
|
||||
events + index-specific notes. Consistent with 4a/4c (one event per skip
|
||||
kind).
|
||||
**(S2) Generalise the existing skip events to be object-agnostic** —
|
||||
*rejected.* Would rewrite 4a/4c wording/behaviour for marginal saving;
|
||||
churn on shipped code.
|
||||
|
||||
## 6. Phase 3 — Selection vs the checklist
|
||||
|
||||
C1 + M1 + S1 satisfy every §3 item: parse/flags (C1 grammar + builder),
|
||||
dispatch (REGISTRY + the existing all-candidates `decide`), execution
|
||||
(reuse `do_add_index`/`do_drop_index` + the `unique` param), persistence
|
||||
round-trip + rebuild survival (M1), skip semantics (S1 + the 4c skip
|
||||
branch), engine-neutral strings (catalog keys under the vocab audit).
|
||||
C1's two risks are bounded with C2 as the pre-identified fallback —
|
||||
neither leaves a requirement unmet. **Selected: C1 (probe; C2 fallback) +
|
||||
M1 + S1.**
|
||||
|
||||
## 7. Devil's Advocate review of this plan
|
||||
|
||||
- **The escalation actually escalated?** Yes — `IndexSchema.unique` + the
|
||||
ADR-0025 supersession were put to the user (2026-05-25) and approved,
|
||||
with simple-mode `add unique index` explicitly kept deferred. The
|
||||
`IF [NOT] EXISTS` scope was also confirmed (both forms). No autonomous
|
||||
scope decision. ✓
|
||||
- **Reuse vs fork?** `do_add_index`/`do_drop_index` stay the single
|
||||
executors; the SQL path adds only `unique` + the skip branch. The
|
||||
redundant-set guard refinement is a *correctness fix the model
|
||||
extension forces* (plain vs unique are not duplicates), not a fork. ✓
|
||||
- **Grammar risk owned — verified in code, not reasoned.** (1)
|
||||
`INDEX_NAME_EXISTING`/`TABLE_NAME_EXISTING` have `validator: None`, so
|
||||
`DROP INDEX IF EXISTS <absent>` parses and reaches the skip path (no
|
||||
hard existence check at parse). (2) `walk_ident`→`consume_ident` does
|
||||
**not** reject keywords, so a bare `Optional(<name>)` would eat `on` —
|
||||
hence the `DI_SELECTOR`-style `on`-led-first `Choice` (§4.1), proven by
|
||||
the shipped drop-index selector. (3) Trailing input after a matched
|
||||
shape is a `Mismatch` (not `Match`), and `decide` commits only on a
|
||||
full `Match`, so `drop index on T (c)` fails the name-only
|
||||
`SQL_DROP_INDEX` and falls back to the simple `DI_POSITIONAL` — the
|
||||
advanced-mode fallback for the positional drop is intact. A parse test
|
||||
for `create index on T (c)` → `name: None` and a fallback test for
|
||||
`drop index on T (c)` (advanced) → simple `DropIndex` are on the
|
||||
checklist. ✓
|
||||
- **Second advanced node verified, not assumed?** `decide` is read
|
||||
(advanced.chain(simple) + first-full-match loop) and a dispatch parse
|
||||
test is on the checklist (`create table`/`create index`/`drop
|
||||
table`/`drop index` each to the right command in advanced mode). ✓
|
||||
- **Round-trip + rebuild for the new flag?** Explicit checklist items:
|
||||
YAML `unique: true` save/load identity, older-file default, rebuild
|
||||
re-emits `CREATE UNIQUE INDEX`, uniqueness actually enforced after
|
||||
rebuild. Two DDL generators stay in sync (pattern 1): `do_add_index`
|
||||
and `rebuild_from_text` both gain the `UNIQUE` emit. ✓
|
||||
- **Undo parity?** One step per statement (`snapshot_then`); skip is
|
||||
journalled-not-snapshotted (4c pattern). Undo tests for both create and
|
||||
drop. ✓
|
||||
- **Anything silently dropped?** Simple-mode `add unique index` is
|
||||
*deferred by user decision*, recorded in the ADR-0025 Amendment — not
|
||||
silently. The 4i list grows by one (help/usage skeleton refresh for the
|
||||
new index forms — but these nodes' keys land now, only the *unified
|
||||
CREATE TABLE* skeleton refresh stays in 4i(a)). Completion merge for
|
||||
the now-two-advanced-node `create`/`drop` widens 4i(d) — already
|
||||
tracked, user-confirmed deferred. ✓
|
||||
- **`describe`/items-list of a unique index — RESOLVED (user chose "add
|
||||
in 4d", 2026-05-25).** The structure view rendered every index as
|
||||
`name (cols)` with no uniqueness marker — a pedagogy gap ("can't tell a
|
||||
UNIQUE index from a plain one"). Now in scope: §4.7. Structure view is
|
||||
cheap (`IndexInfo.unique` already populated); the items list needs the
|
||||
`SchemaCache.table_indexes` value to carry the bit. Both on the
|
||||
checklist. ✓
|
||||
|
||||
## 8. Out of 4d scope (tracked, not dropped)
|
||||
- Simple-mode `add unique index` — deferred by user decision (ADR-0025
|
||||
Amendment).
|
||||
- `ALTER TABLE` (4e–4h).
|
||||
- 4i(a) CREATE TABLE help/usage skeleton refresh; 4i(b) `describe` of
|
||||
**table-level constraints** (composite `UNIQUE` + table `CHECK`) — the
|
||||
*unique-index* display moves **into 4d** (§4.7), but the table-level
|
||||
constraint display stays 4i(b); 4i(d) shared-entry-word completion
|
||||
merge (now widened by `create`/`drop` having two advanced nodes);
|
||||
4i(e) the simple/advanced completion-colour UX discussion.
|
||||
|
||||
## 9. Implementation sequence (test-first)
|
||||
1. **Model extension first (M1), isolated** — add `IndexSchema.unique` +
|
||||
yaml round-trip + `read_schema_snapshot` + rebuild emit + `do_add_index`
|
||||
`unique` param (default-`false` call site) + the dup-guard refinement.
|
||||
Red tests: YAML round-trip of a unique index, rebuild survival,
|
||||
uniqueness enforced, plain≠unique not-a-duplicate → green. **No SQL
|
||||
surface yet** — this keeps the model change reviewable on its own and
|
||||
green before any grammar lands.
|
||||
2. **DROP INDEX (simpler grammar)** — Tier-1 parse + dispatch tests →
|
||||
command/grammar/builder/REGISTRY/worker(`DropIndexOutcome`+skip)/
|
||||
runtime/event/app/catalog → Tier-3 (`tests/sql_drop_index.rs`) → green.
|
||||
3. **CREATE INDEX** — smoke-probe the leading `Choice`; Tier-1 parse +
|
||||
flag + dispatch tests (incl. `create index on T (c)` → `name: None`)
|
||||
→ command/grammar/builder/REGISTRY/worker(`CreateIndexOutcome`+skip)/
|
||||
runtime/event/app/catalog → Tier-3 (`tests/sql_create_index.rs`,
|
||||
incl. unique create + round-trip + rebuild + IF NOT EXISTS skip +
|
||||
duplicate-data refusal) → green.
|
||||
4. **Unique-index display (§4.7)** — structure-view `[unique]` marker
|
||||
(render test) + items-list marker (`SchemaCache.table_indexes` carries
|
||||
the bit; ui test) → green.
|
||||
5. **Full sweep** — `cargo test` (no regression from 1805) + `cargo
|
||||
clippy --all-targets -- -D warnings`.
|
||||
6. **Docs** — ADR-0035 Status note + §13 4d; **ADR-0025 Amendment** (the
|
||||
UNIQUE-index supersession for advanced mode) + README index-upkeep;
|
||||
`requirements.md` Q1/C3. Run `/runda` over this slice. Propose commit;
|
||||
wait for approval.
|
||||
|
||||
## 10. Exit gate
|
||||
- All §3 items satisfied; four tiers green, zero skips; no regression from
|
||||
the 1805 baseline; `/runda` / written-DA PASS; clippy clean; ADR-0025
|
||||
Amendment + ADR-0035 §13 4d + README + `requirements.md` updated
|
||||
lockstep.
|
||||
Reference in New Issue
Block a user