From 2c86a1313e6d5d354fdae9b8c984c43f40b7783e Mon Sep 17 00:00:00 2001 From: "claude@clouddev1" Date: Fri, 22 May 2026 14:59:01 +0000 Subject: [PATCH] =?UTF-8?q?grammar+db:=203f=20=E2=80=94=20SQL=20DELETE=20+?= =?UTF-8?q?=20cascade=20summary=20(ADR-0033=20=C2=A71/=C2=A77)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New src/dsl/grammar/sql_delete.rs (FROM [WHERE] [;]), Command::SqlDelete, Request::RunSqlDelete, do_sql_delete worker. do_sql_delete mirrors the DSL do_delete: detect FK cascade by before/after child row-count diffing, re-persist target + every cascade-affected child, history-on-success inside the tx. Reuses CommandOutcome::Delete -> handle_dsl_delete_success, so the per-relationship cascade summary formatter is shared, not duplicated. ADR-0033 Amendment 2: supersedes §7's WHERE-injected pre-count. Its premise (DSL handler builds pre-counts from the typed Expr) was wrong — do_delete uses count-diff. The pre-count would also have broken the §2 parity promise by reporting SET NULL the DSL path doesn't. Count- diff gives exact parity, no WHERE-byte extraction, and withdraws R2. SET NULL reporting deferred for both paths (user-confirmed). Tests: +6 grammar unit, +12 integration (cascade parity with DSL, both R2 subquery cases, before-execute order, no-WHERE, FK-rejection rollback, childless-parent, two-child cascade). 1542 pass / 0 fail / 1 ignored. Clippy clean. Dev sql_delete entry word removed in 3j. --- docs/adr/0033-sql-dml-grammar.md | 94 ++++++++ docs/adr/README.md | 2 +- src/app.rs | 6 + src/db.rs | 136 +++++++++++ src/dsl/command.rs | 16 +- src/dsl/grammar/data.rs | 52 +++- src/dsl/grammar/mod.rs | 5 + src/dsl/grammar/sql_delete.rs | 145 ++++++++++++ src/runtime.rs | 10 + tests/sql_delete.rs | 392 +++++++++++++++++++++++++++++++ tests/typing_surface/mod.rs | 1 + 11 files changed, 856 insertions(+), 3 deletions(-) create mode 100644 src/dsl/grammar/sql_delete.rs create mode 100644 tests/sql_delete.rs diff --git a/docs/adr/0033-sql-dml-grammar.md b/docs/adr/0033-sql-dml-grammar.md index a36f71b..069d5e0 100644 --- a/docs/adr/0033-sql-dml-grammar.md +++ b/docs/adr/0033-sql-dml-grammar.md @@ -1176,6 +1176,100 @@ Concretely: shared-entry problem (`create`, …): tag the SQL DDL nodes `Advanced`; the dispatcher handles the rest. +## Amendment 2 — Cascade summary on SQL DELETE: count-diff parity supersedes WHERE-injected pre-count (2026-05-22) + +This amendment **supersedes §7's "Predicate extraction" subsection +and the pre-count mechanism in §7 steps 1–3**, and **withdraws Open +Implementation Risk R2**. It was written during sub-phase 3f, after +tracing §7's prescribed mechanism against the actual DSL `do_delete` +handler, and is recorded with explicit user approval before any 3f +code landed. + +### The finding — §7's premise about the DSL handler is incorrect + +§7 prescribes a per-child **pre-execution pre-count** of the form +`SELECT count(*) FROM WHERE IN (SELECT FROM + WHERE )`, with the +`` re-injected from the SQL DELETE's +WHERE-clause source bytes. Its stated justification: + +> "The DSL handler reuses the typed `Expr` AST to construct the +> pre-count subqueries." + +Tracing `do_delete` in `src/db.rs` shows this premise is factually +wrong. The DSL handler does **not** construct pre-count subqueries +from the typed `Expr`. It detects cascade effects by **before/after +row-count diffing**: read the inbound relationships, count each child +table's rows *before* the DELETE, execute the DELETE inside a +transaction, count each child *after*, and report the positive +difference as a `CascadeEffect`. No pre-count query and no +`Expr`-derived subquery exist anywhere in the path. + +### Two consequences of the correct mechanism + +1. **Count-diff reports `ON DELETE CASCADE` row removals only.** `ON + DELETE SET NULL` does not change a child's row count, so the DSL + path does not report it (its own code comment notes this). + Reporting SET NULL would require value-level diffing. + +2. **§7's pre-count is in tension with the parity promise.** §2 + promises "a DSL-style … DELETE in Advanced mode produces identical + observable behaviour whether it routes through the SQL or DSL + path", and the plan's 3f exit gate requires the SQL cascade summary + to *match* the DSL `do_delete` output. A pre-count that counted + `ON DELETE CASCADE` *and* `SET NULL` (as §7 step 1 says) would + report **more** than the DSL path — breaking that parity. To match + the DSL path it would have to drop SET NULL anyway, leaving + WHERE-byte extraction and the R2 N+1-subquery pathology as pure + cost with no benefit over count-diff. + +### The replacement — count-diff parity + +`do_sql_delete` mirrors `do_delete` exactly: read inbound +relationships, snapshot child row counts, run the **verbatim** +validated DELETE SQL inside a transaction via +`execute_with_fk_enrichment`, diff the child counts, build the same +`Vec`, and re-persist the target plus every +cascade-affected child before commit. Because both handlers return +the same `DeleteResult` and both route through +`CommandOutcome::Delete` → `handle_dsl_delete_success`, the +per-relationship summary formatter is **already shared at the render +layer** — no formatter refactor and no duplicate path. + +This dominates the §7 mechanism on every axis the 3f exit gate +measures: + +- **Exact parity** with the DSL path — identical detection mechanism, + not merely a matching formatter. +- **No WHERE-clause byte extraction.** The verbatim DELETE (including + any subquery in its WHERE) executes once; the engine cascades; the + diff observes the result. The R2 invariant case (`DELETE FROM a + WHERE id IN (SELECT … )`) is correct by construction and carries no + N+1 cost. **R2 is withdrawn.** +- **Shared render** with no duplicate formatter, satisfying §7's + shared-formatter promise structurally. + +### SET NULL reporting — deferred for both paths (user-confirmed) + +3f keeps strict DSL parity: `ON DELETE CASCADE` row removals are +reported; `SET NULL` is not, on either path. Adding SET NULL +reporting (value-level diffing) is a tracked future enhancement to be +applied to **both** `do_delete` and `do_sql_delete` together so +parity is preserved. The user confirmed this deferral. + +### Consequences of the amendment + +- §7's "Predicate extraction" subsection and pre-count steps 1–3 are + superseded by count-diff; the per-relationship summary and + shared-formatter outcomes are unchanged. +- **R2 (cascade-summary predicate extraction) is withdrawn** — the + mechanism it budgeted for is no longer used. +- `Command::SqlDelete` carries `{ sql, target_table }` only — no + WHERE-clause field is needed (the worker never inspects the + predicate). +- The handoff-31 §4 "WHERE-byte-extraction is tractable for DELETE" + heads-up is moot — no extraction happens. + ## See also - ADR-0005 — the ten-type vocabulary INSERT works with. diff --git a/docs/adr/README.md b/docs/adr/README.md index 9a9f934..61beb17 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -38,4 +38,4 @@ This directory contains the project's ADRs, recorded per - [ADR-0030 — Advanced mode: the standard-SQL surface](0030-advanced-mode-sql-surface.md) — **Accepted**, SQL added as grammar *within the unified grammar tree* (ADR-0024), not a separate batch parser — so SQL gets the same completion / highlighting / hints / parse-errors as the DSL; mode gates the SQL forms; DDL routes through the typed `Command` executor (metadata + type vocabulary preserved), DML and `SELECT` execute as validated SQL; engine-neutral posture, the DSL→SQL teaching echo; supersedes ADR-0001's `sqlparser-rs` reservation; phased plan (`Q1` / `Q2` / `Q4`) - [ADR-0031 — The SQL expression grammar](0031-sql-expression-grammar.md) — **Accepted**, the stratified SQL expression grammar fragment commissioned by ADR-0030 §3: a single precedence ladder (`OR`/`AND`/`NOT`, the comparison/`LIKE`/`IN`/`BETWEEN`/`IS NULL` predicate set, arithmetic incl. `||`, function calls, `CASE`) — the superset of ADR-0026's DSL `WHERE` grammar, authored as a parallel fragment so simple mode is untouched; pure validation, builds **no** AST (consumers run/store SQL as text per ADR-0030 §4/§6); reuses ADR-0026's `Subgrammar` recursion + depth cap unchanged; subquery expressions and qualified column refs deferred to ADR-0030 Phase 2 - [ADR-0032 — The full SQL `SELECT` grammar](0032-sql-select-grammar.md) — **Accepted**, the Phase-2 grammar commissioned by ADR-0030 §3: full `SELECT` with `INNER`/`LEFT`/`RIGHT`/`FULL OUTER`/`CROSS` joins, `GROUP BY`/`HAVING`, all four set ops (`UNION`/`UNION ALL`/`INTERSECT`/`EXCEPT`), `WITH` and `WITH RECURSIVE` CTEs, `LIMIT … OFFSET`, `DISTINCT`, `t.*`, and bare-alias projection (lifting Phase-1 §4.2); additive extensions to ADR-0031's `sql_expr` for scalar subqueries, `IN (SELECT …)`, `[NOT] EXISTS`, and qualified column refs (redeeming ADR-0031 §7 OOS-1/OOS-2); grammar-recursion via `Subgrammar(&SQL_SELECT_COMPOUND)` reuses ADR-0026's `MAX_SUBGRAMMAR_DEPTH = 64` cap unchanged; **softens ADR-0030 §8's "ambient assistance comes for free" claim**: completion scope needs new `WalkContext` accumulators (a `from_scope_stack` of `ScopeFrame`s holding `from_scope` / `cte_bindings` / `projection_aliases`), a **new walker node variant `Node::ScopedSubgrammar(&Node)`** as the push/pop trigger (existing `Node::Subgrammar` unchanged so DSL `Expr` and `sql_expr` recursion are unaffected), qualified-prefix completion narrowing, body-projection-derived CTE column resolution (so `SELECT *` and explicit-projection CTE bodies both yield real column completion past `cte_alias.|`), and a **post-walk fixup pass** that re-resolves projection-list identifier highlighting/validity once `FROM` is parsed (the projection-before-FROM problem); classifies every Phase-2 validation case against ADR-0027's ERROR/WARNING guideline (§11): five new `diagnostic.*` keys for parse-time-detectable cases (unknown qualifier, ambiguous column, projection-alias misplaced, CTE/compound arity mismatch) plus eight `engine.*` translation keys; a MatchedPath-walking predicate-warnings variant that closes the Phase-1 gap where SQL `WHERE` expressions emitted no `LIKE`-on-numeric / `= NULL` / type-mismatch warnings (ADR-0027 Amendment 1 finally extends to the SQL surface); adds a worker-side post-prepare type-resolution pass via engine column-origin metadata so bare column refs recover their playground type (partially lifting Phase-1 §4.5, the bool→0/1 case) — `Cargo.toml` gains `column_metadata` to rusqlite features (verified against pinned 0.39.0); `__rdbms_*` rejection extended to every new table-source slot; Amendment 1 narrows §12's resolution rule from a grammar-side structural classification to "trust the engine's column-origin metadata verbatim" after an empirical probe showed origin metadata follows through non-recursive CTEs, scalar subqueries, derived tables, set ops, and joins — the one structural exception is recursive CTE result columns, which return None and stay typeless; Amendment 2 records that §10.6's "rewrite the highlight class" prescription is realised via the two-pass schema-existence diagnostic + the renderer's diagnostic-overlay path (no separate per-byte rewrite step needed; no new HighlightClass variant), and that the projection-before-FROM completion narrowing has been improved by an `src/completion.rs` look-ahead probe when the leading walk's `from_scope` is empty but the full input parses -- [ADR-0033 — The full SQL DML grammar (`INSERT` / `UPDATE` / `DELETE`)](0033-sql-dml-grammar.md) — **Proposed**, the Phase-3 grammar commissioned by ADR-0030 §3: single- and multi-row `INSERT` (incl. `INSERT … SELECT` recursing through ADR-0032's `SQL_SELECT_COMPOUND`), `UPDATE` with `SET` assignment list, `DELETE`, all three optionally followed by `RETURNING projection_list`, plus full `ON CONFLICT … DO NOTHING / DO UPDATE` UPSERT on INSERT; **fixes the DSL-vs-SQL dispatch architecture for shared entry words (`insert`/`update`/`delete`)**: SQL-first / DSL-fallback in Advanced mode via a `Choice(SQL_shape, DSL_shape)` per shape, gated by a new walker capability `Node::Guard(fn)` — a zero-byte-consumption gating node that fails the enclosing Seq with a `ValidationError`; carries `Command::SqlInsert` / `SqlUpdate` / `SqlDelete` variants and `do_sql_*` worker handlers each of which knows the target table (for re-persistence) and the `returning: bool` flag (for DataResult routing); `shortid` auto-fill mirrors the DSL `do_insert` mechanism via worker post-fill; SQL DELETE produces the same per-relationship cascade summary the DSL DELETE does (ADR-0014 parity), with the WHERE-clause source bytes re-injected into per-child pre-count subqueries; three new walker diagnostics (`insert_arity_mismatch` ERROR, `auto_column_overridden` WARNING, `not_null_missing` WARNING) with positive + negative tests each; OOS list explicitly carves out `DEFAULT VALUES` (the project's planned seed feature), SQLite-specific `OR REPLACE` / `OR IGNORE` / `OR ABORT` / `OR FAIL` / `OR ROLLBACK` prefixes, `UPDATE FROM` multi-table updates, and WITH-prefixed DML; the `excluded` keyword inside `ON CONFLICT DO UPDATE` is a deliberate carve-out from ADR-0030 §7's engine-neutral posture (no standard-SQL UPSERT spelling exists that SQLite and PostgreSQL share); eleven phased sub-phases each with explicit exit gates + written DA gate, opening with the dispatch mechanism before any DML grammar lands; initial DA review recorded seven critiques that were resolved before status moved to Proposed; **Amendment 1 supersedes §2's dispatch mechanism**: the originally-chosen `Node::Guard(fn)` + `Choice(SQL_shape, DSL_shape)` was found during 3a to be unworkable as framed (any guard-in-`Choice` mechanism forces a `walk_choice` change — `walk_choice` only falls through on `NoMatch`, so Simple-mode valid-DSL would wrongly surface "this is SQL", and `walk_seq` treats a `NoMatch` past `idx 0` as a hard `Failed`, breaking Advanced-mode DSL fall-through); replaced by **category-grouped, mode-aware dispatch** in `walker::walk` (each `REGISTRY` entry tagged `CommandCategory::{Simple, Advanced}`, generalising the existing whole-command `is_advanced_only` gate), shared entry words carrying a node in both groups, no `Node::Guard` and no `walk_choice`/`walk_seq` change, advanced-mode completion SQL-first with DSL as a full-line fallback +- [ADR-0033 — The full SQL DML grammar (`INSERT` / `UPDATE` / `DELETE`)](0033-sql-dml-grammar.md) — **Proposed**, the Phase-3 grammar commissioned by ADR-0030 §3: single- and multi-row `INSERT` (incl. `INSERT … SELECT` recursing through ADR-0032's `SQL_SELECT_COMPOUND`), `UPDATE` with `SET` assignment list, `DELETE`, all three optionally followed by `RETURNING projection_list`, plus full `ON CONFLICT … DO NOTHING / DO UPDATE` UPSERT on INSERT; **fixes the DSL-vs-SQL dispatch architecture for shared entry words (`insert`/`update`/`delete`)**: SQL-first / DSL-fallback in Advanced mode via a `Choice(SQL_shape, DSL_shape)` per shape, gated by a new walker capability `Node::Guard(fn)` — a zero-byte-consumption gating node that fails the enclosing Seq with a `ValidationError`; carries `Command::SqlInsert` / `SqlUpdate` / `SqlDelete` variants and `do_sql_*` worker handlers each of which knows the target table (for re-persistence) and the `returning: bool` flag (for DataResult routing); `shortid` auto-fill mirrors the DSL `do_insert` mechanism via worker post-fill; SQL DELETE produces the same per-relationship cascade summary the DSL DELETE does (ADR-0014 parity); three new walker diagnostics (`insert_arity_mismatch` ERROR, `auto_column_overridden` WARNING, `not_null_missing` WARNING) with positive + negative tests each; OOS list explicitly carves out `DEFAULT VALUES` (the project's planned seed feature), SQLite-specific `OR REPLACE` / `OR IGNORE` / `OR ABORT` / `OR FAIL` / `OR ROLLBACK` prefixes, `UPDATE FROM` multi-table updates, and WITH-prefixed DML; the `excluded` keyword inside `ON CONFLICT DO UPDATE` is a deliberate carve-out from ADR-0030 §7's engine-neutral posture (no standard-SQL UPSERT spelling exists that SQLite and PostgreSQL share); eleven phased sub-phases each with explicit exit gates + written DA gate, opening with the dispatch mechanism before any DML grammar lands; initial DA review recorded seven critiques that were resolved before status moved to Proposed; **Amendment 1 supersedes §2's dispatch mechanism**: the originally-chosen `Node::Guard(fn)` + `Choice(SQL_shape, DSL_shape)` was found during 3a to be unworkable as framed (any guard-in-`Choice` mechanism forces a `walk_choice` change — `walk_choice` only falls through on `NoMatch`, so Simple-mode valid-DSL would wrongly surface "this is SQL", and `walk_seq` treats a `NoMatch` past `idx 0` as a hard `Failed`, breaking Advanced-mode DSL fall-through); replaced by **category-grouped, mode-aware dispatch** in `walker::walk` (each `REGISTRY` entry tagged `CommandCategory::{Simple, Advanced}`, generalising the existing whole-command `is_advanced_only` gate), shared entry words carrying a node in both groups, no `Node::Guard` and no `walk_choice`/`walk_seq` change, advanced-mode completion SQL-first with DSL as a full-line fallback; **Amendment 2 (sub-phase 3f) supersedes §7's cascade mechanism**: the WHERE-injected per-child pre-count rested on a premise that was factually wrong about the DSL handler (which detects cascades by before/after row-count diffing inside a transaction, not by `Expr`-derived pre-count subqueries) and would have broken the §2 parity promise by reporting `SET NULL` the DSL path doesn't; replaced by mirroring `do_delete`'s count-diff exactly (verbatim DELETE executes, child-count diff observes the cascade — `ON DELETE CASCADE` row removals only, SET NULL deferred for both paths to preserve parity), which shares the render-layer formatter for free via `CommandOutcome::Delete` and **withdraws risk R2** (no WHERE-byte extraction, no N+1 subquery) diff --git a/src/app.rs b/src/app.rs index af91bf0..e44d030 100644 --- a/src/app.rs +++ b/src/app.rs @@ -1485,6 +1485,12 @@ impl App { C::SqlUpdate { target_table, .. } => { (Operation::Update, Some(target_table.as_str()), None) } + // A SQL `DELETE` (ADR-0033 §1/§7) — route engine errors + // (e.g. an FK violation with no cascade) through the + // delete operation with the parsed target. + C::SqlDelete { target_table, .. } => { + (Operation::Delete, Some(target_table.as_str()), None) + } C::Replay { .. } => (Operation::Replay, None, None), // An `explain` failure (e.g. unknown table) is best // described by the wrapped query it failed to plan. diff --git a/src/db.rs b/src/db.rs index 9dc233f..becbb9b 100644 --- a/src/db.rs +++ b/src/db.rs @@ -605,6 +605,18 @@ enum Request { target_table: String, reply: oneshot::Sender>, }, + /// Run a grammar-validated SQL `DELETE` (ADR-0033 §1/§7). The + /// worker executes `sql` as text, detects FK cascade by + /// row-count diffing the inbound children (Amendment 2), + /// re-persists `target_table`'s CSV plus every cascade-affected + /// child (ADR-0030 §11), and appends the literal line to + /// `history.log`. + RunSqlDelete { + sql: String, + source: Option, + target_table: String, + reply: oneshot::Sender>, + }, /// Capture the query plan for an explainable command via /// `EXPLAIN QUERY PLAN` (ADR-0028 §2). `query` is the inner /// `ShowData` / `Update` / `Delete`; `EXPLAIN QUERY PLAN` @@ -1110,6 +1122,29 @@ impl Database { recv.await.map_err(|_| DbError::WorkerGone)? } + /// Run a validated SQL `DELETE` and return the affected-row + /// count plus any cascade effects (ADR-0033 §1/§7, sub-phase + /// 3f). `sql` is the grammar-validated statement text; `source` + /// is the literal submitted line for `history.log`; + /// `target_table` is the parsed target whose CSV (and whose + /// cascade-affected children's CSVs) are re-persisted. + pub async fn run_sql_delete( + &self, + sql: String, + source: Option, + target_table: String, + ) -> Result { + let (reply, recv) = oneshot::channel(); + self.send(Request::RunSqlDelete { + sql, + source, + target_table, + reply, + }) + .await?; + recv.await.map_err(|_| DbError::WorkerGone)? + } + /// Capture the query plan for an explainable command /// (ADR-0028 §2). The wrapped command is not executed — /// `EXPLAIN QUERY PLAN` only inspects how the engine would @@ -1587,6 +1622,20 @@ fn handle_request(conn: &Connection, persistence: Option<&Persistence>, req: Req &target_table, )); } + Request::RunSqlDelete { + sql, + source, + target_table, + reply, + } => { + let _ = reply.send(do_sql_delete( + conn, + persistence, + source.as_deref(), + &sql, + &target_table, + )); + } Request::RebuildFromText { project_path, source, @@ -6069,6 +6118,93 @@ fn do_sql_update( }) } +/// Worker handler for `Request::RunSqlDelete` (ADR-0033 §1/§7, +/// sub-phase 3f). Mirrors the DSL `do_delete` exactly, differing +/// only in that it executes the verbatim grammar-validated `sql` +/// rather than building the statement from a typed filter. +/// +/// Cascade detection (ADR-0033 Amendment 2): the worker snapshots +/// each inbound child table's row count *before* the DELETE, runs +/// the statement inside a transaction (the engine applies any +/// `ON DELETE CASCADE`), counts the children again *after*, and +/// reports the positive difference as a [`CascadeEffect`]. This is +/// the identical mechanism the DSL path uses, so the SQL and DSL +/// DELETE produce the same per-relationship summary on the same +/// schema/data — and since both return a [`DeleteResult`] routed +/// through `CommandOutcome::Delete`, the render-layer formatter is +/// shared with no duplication. `ON DELETE SET NULL` leaves row +/// counts unchanged and so is not reported on either path (a +/// deferred enhancement for both). +/// +/// Because the diff observes the result of executing the whole +/// statement, the WHERE clause is never inspected — a WHERE that +/// itself contains a subquery (the R2 invariant) is correct by +/// construction and carries no extra per-child query cost. +/// +/// Persistence discipline matches `do_delete` / `do_sql_update`: +/// re-persist the target's CSV *and every cascade-affected child's* +/// CSV via `finalize_persistence` (which also appends `source` to +/// `history.log`) *before* `tx.commit()`, so a persistence failure +/// rolls the delete back. A DELETE matching zero rows is a success +/// (`rows_affected == 0`, empty cascade); the target's CSV is still +/// re-persisted, keeping the path uniform. +fn do_sql_delete( + conn: &Connection, + persistence: Option<&Persistence>, + source: Option<&str>, + sql: &str, + target_table: &str, +) -> Result { + debug!(sql = %sql, table = %target_table, "sql_delete"); + + // Snapshot child-table row counts before the delete so cascade + // effects can be detected by diffing afterwards (Amendment 2; + // identical to `do_delete`). ON UPDATE CASCADE / ON DELETE SET + // NULL do not change row counts and so are not detected here. + let inbound = read_relationships_inbound(conn, target_table)?; + let mut before_counts: Vec = Vec::with_capacity(inbound.len()); + for r in &inbound { + before_counts.push(count_rows(conn, &r.other_table)?); + } + + let tx = conn + .unchecked_transaction() + .map_err(DbError::from_rusqlite)?; + let rows_affected = execute_with_fk_enrichment(conn, target_table, sql, &[])?; + + // Compare child-table counts after the delete; positive diffs + // are cascade effects. Collect the cascaded tables so the + // persistence phase rewrites their CSVs too. + let mut cascade: Vec = Vec::new(); + let mut rewritten_tables: Vec = vec![target_table.to_string()]; + for (rel, before_count) in inbound.iter().zip(before_counts.iter()) { + let after_count = count_rows(conn, &rel.other_table)?; + let diff = before_count - after_count; + if diff > 0 { + cascade.push(CascadeEffect { + relationship_name: rel.name.clone(), + child_table: rel.other_table.clone(), + rows_changed: diff, + action: rel.on_delete, + }); + rewritten_tables.push(rel.other_table.clone()); + } + } + + let changes = Changes { + schema_dirty: false, + rewritten_tables, + ..Changes::default() + }; + finalize_persistence(conn, persistence, source, &changes)?; + tx.commit().map_err(DbError::from_rusqlite)?; + + Ok(DeleteResult { + rows_affected, + cascade, + }) +} + /// Execute a grammar-validated SQL `SELECT` and collect its /// rows into a [`DataResult`] (ADR-0030 §6, ADR-0032 §12 + /// Amendment 1). diff --git a/src/dsl/command.rs b/src/dsl/command.rs index 6cecd0d..9651e08 100644 --- a/src/dsl/command.rs +++ b/src/dsl/command.rs @@ -320,6 +320,18 @@ pub enum Command { sql: String, target_table: String, }, + /// A SQL `DELETE` validated by the walker (ADR-0033 §1/§7, + /// advanced mode). Grammar-as-text: the worker executes `sql`, + /// observes any FK cascade by row-count diffing (Amendment 2 — + /// the same mechanism the DSL `do_delete` uses), and re-persists + /// `target_table`'s CSV plus every cascade-affected child + /// (ADR-0030 §11). The worker never inspects the WHERE clause, so + /// no predicate is carried here. `RETURNING` (3g) is added by the + /// sub-phase that reads it. + SqlDelete { + sql: String, + target_table: String, + }, /// App-lifecycle command (per ADR-0003). These work in both /// simple and advanced modes; the dispatcher branches on the /// `Command::App(...)` variant before mode-specific routing. @@ -617,6 +629,7 @@ impl Command { Self::Select { .. } => "select", Self::SqlInsert { .. } => "insert into", Self::SqlUpdate { .. } => "update", + Self::SqlDelete { .. } => "delete from", Self::App(app) => match app { AppCommand::Quit => "quit", AppCommand::Help => "help", @@ -687,7 +700,8 @@ impl Command { // A SQL `INSERT` carries its parsed target table (for // CSV re-persistence and ok-summary subject). Self::SqlInsert { target_table, .. } - | Self::SqlUpdate { target_table, .. } => target_table, + | Self::SqlUpdate { target_table, .. } + | Self::SqlDelete { target_table, .. } => target_table, // App commands aren't tied to schema entities — the // verb is the most identifying thing. The // display_subject override below provides a richer diff --git a/src/dsl/grammar/data.rs b/src/dsl/grammar/data.rs index cc4cd57..4430699 100644 --- a/src/dsl/grammar/data.rs +++ b/src/dsl/grammar/data.rs @@ -20,7 +20,7 @@ use crate::dsl::command::{Command, Expr, RowFilter}; use crate::dsl::grammar::{ CommandNode, IdentSource, Node, NumberValidator, ValidationError, Word, expr, shared::{column_value_list, current_column_value}, - sql_insert, sql_select, sql_update, + sql_delete, sql_insert, sql_select, sql_update, }; use crate::dsl::walker::context::WalkContext; use crate::dsl::value::Value; @@ -952,6 +952,39 @@ fn build_sql_update(path: &MatchedPath, source: &str) -> Result Result { + // The DELETE target is the first `table_name` ident (it precedes + // any table referenced inside a WHERE subquery). + let target_table = path + .items + .iter() + .find_map(|item| match item.kind { + MatchedKind::Ident { + role: "table_name", .. + } => Some(item.text.clone()), + _ => None, + }) + .unwrap_or_default(); + let tail = path + .items + .first() + .map_or(source, |entry| &source[entry.span.1..]); + let sql = format!("delete {}", tail.trim()); + Ok(Command::SqlDelete { sql, target_table }) +} + // ================================================================= // CommandNodes // ================================================================= @@ -1061,6 +1094,23 @@ pub static SQL_UPDATE: CommandNode = CommandNode { usage_ids: &[], }; +/// SQL `DELETE` development scaffold (ADR-0033 sub-phase 3f). +/// +/// Registered under the temporary entry word `sql_delete` so the +/// SQL DELETE grammar and execution path (including cascade-summary +/// parity) can be exercised in isolation, WITHOUT yet making +/// `delete` a shared DSL/SQL entry word. Sharing `delete` is +/// sub-phase 3j. This scaffold (entry word + reconstruction in +/// `build_sql_delete`) is removed when 3j wires the real `delete` +/// entry word. +pub static SQL_DELETE: CommandNode = CommandNode { + entry: Word::keyword("sql_delete"), + shape: Node::Subgrammar(&sql_delete::SQL_DELETE_SHAPE), + ast_builder: build_sql_delete, + help_id: None, + usage_ids: &[], +}; + // ================================================================= // Tests — `explain` grammar (ADR-0028 §1) // ================================================================= diff --git a/src/dsl/grammar/mod.rs b/src/dsl/grammar/mod.rs index 652dce8..dcb6d03 100644 --- a/src/dsl/grammar/mod.rs +++ b/src/dsl/grammar/mod.rs @@ -28,6 +28,7 @@ pub mod ddl; pub mod expr; pub mod shared; pub mod sql_expr; +pub mod sql_delete; pub mod sql_insert; pub mod sql_select; pub mod sql_update; @@ -580,6 +581,10 @@ pub static REGISTRY: &[(&CommandNode, CommandCategory)] = &[ // `sql_update` entry word keeps it isolated from the DSL // `update` word until 3j wires the shared entry. (&data::SQL_UPDATE, CommandCategory::Advanced), + // SQL DELETE development scaffold (sub-phase 3f); the temporary + // `sql_delete` entry word keeps it isolated from the DSL + // `delete` word until 3j wires the shared entry. + (&data::SQL_DELETE, CommandCategory::Advanced), ]; /// Whether `entry` names an advanced-mode-only command (ADR-0030 diff --git a/src/dsl/grammar/sql_delete.rs b/src/dsl/grammar/sql_delete.rs new file mode 100644 index 0000000..e122315 --- /dev/null +++ b/src/dsl/grammar/sql_delete.rs @@ -0,0 +1,145 @@ +//! SQL `DELETE` grammar (ADR-0033 §1/§7, sub-phase 3f). +//! +//! Grammar-as-text (ADR-0030 §4): the walker validates that the +//! `DELETE` is in the supported subset; the worker executes the +//! validated SQL text, observes any FK cascade by row-count diffing +//! (ADR-0033 Amendment 2), and re-persists the target table's CSV +//! plus every cascade-affected child (ADR-0030 §11). The shape here +//! is the post-`DELETE` portion — the entry-word dispatch consumes +//! the leading `DELETE` keyword before this shape walks, so the +//! shape opens at `FROM` (mirroring `sql_update::SQL_UPDATE_SHAPE`, +//! where the dev `sql_update` word stands in for `UPDATE`). +//! +//! Scope (3f): `FROM
[ WHERE … ] [ ';' ]`, the `__rdbms_*` +//! target rejection, and the shared `sql_expr` on the WHERE +//! predicate. There is no `--all-rows` rail — a SQL `DELETE` without +//! `WHERE` runs as written (ADR-0030 §12). `RETURNING` (3g) lands +//! later. The worker never inspects the WHERE clause (Amendment 2), +//! so no predicate-byte extraction is needed. + +use crate::dsl::grammar::sql_select::{WHERE_CLAUSE, reject_internal_table}; +use crate::dsl::grammar::{IdentSource, Node, Word}; + +/// The `DELETE` target table. `__rdbms_*` rejected (ADR-0030 §6 / +/// ADR-0033 §1). `writes_table` populates `current_table` / +/// `current_table_columns` so the WHERE predicate gets column +/// completion against the target. +/// +/// Uses the shared `table_name` role (not a bespoke one) so the +/// Phase-2 schema-existence + predicate-warning passes collect it +/// as a scope binding and check the WHERE columns against it for +/// free (ADR-0033 §2's "cross-cut from Phase-2 machinery"; the +/// handoff-31 §3e finding that a bespoke role leaves the WHERE +/// unchecked). +const TARGET_TABLE: Node = Node::Ident { + source: IdentSource::Tables, + role: "table_name", + validator: Some(reject_internal_table), + highlight_override: None, + writes_table: true, + writes_column: false, + writes_user_listed_column: false, + writes_table_alias: false, + writes_cte_name: false, + writes_projection_alias: false, +}; + +static SQL_DELETE_TAIL_NODES: &[Node] = &[ + Node::Word(Word::keyword("from")), + TARGET_TABLE, + Node::Optional(&WHERE_CLAUSE), + Node::Optional(&Node::Punct(';')), +]; + +/// The post-`DELETE` portion of a SQL `DELETE` statement +/// (ADR-0033 §1): `FROM
[ WHERE … ] [ ';' ]`. +/// +/// The entry-word dispatch consumes the leading `DELETE` keyword +/// before this shape walks, so a `CommandNode` references it as its +/// `shape` (sub-phase 3f registers a development entry word; +/// sub-phase 3j wires the shared `delete` entry word). +pub static SQL_DELETE_SHAPE: Node = Node::Seq(SQL_DELETE_TAIL_NODES); + +// ================================================================= +// Tests — grammar accept/reject for the post-`DELETE` tail. +// ================================================================= + +#[cfg(test)] +mod tests { + use super::SQL_DELETE_SHAPE; + use crate::dsl::walker::context::WalkContext; + use crate::dsl::walker::driver::{NodeWalkResult, walk_node}; + use crate::dsl::walker::outcome::MatchedPath; + + /// Walk `input` against the DELETE tail. Returns `true` only + /// when the walk matches *and* consumes all of `input` + /// (trailing whitespace allowed). Schemaless: the shape is + /// structural, so table/column idents match by shape and + /// `reject_internal_table` still fires on `__rdbms_*`. + fn walks(input: &str) -> bool { + let mut ctx = WalkContext::new(); + let mut path = MatchedPath::new(); + let mut per_byte = Vec::new(); + match walk_node(input, 0, &SQL_DELETE_SHAPE, &mut ctx, &mut path, &mut per_byte) { + NodeWalkResult::Matched { end, .. } => input[end..].trim().is_empty(), + _ => false, + } + } + + fn good(input: &str) { + assert!(walks(input), "{input:?} should be a valid DELETE tail"); + } + + fn bad(input: &str) { + assert!(!walks(input), "{input:?} should NOT walk as a complete DELETE tail"); + } + + #[test] + fn delete_with_where() { + good("from orders where id = 1"); + good("from orders where id = 1;"); + } + + #[test] + fn delete_without_where_runs_across_all_rows() { + // ADR-0030 §12: no `--all-rows` rail — a SQL DELETE without + // WHERE is structurally valid. + good("from orders"); + good("from orders;"); + } + + #[test] + fn where_admits_sql_expr() { + good("from orders where total > 100 and note is null"); + good("from orders where created < '2025-01-01'"); + } + + #[test] + fn where_admits_subquery_r2() { + // R2 invariant (ADR-0033 §7 / Amendment 2): the WHERE may + // itself contain a subquery. The shape admits it; the worker + // executes the verbatim statement and never extracts the + // predicate, so the nested subquery is just part of the SQL. + good("from orders where customer_id in (select id from customers where country = 'DE')"); + } + + #[test] + fn internal_target_table_rejected() { + bad("from __rdbms_playground_columns"); + bad("from __rdbms_playground_relationships where id = 1"); + } + + #[test] + fn structurally_incomplete_or_wrong_rejected() { + // Missing FROM. + bad("orders where id = 1"); + bad("orders"); + // FROM with no table. + bad("from"); + bad("from where id = 1"); + // Incomplete WHERE predicate. + bad("from orders where"); + // DELETE has no SET clause — trailing SET must not consume. + bad("from orders set v = 1"); + } +} diff --git a/src/runtime.rs b/src/runtime.rs index 313e6bd..424564c 100644 --- a/src/runtime.rs +++ b/src/runtime.rs @@ -1902,6 +1902,16 @@ async fn execute_command_typed( .run_sql_update(sql, src, target_table) .await .map(CommandOutcome::Update), + // A SQL `DELETE` (advanced mode; ADR-0033 §1/§7). Grammar- + // as-text: the worker runs the validated `sql`, detects FK + // cascade by row-count diffing, and re-persists the target + // plus every cascade-affected child. Reuses the DSL delete + // outcome (affected-row count + per-relationship cascade + // summary). + Command::SqlDelete { sql, target_table } => database + .run_sql_delete(sql, src, target_table) + .await + .map(CommandOutcome::Delete), // `EXPLAIN QUERY PLAN` never executes the wrapped // statement (ADR-0028 §2), so explaining a destructive // command is safe. `src` is unused here — explain is a diff --git a/tests/sql_delete.rs b/tests/sql_delete.rs new file mode 100644 index 0000000..66b68fa --- /dev/null +++ b/tests/sql_delete.rs @@ -0,0 +1,392 @@ +//! Sub-phase 3f integration tests for the advanced-mode SQL +//! `DELETE` surface (ADR-0033 §1/§7). +//! +//! Covers the parse path (the dev `sql_delete` scaffold lowers to +//! `Command::SqlDelete`, reconstructing valid `delete from …` SQL) +//! and the worker round-trip (execute, detect FK cascade by +//! row-count diffing per ADR-0033 Amendment 2, re-persist the +//! target *and every cascade-affected child* CSV, append +//! `history.log`). A SQL `DELETE` without `WHERE` runs across all +//! rows with no rail (ADR-0030 §12). +//! +//! The cascade tests pin the Amendment-2 decision: the SQL path +//! uses the *same* count-diff mechanism as the DSL `do_delete`, so +//! the two produce identical `DeleteResult.cascade` on identical +//! schema/data (the `cascade_parity_with_dsl` test asserts this +//! directly). The R2 invariant — a WHERE that itself contains a +//! subquery — is correct by construction because the verbatim SQL +//! executes once and the diff observes the result; no predicate +//! bytes are extracted. + +use rdbms_playground::db::{Database, DbError, DeleteResult}; +use rdbms_playground::dsl::{ + ColumnSpec, Command, ReferentialAction, RowFilter, Type, Value, parse_command, +}; +use rdbms_playground::persistence::Persistence; +use rdbms_playground::project; + +fn rt() -> tokio::runtime::Runtime { + tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .expect("tokio rt") +} + +fn open_project_db() -> (project::Project, Database, tempfile::TempDir) { + let dir = tempfile::tempdir().expect("create tempdir"); + let project = + project::open_or_create(None, Some(dir.path())).expect("open or create project"); + let persistence = Persistence::new(project.path().to_path_buf()); + let db = Database::open_with_persistence(project.db_path(), persistence) + .expect("open db with persistence"); + (project, db, dir) +} + +fn read_csv(project: &project::Project, table: &str) -> Option { + std::fs::read_to_string(project.path().join("data").join(format!("{table}.csv"))).ok() +} + +fn create_cols( + db: &Database, + rt: &tokio::runtime::Runtime, + name: &str, + cols: &[(&str, Type)], + pk: &[&str], +) { + rt.block_on(db.create_table( + name.to_string(), + cols.iter().map(|(n, t)| ColumnSpec::new(*n, *t)).collect(), + pk.iter().map(|s| (*s).to_string()).collect(), + None, + )) + .unwrap_or_else(|e| panic!("create table {name}: {e:?}")); +} + +/// Seed via the SQL INSERT worker path (no shortid columns here, so +/// it executes verbatim). +fn seed(db: &Database, rt: &tokio::runtime::Runtime, sql: &str, target: &str) { + rt.block_on(db.run_sql_insert( + sql.to_string(), + None, + target.to_string(), + Vec::new(), + String::new(), + )) + .unwrap_or_else(|e| panic!("seed {sql:?}: {e:?}")); +} + +/// Full-stack: parse the dev `sql_delete …` scaffold and run it. +fn run_delete( + db: &Database, + rt: &tokio::runtime::Runtime, + input: &str, +) -> Result { + match parse_command(input).expect("parse sql_delete") { + Command::SqlDelete { sql, target_table } => { + rt.block_on(db.run_sql_delete(sql, Some(input.to_string()), target_table)) + } + other => panic!("expected Command::SqlDelete, got {other:?}"), + } +} + +/// `Customers (id int pk, Name text)` parent and `Orders (id int +/// pk, CustId int)` child, with `Customers.id → Orders.CustId` +/// `ON DELETE CASCADE`. Seeds Alice (1) with two orders (10, 11) +/// and Bob (2) with one order (12). +fn cascade_fixture(db: &Database, rt: &tokio::runtime::Runtime) { + create_cols(db, rt, "Customers", &[("id", Type::Int), ("Name", Type::Text)], &["id"]); + create_cols(db, rt, "Orders", &[("id", Type::Int), ("CustId", Type::Int)], &["id"]); + rt.block_on(db.add_relationship( + Some("places".to_string()), + "Customers".to_string(), + "id".to_string(), + "Orders".to_string(), + "CustId".to_string(), + ReferentialAction::Cascade, + ReferentialAction::NoAction, + false, + None, + )) + .expect("add cascade relationship"); + seed(db, rt, "insert into Customers (id, Name) values (1, 'Alice'), (2, 'Bob')", "Customers"); + seed(db, rt, "insert into Orders (id, CustId) values (10, 1), (11, 1), (12, 2)", "Orders"); +} + +#[test] +fn parse_path_lowers_sql_delete_to_command() { + let command = parse_command("sql_delete from Orders where id = 1") + .expect("sql_delete parses in advanced mode"); + match command { + Command::SqlDelete { sql, target_table } => { + assert_eq!(sql, "delete from Orders where id = 1"); + assert_eq!(target_table, "Orders"); + } + other => panic!("expected Command::SqlDelete, got {other:?}"), + } +} + +#[test] +fn delete_with_where_persists() { + let (project, db, _dir) = open_project_db(); + let rt = rt(); + create_cols(&db, &rt, "t", &[("id", Type::Int), ("v", Type::Text)], &["id"]); + seed(&db, &rt, "insert into t (id, v) values (1, 'gone'), (2, 'keep')", "t"); + let result = run_delete(&db, &rt, "sql_delete from t where id = 1").expect("delete runs"); + assert_eq!(result.rows_affected, 1, "one row deleted"); + assert!(result.cascade.is_empty(), "no children, no cascade"); + let csv = read_csv(&project, "t").expect("t.csv"); + assert!(csv.contains("keep"), "untouched row preserved: {csv:?}"); + assert!(!csv.contains("gone"), "deleted row removed from CSV: {csv:?}"); +} + +#[test] +fn delete_without_where_runs_across_all_rows() { + // ADR-0030 §12: no `--all-rows` rail. + let (project, db, _dir) = open_project_db(); + let rt = rt(); + create_cols(&db, &rt, "t", &[("id", Type::Int), ("v", Type::Text)], &["id"]); + seed(&db, &rt, "insert into t (id, v) values (1, 'a'), (2, 'b'), (3, 'c')", "t"); + let result = run_delete(&db, &rt, "sql_delete from t").expect("unfiltered delete runs"); + assert_eq!(result.rows_affected, 3, "all rows deleted"); + // Empty tables produce no CSV (CLAUDE.md persistence note), so the + // file is either absent or has only a header — either way, no data. + let csv = read_csv(&project, "t").unwrap_or_default(); + assert!(!csv.contains('a') && !csv.contains('b') && !csv.contains('c'), "no rows left: {csv:?}"); + let remaining = rt + .block_on(db.query_data("t".to_string(), None, None, None)) + .expect("query t"); + assert!(remaining.rows.is_empty(), "table empty after unfiltered delete"); +} + +#[test] +fn cascade_delete_reports_summary_and_repersists_child() { + let (project, db, _dir) = open_project_db(); + let rt = rt(); + cascade_fixture(&db, &rt); + // Delete Alice (customer 1) — cascades to her two orders (10, 11). + let result = run_delete(&db, &rt, "sql_delete from Customers where id = 1") + .expect("cascading delete runs"); + assert_eq!(result.rows_affected, 1, "one parent row deleted"); + assert_eq!(result.cascade.len(), 1, "one cascade relationship reported"); + let effect = &result.cascade[0]; + assert_eq!(effect.relationship_name, "places"); + assert_eq!(effect.child_table, "Orders"); + // rows_changed == 2 pins the before-execute ordering: counted + // after the delete it would be 0. Alice had exactly two orders. + assert_eq!(effect.rows_changed, 2, "both of Alice's orders cascaded"); + // The child CSV must be re-persisted to reflect the cascade. + let orders_csv = read_csv(&project, "Orders").expect("Orders.csv"); + assert!(orders_csv.contains("12"), "Bob's order (12) preserved: {orders_csv:?}"); + assert!(!orders_csv.contains("10") && !orders_csv.contains("11"), + "Alice's cascaded orders gone from CSV: {orders_csv:?}"); +} + +#[test] +fn cascade_parity_with_dsl() { + // ADR-0033 §2 / Amendment 2: the SQL DELETE cascade summary must + // match the DSL `do_delete` output on the same schema/data — + // because they use the identical count-diff mechanism. Run the + // same operation through both paths on two identical fixtures and + // compare the cascade vectors directly (CascadeEffect: PartialEq). + let rt = rt(); + + let (_p_sql, db_sql, _d_sql) = open_project_db(); + cascade_fixture(&db_sql, &rt); + let sql_result = run_delete(&db_sql, &rt, "sql_delete from Customers where id = 1") + .expect("SQL delete runs"); + + let (_p_dsl, db_dsl, _d_dsl) = open_project_db(); + cascade_fixture(&db_dsl, &rt); + let dsl_result = rt + .block_on(db_dsl.delete( + "Customers".to_string(), + RowFilter::eq("id", Value::Number("1".to_string())), + Some("delete from Customers where id = 1".to_string()), + )) + .expect("DSL delete runs"); + + assert_eq!(sql_result.rows_affected, dsl_result.rows_affected, "row counts match"); + assert_eq!(sql_result.cascade, dsl_result.cascade, "cascade summaries identical"); +} + +#[test] +fn r2_where_with_subquery() { + // R2 invariant (ADR-0033 §7 / Amendment 2): a WHERE containing a + // subquery. Plan shape: `DELETE FROM orders WHERE customer_id IN + // (SELECT id FROM customers WHERE …)`. The verbatim statement + // executes once; no predicate extraction. Orders has no children, + // so cascade is empty — the point is the subquery resolves. + let (project, db, _dir) = open_project_db(); + let rt = rt(); + cascade_fixture(&db, &rt); + let result = run_delete( + &db, + &rt, + "sql_delete from Orders where CustId in (select id from Customers where Name = 'Alice')", + ) + .expect("subquery-WHERE delete runs"); + assert_eq!(result.rows_affected, 2, "Alice's two orders deleted"); + assert!(result.cascade.is_empty(), "Orders has no cascade children"); + let orders_csv = read_csv(&project, "Orders").expect("Orders.csv"); + assert!(orders_csv.contains("12"), "Bob's order preserved: {orders_csv:?}"); + assert!(!orders_csv.contains("10") && !orders_csv.contains("11"), + "Alice's orders deleted: {orders_csv:?}"); +} + +#[test] +fn r2_cascade_with_subquery_where() { + // The strongest R2 case: the parent is the DELETE target AND the + // WHERE subquery reads the very child table that will be cascade- + // deleted. The engine evaluates the subquery against pre-delete + // state, deletes the matched parent, then cascades — and the + // count-diff observes the child rows that vanished. Pins both the + // subquery correctness and the before-execute ordering together. + let (project, db, _dir) = open_project_db(); + let rt = rt(); + cascade_fixture(&db, &rt); + // order 11 belongs to Alice (CustId 1); the subquery yields 1, so + // Alice is deleted and BOTH her orders (10, 11) cascade. + let result = run_delete( + &db, + &rt, + "sql_delete from Customers where id in (select CustId from Orders where id = 11)", + ) + .expect("cascade + subquery-WHERE delete runs"); + assert_eq!(result.rows_affected, 1, "Alice deleted"); + assert_eq!(result.cascade.len(), 1, "one cascade relationship"); + assert_eq!(result.cascade[0].rows_changed, 2, "both Alice orders cascaded"); + let orders_csv = read_csv(&project, "Orders").expect("Orders.csv"); + assert!(orders_csv.contains("12") && !orders_csv.contains("10") && !orders_csv.contains("11"), + "only Bob's order remains: {orders_csv:?}"); +} + +#[test] +fn delete_appends_literal_line_to_history() { + let (project, db, _dir) = open_project_db(); + let rt = rt(); + create_cols(&db, &rt, "t", &[("id", Type::Int), ("v", Type::Text)], &["id"]); + seed(&db, &rt, "insert into t (id, v) values (1, 'x')", "t"); + let input = "sql_delete from t where id = 1"; + run_delete(&db, &rt, input).expect("delete runs"); + let body = std::fs::read_to_string(project.path().join("history.log")) + .expect("history.log present"); + assert!(body.contains(input), "history records the literal line: {body:?}"); +} + +#[test] +fn cascade_to_two_children_reports_both() { + // DA gate (untested branch): a parent with TWO cascade children + // must emit a CascadeEffect per affected child, and re-persist + // both. The single-relationship tests never exercise the loop + // emitting more than one effect. + let (project, db, _dir) = open_project_db(); + let rt = rt(); + create_cols(&db, &rt, "Customers", &[("id", Type::Int), ("Name", Type::Text)], &["id"]); + create_cols(&db, &rt, "Orders", &[("id", Type::Int), ("CustId", Type::Int)], &["id"]); + create_cols(&db, &rt, "Reviews", &[("id", Type::Int), ("CustId", Type::Int)], &["id"]); + for (child, name) in [("Orders", "places"), ("Reviews", "writes")] { + rt.block_on(db.add_relationship( + Some(name.to_string()), + "Customers".to_string(), + "id".to_string(), + child.to_string(), + "CustId".to_string(), + ReferentialAction::Cascade, + ReferentialAction::NoAction, + false, + None, + )) + .unwrap_or_else(|e| panic!("add rel {name}: {e:?}")); + } + seed(&db, &rt, "insert into Customers (id, Name) values (1, 'Alice')", "Customers"); + seed(&db, &rt, "insert into Orders (id, CustId) values (10, 1), (11, 1)", "Orders"); + seed(&db, &rt, "insert into Reviews (id, CustId) values (20, 1)", "Reviews"); + + let result = run_delete(&db, &rt, "sql_delete from Customers where id = 1") + .expect("cascade-to-two delete runs"); + assert_eq!(result.rows_affected, 1); + assert_eq!(result.cascade.len(), 2, "both cascade relationships reported"); + let by_child: std::collections::HashMap<&str, i64> = result + .cascade + .iter() + .map(|e| (e.child_table.as_str(), e.rows_changed)) + .collect(); + assert_eq!(by_child.get("Orders"), Some(&2), "two orders cascaded"); + assert_eq!(by_child.get("Reviews"), Some(&1), "one review cascaded"); + // Both child CSVs re-persisted to the post-cascade (empty) state. + let orders = rt.block_on(db.query_data("Orders".to_string(), None, None, None)).unwrap(); + let reviews = rt.block_on(db.query_data("Reviews".to_string(), None, None, None)).unwrap(); + assert!(orders.rows.is_empty() && reviews.rows.is_empty(), "both children emptied"); + let _ = &project; +} + +#[test] +fn delete_childless_parent_reports_no_cascade() { + // DA gate (untested branch): a cascade relationship EXISTS, but + // the deleted parent row has no children. The `diff > 0` guard + // must yield an empty cascade and must NOT touch the child's CSV + // (a `>= 0` regression would report a phantom 0-row cascade). + let (project, db, _dir) = open_project_db(); + let rt = rt(); + cascade_fixture(&db, &rt); + // Carol (3) exists with no orders; deleting her cascades nothing. + seed(&db, &rt, "insert into Customers (id, Name) values (3, 'Carol')", "Customers"); + let result = run_delete(&db, &rt, "sql_delete from Customers where id = 3") + .expect("childless-parent delete runs"); + assert_eq!(result.rows_affected, 1, "Carol deleted"); + assert!(result.cascade.is_empty(), "no children → no cascade effect reported"); + // All three orders untouched. + let orders_csv = read_csv(&project, "Orders").expect("Orders.csv"); + assert!( + orders_csv.contains("10") && orders_csv.contains("11") && orders_csv.contains("12"), + "no order touched by a childless-parent delete: {orders_csv:?}" + ); +} + +#[test] +fn delete_violating_fk_fails_and_persists_nothing() { + // DA gate (untested error path): with an ON DELETE NO ACTION + // child, deleting a referenced parent is rejected by the engine. + // `do_sql_delete` runs persistence+history INSIDE the tx AFTER a + // successful execute, so a rejected delete must roll back: the + // parent row survives and history records no line. + let (project, db, _dir) = open_project_db(); + let rt = rt(); + create_cols(&db, &rt, "Customers", &[("id", Type::Int), ("Name", Type::Text)], &["id"]); + create_cols(&db, &rt, "Orders", &[("id", Type::Int), ("CustId", Type::Int)], &["id"]); + rt.block_on(db.add_relationship( + Some("places".to_string()), + "Customers".to_string(), + "id".to_string(), + "Orders".to_string(), + "CustId".to_string(), + ReferentialAction::NoAction, // on delete: reject if referenced + ReferentialAction::NoAction, + false, + None, + )) + .expect("add NO ACTION relationship"); + seed(&db, &rt, "insert into Customers (id, Name) values (1, 'Alice')", "Customers"); + seed(&db, &rt, "insert into Orders (id, CustId) values (10, 1)", "Orders"); + + let input = "sql_delete from Customers where id = 1"; + let result = run_delete(&db, &rt, input); + assert!(result.is_err(), "delete of a referenced parent must be rejected"); + // Rolled back: Alice survives. + let customers = rt.block_on(db.query_data("Customers".to_string(), None, None, None)).unwrap(); + assert_eq!(customers.rows.len(), 1, "parent row preserved after rejected delete"); + // No history line for the failed statement (written only on success). + let history = std::fs::read_to_string(project.path().join("history.log")).unwrap_or_default(); + assert!(!history.contains(input), "failed delete not logged: {history:?}"); +} + +#[test] +fn internal_target_table_rejected_at_parse() { + // ADR-0030 §6 / ADR-0033 §1: the `__rdbms_*` metadata tables are + // rejected at the target slot — the parse fails, the statement + // never reaches the worker. + assert!( + parse_command("sql_delete from __rdbms_playground_columns").is_err(), + "internal table must be rejected at the DELETE target slot" + ); +} diff --git a/tests/typing_surface/mod.rs b/tests/typing_surface/mod.rs index 81bd2f0..2ad3b97 100644 --- a/tests/typing_surface/mod.rs +++ b/tests/typing_surface/mod.rs @@ -221,6 +221,7 @@ fn command_kind_label(cmd: &rdbms_playground::dsl::Command) -> String { Select { .. } => "Select".into(), SqlInsert { .. } => "SqlInsert".into(), SqlUpdate { .. } => "SqlUpdate".into(), + SqlDelete { .. } => "SqlDelete".into(), App(app) => match app { AppCommand::Quit => "App(Quit)".into(), AppCommand::Help => "App(Help)".into(),