feat: ADR-0035 4c — DROP TABLE [IF EXISTS]
Add advanced-mode SQL `DROP TABLE [IF EXISTS] <name>` -> SqlDropTable, executing through the existing do_drop_table (cascade / inbound- relationship refusal / metadata cleanup) — full parity with the simple `drop table`. The only new behaviour is `IF EXISTS` as a no-op-with-note: a new DropOutcome::Skipped mirroring CreateOutcome::Skipped (journalled, no snapshot), rendered via a new ddl.drop_skipped_absent note + DslDropSkipped event. - Grammar: SQL_DROP_TABLE node (entry `drop`, shape `table [if exists] <name> [;]`), registered Advanced. SQL-first dispatch: `drop table T` -> SqlDropTable in advanced; `drop column`/`relationship`/`index`/ `constraint` fall back to the simple `drop` node (and still execute). - Worker: Request::SqlDropTable + db.sql_drop_table; the if-exists-and- absent arm journals + replies Skipped without a snapshot, else snapshot_then(do_drop_table) -> Dropped. - Completion: advanced `drop ` now surfaces the SQL `table` (the shared-entry-word behaviour from `create`); test split into simple (full DSL list) + advanced (SQL surface). Known shared-entry-word completion unevenness (advanced `drop ` offers only `table`; partial `drop rel` returns an empty list) deferred to 4i (merge candidate sets for shared entry words) along with a flagged user request to visually distinguish simple- vs advanced-mode completions in the hint UI — tracked in ADR §13 4i (d)/(e), the 4c plan, and the completion test. The DSL drops still parse + execute via fallback. 10 new tests (parse/builder + Tier-3: drop existing + one-undo-step + restore, IF EXISTS skip + journal, plain-absent error, inbound refusal). Docs: ADR-0035 Status/§13, README, requirements.md Q1. Tests: 1805 passing, 0 failing, 1 ignored. Clippy clean.
This commit is contained in:
claude@clouddev1
@@ -3,13 +3,13 @@
|
||||
## Status
|
||||
|
||||
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**
|
||||
**validated end-to-end by sub-phases 4a / 4a.2 / 4a.3 / 4b / 4c**
|
||||
(`CREATE TABLE` with column- and table-level constraints and foreign
|
||||
keys, implemented 2026-05-25 — plans
|
||||
keys, and `DROP TABLE [IF EXISTS]`, implemented 2026-05-25 — plans
|
||||
`docs/plans/20260524-adr-0035-sql-ddl-4a.md`, `…-4a2.md`, `…-4a3.md`,
|
||||
`docs/plans/20260525-adr-0035-sql-ddl-4b.md`), so the decision is
|
||||
accepted while the remaining sub-phases (**4c–4i**, §13) continue. This
|
||||
is **Phase 4** of the ADR-0030 roadmap (the
|
||||
`docs/plans/20260525-adr-0035-sql-ddl-4b.md`, `…-4c.md`), so the
|
||||
decision is accepted while the remaining sub-phases (**4d–4i**, §13)
|
||||
continue. 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.
|
||||
@@ -367,8 +367,19 @@ ADR-0033's structure:
|
||||
takes `CONSTRAINT <name>`. PK-target only (UNIQUE-target deferred with
|
||||
`add relationship`); `Type::fk_target_type` (ADR-0011) governs type
|
||||
compatibility.
|
||||
- **4c — `DROP TABLE [IF EXISTS]`** → `SqlDropTable` (cascade parity;
|
||||
`IF EXISTS` no-op-with-note, §4).
|
||||
- **4c — `DROP TABLE [IF EXISTS]`** → `SqlDropTable`. *(Implemented
|
||||
2026-05-25 — plan `docs/plans/20260525-adr-0035-sql-ddl-4c.md`.)*
|
||||
Reuses `do_drop_table` (cascade parity + the inbound-relationship
|
||||
refusal + metadata cleanup), so it matches the simple `drop table`;
|
||||
`IF EXISTS` on an absent table is a no-op-with-note (a new
|
||||
`DropOutcome::Skipped` mirroring `CreateOutcome::Skipped`; journalled,
|
||||
no snapshot, §4). `drop` is a shared entry word: `drop table` parses
|
||||
as `SqlDropTable` in advanced mode, `drop column`/`relationship`/
|
||||
`index`/`constraint` fall back to the simple `drop` node. Advanced-
|
||||
mode `drop ` completion now surfaces the SQL `table` (the
|
||||
shared-entry-word behaviour from `create`, ADR-0033 Amendment 3); the
|
||||
DSL drops still parse via fallback — 4i grows the surface as `DROP
|
||||
INDEX` lands in 4d.
|
||||
- **4d — `CREATE [UNIQUE] INDEX` / `DROP INDEX`** → `SqlCreateIndex`
|
||||
/ `SqlDropIndex` (ADR-0025; the `UNIQUE` flag extension if needed).
|
||||
- **4e — `ALTER TABLE` add/drop/rename column.** Drop/rename column
|
||||
@@ -394,7 +405,21 @@ ADR-0033's structure:
|
||||
schema-existence diagnostic falsely flags the not-yet-created self
|
||||
table as unknown (the FK parent slot is `IdentSource::Tables`). Make
|
||||
the diagnostic treat a FK parent equal to the `CREATE TABLE` target as
|
||||
valid, so the indicator stops lying for self-references.
|
||||
valid, so the indicator stops lying for self-references. (d) **4c
|
||||
shared-entry-word completion merge** — in advanced mode a shared entry
|
||||
word surfaces only the SQL node's continuations, so `drop ` offers
|
||||
only `table` (not the DSL `column`/`relationship`/`index`/`constraint`)
|
||||
and a partial keyword like `drop rel` returns an *empty* list (a
|
||||
mid-word dead end), even though the DSL drops still parse + execute via
|
||||
fallback. Merge the expected sets of all candidate nodes for a shared
|
||||
entry word so advanced completion offers every valid continuation
|
||||
(`drop ` → table + column + relationship + index + constraint; `drop
|
||||
rel` → relationship); verify `create`/`insert`/`update`/`delete`
|
||||
completion stays sensible. (e) **Discussion flag (user, 2026-05-25):**
|
||||
before/with (d), discuss **visually distinguishing simple- vs
|
||||
advanced-mode completions in the hint UI (likely by colour)** so a
|
||||
learner can see which continuations are DSL and which are SQL — a UX
|
||||
design conversation, not just the mechanical merge.
|
||||
|
||||
## Consequences
|
||||
|
||||
|
||||
+1
-1
File diff suppressed because one or more lines are too long
@@ -0,0 +1,158 @@
|
||||
# Plan: ADR-0035 Phase 4, sub-phase 4c — `DROP TABLE [IF EXISTS]`
|
||||
|
||||
Add advanced-mode SQL `DROP TABLE [IF EXISTS] <name>` → `SqlDropTable`.
|
||||
Reuses the existing `do_drop_table` (cascade/inbound-refusal +
|
||||
metadata cleanup), so it has full parity with the simple `drop table`;
|
||||
the only new behaviour is `IF EXISTS` as a **no-op-that-succeeds-with-a-
|
||||
note**, mirroring 4a's `CreateOutcome::Skipped` with a `DropOutcome`
|
||||
(ADR-0035 §4/§13). Small slice.
|
||||
|
||||
## 1. Baseline
|
||||
|
||||
- Tests: **1795 passing, 0 failing, 1 ignored**; clippy clean. Branch
|
||||
`main`, last commit `76d6059` (4b). 4c starts here.
|
||||
|
||||
## 2. Decisions (from ADR §4/§13 + the 4a precedent — not re-litigated)
|
||||
|
||||
1. **`IF EXISTS` → no-op-with-note** (universal cross-vendor idiom,
|
||||
already in scope per the 4a `IF [NOT] EXISTS` decision): dropping an
|
||||
absent table succeeds with a note instead of the plain "no such
|
||||
table" error. Mirrors `CreateOutcome::Skipped` via a new
|
||||
`DropOutcome { Dropped, Skipped }`. The skip is **journalled** (no
|
||||
snapshot), exactly as the create-skip is.
|
||||
2. **Table only.** SQL `DROP TABLE` drops a table; `DROP INDEX` is 4c's
|
||||
sibling 4d (`SqlDropIndex`); column/constraint drops are `ALTER TABLE`
|
||||
(4e+). `drop column`/`drop relationship`/`drop index` keep falling
|
||||
back to the simple `drop` node.
|
||||
3. **Dispatch (ADR-0033 Amendment 1).** `drop` is a shared entry word;
|
||||
the SQL node is tried first in advanced mode. `drop table T` parses
|
||||
as `SqlDropTable` in advanced mode (equivalent execution to the
|
||||
simple `DropTable` — both call `do_drop_table`); the simple form is
|
||||
the only one in simple mode.
|
||||
4. **Cascade / inbound-relationship refusal parity** — reuse
|
||||
`do_drop_table` unchanged (it already refuses dropping a table with
|
||||
inbound relationships and cleans `__rdbms_*` metadata, incl. the 4a.3
|
||||
`table_checks` rows). One undo step (`snapshot_then`).
|
||||
|
||||
## 3. Phase 1 — Requirements checklist (4c)
|
||||
|
||||
- [ ] `DROP TABLE <name>` parses in advanced mode → `SqlDropTable`.
|
||||
- [ ] `DROP TABLE IF EXISTS <name>` sets the `if_exists` flag.
|
||||
- [ ] Dropping an existing table removes it + its metadata (parity with
|
||||
simple `drop table`); one undo step; `undo` restores it.
|
||||
- [ ] `IF EXISTS` on an **absent** table → success with a note, no
|
||||
error, no snapshot; the line is journalled.
|
||||
- [ ] Plain `DROP TABLE` on an absent table → the existing "no such
|
||||
table" error (unchanged).
|
||||
- [ ] Dropping a table with **inbound relationships** is refused
|
||||
(existing `do_drop_table` semantics), in advanced mode too.
|
||||
- [ ] `drop column` / `drop relationship` / `drop index` still parse as
|
||||
the simple forms in advanced mode (fallback).
|
||||
- [ ] Engine-neutral note wording.
|
||||
|
||||
### Testing
|
||||
|
||||
- [ ] **Tier 1** (builder): `drop table <name>` → `SqlDropTable{if_exists:
|
||||
false}`; `drop table if exists <name>` → `if_exists: true`; the simple
|
||||
`drop column`/`drop relationship` forms still parse (fallback).
|
||||
- [ ] **Tier 3** (`tests/sql_drop_table.rs`): drop existing → gone +
|
||||
one undo step + undo restores; `IF EXISTS` absent → skipped + note +
|
||||
journalled; plain absent → error; inbound-relationship refusal.
|
||||
- [ ] **Catalog** lockstep + vocab audit for the new note key.
|
||||
|
||||
## 4. Architecture & design
|
||||
|
||||
### 4.1 Grammar (`src/dsl/grammar/ddl.rs`)
|
||||
- `IF_EXISTS_OPT` = `Optional(Seq[Word("if"), Word("exists")])`.
|
||||
- `SQL_DROP_TABLE_SHAPE` = `Seq[Word("table"), IF_EXISTS_OPT,
|
||||
TABLE_NAME_EXISTING, Optional(Punct(';'))]` (mirrors the simple
|
||||
`DROP_TABLE` shape + the optional `IF EXISTS` + trailing `;`).
|
||||
- `pub static SQL_DROP_TABLE: CommandNode { entry: "drop", shape,
|
||||
ast_builder: build_sql_drop_table, help_id: "ddl.sql_drop_table",
|
||||
usage_ids: ["parse.usage.sql_drop_table"] }`.
|
||||
- `build_sql_drop_table`: `name = require_ident("table_name")`,
|
||||
`if_exists = path.contains_word("if")` (the `if` only appears in the
|
||||
`IF EXISTS` prefix; mirror the create builder's `if_not_exists`).
|
||||
|
||||
### 4.2 Command (`src/dsl/command.rs`)
|
||||
`SqlDropTable { name: String, if_exists: bool }`. Verb label
|
||||
`"drop table"`; `target_table()` → `name` (add to the existing match
|
||||
arms alongside `DropTable`).
|
||||
|
||||
### 4.3 Worker (`src/db.rs`)
|
||||
- `DropOutcome { Dropped, Skipped }` (the drop peer of `CreateOutcome`;
|
||||
`Skipped` needs no payload — the runtime renders the note from the
|
||||
command's name).
|
||||
- `Request::SqlDropTable { name, if_exists, source, reply:
|
||||
oneshot<Result<DropOutcome>> }` + `db.sql_drop_table` method.
|
||||
- Dispatch arm: if `if_exists && !user_table_exists(name)` → journal the
|
||||
line (no snapshot) and reply `Skipped` (mirror the create-skip arm);
|
||||
else `snapshot_then(… do_drop_table(name) … → Dropped)`.
|
||||
- `do_drop_table` unchanged.
|
||||
|
||||
### 4.4 Runtime + event + app (`src/runtime.rs`, `src/event.rs`, `src/app.rs`)
|
||||
- `Command::SqlDropTable` → `db.sql_drop_table` → map `Dropped` →
|
||||
`CommandOutcome::Schema(None)` (same as simple drop), `Skipped` → a new
|
||||
`CommandOutcome::SchemaDropSkipped`.
|
||||
- `CommandOutcome::SchemaDropSkipped` → `AppEvent::DslDropSkipped {
|
||||
command }`; the app handler notes `ddl.drop_skipped_absent` with
|
||||
`command.target_table()` (mirror `DslCreateSkipped`). No structure to
|
||||
render (the table is gone / never existed).
|
||||
|
||||
### 4.5 Friendly catalog
|
||||
- New key `ddl.drop_skipped_absent: "table '{name}' doesn't exist —
|
||||
skipped (no changes made)"` + a `keys.rs` `("ddl.drop_skipped_absent",
|
||||
&["name"])` entry. Engine-neutral. Help/usage skeleton body for
|
||||
`ddl.sql_drop_table` + `parse.usage.sql_drop_table` (a fresh node — its
|
||||
keys must exist; unlike the deferred *refresh* of the create skeleton,
|
||||
these are new keys the node references, so they land now).
|
||||
|
||||
## 5. Out of 4c scope
|
||||
- `DROP INDEX` (4d → `SqlDropIndex`); `ALTER TABLE` (4e–4h).
|
||||
- **Shared-entry-word completion (deferred to 4i, user-confirmed
|
||||
2026-05-25).** Adding the SQL `drop` node makes advanced-mode `drop `
|
||||
completion surface only `table`; a partial DSL subcommand keyword
|
||||
(`drop rel`) returns an empty list (mid-word dead end). The DSL drops
|
||||
still parse + execute via fallback — only the completion hint is
|
||||
affected. This is the pre-existing shared-entry-word model (same as
|
||||
`create`/`insert`/`update`/`delete`), exposed here because `drop` has
|
||||
five distinct DSL subcommands with no SQL equivalent. 4i merges the
|
||||
candidate sets for shared entry words; **the user also wants to
|
||||
discuss visually distinguishing simple- vs advanced-mode completions
|
||||
in the hint UI (likely by colour)** before/with that work. Tracked in
|
||||
ADR-0035 §13 4i (d)/(e) and the advanced-mode completion test in
|
||||
`src/completion.rs`.
|
||||
|
||||
## 6. Devil's Advocate review of this plan
|
||||
- **Reuse vs fork?** `do_drop_table` is the single executor; the SQL
|
||||
path differs only at the `IF EXISTS` branch (mirrors the create-skip).
|
||||
✓
|
||||
- **Dispatch fallback?** A builder test asserts `drop column`/`drop
|
||||
relationship` still parse simple in advanced mode. ✓
|
||||
- **One undo step + restore?** `snapshot_then` wrap + a dedicated undo
|
||||
test (drop → undo → table back, with its data/metadata). ✓
|
||||
- **No-op journalled, not snapshotted?** Mirrors the create-skip arm
|
||||
exactly; a journalling test. ✓
|
||||
- **Engine-neutral?** The note is a catalog key under the vocab audit;
|
||||
the plain-absent error is the existing `do_drop_table` path (unchanged,
|
||||
already engine-neutral). ✓
|
||||
|
||||
## 7. Implementation sequence (test-first)
|
||||
1. **Command + grammar + builder** — Tier-1 builder tests (parse +
|
||||
`if_exists` + simple fallback) → red → add `SqlDropTable`, the grammar
|
||||
node + shape + builder, REGISTRY entry, thread `Request`/method/
|
||||
dispatch/runtime (drop execution can land here too) → green.
|
||||
2. **Worker no-op-with-note** — Tier-3 (`tests/sql_drop_table.rs`): drop
|
||||
existing, `IF EXISTS` skip + journal, plain-absent error, inbound
|
||||
refusal, one undo step + restore → red → add `DropOutcome` + the
|
||||
dispatch arm + the `SchemaDropSkipped`/`DslDropSkipped`/app-note path
|
||||
→ green.
|
||||
3. **Catalog** — add the note + help/usage keys + bodies; lockstep +
|
||||
vocab audit → green.
|
||||
4. **Full sweep** — `cargo test` (no regression from 1795) + clippy.
|
||||
5. **Docs** — ADR-0035 Status/§13 4c; README; `requirements.md` Q1.
|
||||
Propose commit; wait for approval.
|
||||
|
||||
## 8. Exit gate
|
||||
- All §3 items satisfied; four tiers green, zero skips; no regression
|
||||
from the 1795 baseline; written DA pass; clippy clean.
|
||||
@@ -223,8 +223,9 @@ handoff-14 cleanup; 449 after B2/C2.)
|
||||
table, since the engine reports no CHECK constraints), then foreign
|
||||
keys (4b — inline `REFERENCES` + table-level `FOREIGN KEY` → ADR-0013
|
||||
named relationships in the create transaction; self-references and
|
||||
bare `REFERENCES <parent>` supported)). Remaining DDL — `DROP TABLE`
|
||||
(4c), indexes (4d), `ALTER TABLE` (4e–4h) — is phased per
|
||||
bare `REFERENCES <parent>` supported), then `DROP TABLE [IF EXISTS]`
|
||||
(4c — reuses `do_drop_table`; `IF EXISTS` is a no-op-with-note)).
|
||||
Remaining DDL — indexes (4d), `ALTER TABLE` (4e–4h) — is phased per
|
||||
ADR-0035 §13.)*
|
||||
- [ ] **Q2** Non-standard syntax rejected with a clear message
|
||||
pointing at the supported subset.
|
||||
|
||||
Reference in New Issue
Block a user