From f62cccec559eda222d258b345a05ed439d9df7a7 Mon Sep 17 00:00:00 2001 From: "claude@clouddev1" Date: Sat, 30 May 2026 18:44:05 +0000 Subject: [PATCH] feat: support explain over advanced-mode SQL queries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit explain now wraps the advanced SQL commands — select, with (CTE), insert, update, delete — in addition to the DSL show data/update/ delete it already covered, rendering through the same plan tree (ADR-0039, closing the ADR-0030 OOS-2 gap). Implemented as a second Advanced `explain` CommandNode under the shared entry word, reusing the established shared-word dispatch (SQL-first, DSL-fallback) rather than new grammar machinery. build_explain_sql slices the inner SQL off the source and reuses the existing SQL builders; do_explain_plan runs EXPLAIN QUERY PLAN over the carried text verbatim (never executes, so safe for destructive verbs). Advanced explain update/delete now route through SQL with an identical plan; DSL-explain tests pinned to simple mode. Help and usage text now list the advanced explain forms. --- CLAUDE.md | 6 +- docs/adr/0039-explain-over-advanced-sql.md | 50 +++- docs/adr/README.md | 2 +- docs/requirements.md | 6 +- src/db.rs | 166 +++++++++++++ src/dsl/grammar/data.rs | 259 ++++++++++++++++++++- src/dsl/grammar/mod.rs | 25 ++ src/friendly/strings/en-US.yaml | 3 + 8 files changed, 503 insertions(+), 14 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index f4d72ca..c5629db 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -101,8 +101,10 @@ Current decisions at a glance (each backed by an ADR): - **Indexes & query plans:** `add index` / `drop index` (ADR-0025); `explain show data|update|delete` runs `EXPLAIN QUERY PLAN` and renders an annotated, span-styled - plan tree (ADR-0028). `EXPLAIN QUERY PLAN` never executes, - so explaining a destructive command is safe. + plan tree (ADR-0028). In advanced mode `explain` also wraps + SQL `select` / `with` / `insert` / `update` / `delete` + (ADR-0039). `EXPLAIN QUERY PLAN` never executes, so + explaining a destructive command is safe. ## Repository layout diff --git a/docs/adr/0039-explain-over-advanced-sql.md b/docs/adr/0039-explain-over-advanced-sql.md index 5aca38f..9116031 100644 --- a/docs/adr/0039-explain-over-advanced-sql.md +++ b/docs/adr/0039-explain-over-advanced-sql.md @@ -2,11 +2,8 @@ ## Status -**Accepted** — decision recorded 2026-05-27. **Implementation deferred** -as a follow-up to the ADR-0037/ADR-0038 teaching-echo effort: the -decision below is settled, but the full design has not been `/runda`'d -or built, and is *not* part of that pass. **Supersedes ADR-0030 §13 -OOS-2.** +**Accepted** — decision recorded 2026-05-27. **Implemented 2026-05-30** +(issue #7; see Implementation below). **Supersedes ADR-0030 §13 OOS-2.** ## Context @@ -51,6 +48,49 @@ modes, unchanged. **Supersedes ADR-0030 §13 OOS-2.** Built test-first when picked up. +## Implementation (2026-05-30) + +Built as designed, with the mode-gating and DSL/SQL disambiguation +handled by the **existing shared-entry-word dispatch** rather than any +new grammar machinery: + +- **Two `explain` CommandNodes under one entry word.** The original + `data::EXPLAIN` (`Simple`, DSL inner) is unchanged. A new + `data::EXPLAIN_SQL` (`Advanced`) carries `EXPLAIN_SQL_SHAPE` — a + `Choice` over `select` / `with` / `insert` / `update` / `delete`, + each `[Word, Subgrammar(&SQL_*_SHAPE)]` reusing the standalone SQL + command shapes. Both register under `explain` in `REGISTRY`, exactly + mirroring the `insert`/`update`/`delete` shared-word pattern + (ADR-0033 §2). The walker's `decide` then does all the work: advanced + mode tries `EXPLAIN_SQL` first and falls back to the DSL `EXPLAIN` + when no SQL branch matches (`explain show data …`, or a DSL-only + `--all-rows`); simple mode reaches only the DSL node. **`with` (CTE) + is included** — it builds a `Command::Select`, in scope per the + decision's AST naming. +- **Rejected `DynamicSubgrammar` mode-gating.** A factory reading + `ctx.mode` would be memoised wrongly: the dynamic-resolution cache + key omits `mode`, so a node resolved in one mode would be served back + in the other. The two-CommandNode route avoids this and stays on the + established dispatch path. +- **Clean inner SQL.** `build_explain_sql` slices the inner SQL text + from the source starting at the inner entry keyword's span (so the + carried text excludes `explain`), then delegates to the existing + `build_select` / `build_sql_*` builders. Their metadata extraction + (target table, etc.) reads the path by role, which is offset- + independent, so wrapping is transparent. +- **Execution.** `do_explain_plan` gains arms for + `Select` / `SqlInsert` / `SqlUpdate` / `SqlDelete` that run + `EXPLAIN QUERY PLAN` over the carried SQL text verbatim, no bound + params (grammar-as-text). `display_sql` is the user's text as written + (the DSL path canonicalises only because it *synthesises* SQL). The + ADR-0028 renderer is reused unchanged. +- **Behaviour note.** In advanced mode `explain update …` / + `explain delete …` now route through the SQL path (previously the + DSL inner). The plan is identical (§6/§7 parity), and the SQL grammar + accepts the full SQL syntax the DSL grammar rejected. DSL-explain + tests were pinned to simple mode; advanced SQL wrapping has its own + tests. + ## Out of scope - **EXPLAIN of DDL** (`CREATE` / `ALTER` / `DROP`). `EXPLAIN QUERY PLAN` diff --git a/docs/adr/README.md b/docs/adr/README.md index 608b6a5..581ade8 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -44,4 +44,4 @@ This directory contains the project's ADRs, recorded per - [ADR-0036 — Value validation for advanced-mode DML](0036-typed-dml-values-vs-verbatim.md) — **Accepted** (design agreed + `/runda`'d 2026-05-26; mechanism then **deliberately narrowed** from "bind literals via the DSL path" to surgical **"validate-and-retain, execute verbatim"** after the user resisted consolidating the modes and a concrete auto-fill difference confirmed even the single-row literal case isn't identical across modes; **Phases 1–2 implemented** 2026-05-26 — `INSERT … VALUES` and `UPDATE … SET` literal validation + offending-value retention, capture-at-parse with no grammar change; **Phase 3a implemented** 2026-05-26 — live typed-slot hints + numeric-shape highlighting for `UPDATE`/UPSERT `SET col = ` via a boundary-aware lookahead (Amendment 1 corrects this ADR's naive-`Choice` sketch); **Phase 3b implemented** 2026-05-27 — per-position typed slots for `INSERT … VALUES` (single/multi-row, Form A/B) via a new zero-width `Node::SetColumn` primitive + an arity-gating tuple lookahead that preserves the §8.1 arity diagnostic; **fully implemented**). **Augments — does NOT supersede — ADR-0030 §4 / ADR-0033 §10**: execution stays verbatim, ADR-0033 Amendment 3's two-command identity (`Insert` vs `SqlInsert`) **stands**. The problem (investigated 2026-05-26; characterization test `sql_insert.rs::sql_dml_skips_app_level_value_validation_that_the_dsl_enforces` proves it): advanced-mode SQL DML gets **none** of the DSL's value feedback — a malformed `date` like `2025/01/15` is silently written, and the offending value is missing from constraint errors — because literal values are spliced into text and discarded (only `STRICT` storage types check them). **Fix (surgical): validate each literal value against its column type before the verbatim insert, and retain it for error reporting — sharing only the per-type validators (`Value::bind_for_column`/`validate_date`/`shortid::validate`), nothing else.** No binding, no statement reconstruction, no auto-fill change, no command-identity collapse — because the two gaps are closed by validation + retention alone, and executing the user's own text is already safe. The literal set = `NULL`/boolean/string/**signed**-numeric; arithmetic/functions/subqueries/column-refs are expressions (skipped — the engine evaluates them). `WHERE` not validated (it's an expression in general; motivation met by `VALUES`/`SET`); `SELECT`/`INSERT … SELECT`/`RETURNING`/`ON CONFLICT` need no special handling since execution is untouched. Phased: **Phase 1** capture-at-parse + validate + retain for `INSERT … VALUES` (no grammar change, no reparse — closes both proven gaps); **Phase 2** `UPDATE … SET` literals; **Phase 3** completion hinting/highlighting (the only part needing a grammar change — a typed-literal slot vs `sql_expr` reusing the DSL `TypedValueSlot`s at `data.rs:141`/`189`/`269`, discriminated by a **boundary-aware lookahead** not a naive `Choice` per **Amendment 1**; split into **3a** `SET` (done) and **3b** `VALUES` (pending); supersedes only Phase 1/2's literal *detection*, not the validation/enrichment on top). Non-goals: binding/reconstruction, collapsing command identity (Am3 stands), changing `serial`/`shortid` auto-fill (`requirements.md` **X4**, a separate possible-bug), a structural `SELECT`, a full SQL-expression AST. Embodies `requirements.md` **X5** (share a *mechanic*, not a *command*); the neutral "that value" safety net (ADR-0035 Amendment 1) stays correct for genuinely-computed values; **Amendment 2 (issue #17, 2026-05-29)** brings the §8.1 arity diagnostic to **simple mode** at parity with advanced: a `tuple_value_list`-style gate (`dsl_insert_value_list`, **simple-mode-gated** so advanced is byte-for-byte unchanged) routes a wrong-count DSL insert tuple to the type-blind fallback so it matches and the friendly arity diagnostic fires (instead of a bare `expected `,`/`)``); `dml_insert_arity_diagnostics` is now mode-aware (advanced Form B = all columns, simple Form B/C = user-fillable since serial/shortid auto-fill, ADR-0018 §3), counts the DSL Form A role (`insert_first_item`) and the keyword-less Form C tuple, with new keys `insert_arity_mismatch_form_b_simple` / `_all_auto`; a wrong-count DSL insert now parses `Ok` + carries the ERROR diagnostic (the `[ERR]` verdict), with a unified Ok-arm submit pre-flight (`dsl_insert_count_mismatch_notes`) blocking dispatch + teaching (the issue #1 Err-arm note retires). **Arity-UX parity only — no consolidation of value-handling/execution/auto-fill; the deliberate mode-distinctness stands** - [ADR-0037 — Execution-time mode side-channel (the three-way submission mode)](0037-execution-time-mode-side-channel.md) — **Accepted** (design agreed 2026-05-27; channel **implemented + verified end-to-end** by its motivating consumer — ADR-0038's fully-shipped DSL → SQL teaching echo — across handoff-46 `04c8e42` (channel + first echo slice), handoff-47 `90479cb` (full Bucket A), `275c726` (Bucket B resolved-name + multi-statement renderers), `e6ad1ae` (the category-3 `--dont-convert` caveat — gated on this channel too), and `2aab457` (the §4 styled-runs rendering polish)), **redeems the follow-up deferred by ADR-0033 Amendment 3** (which named this ADR and its motivating consumer). Establishes the channel that lets a command know, **at execution time**, the effective mode it ran under — so execution can adjust **output** without touching **identity** (the motivating case: a DSL-form `create table` echoing the equivalent SQL when run in advanced mode, silent in simple — ADR-0030 §10, realised by ADR-0038). Introduces a **new per-submission enum `SubmissionMode` { Simple, Advanced, AdvancedOneShot }** — *refining* Amendment 3's "widen `Mode`" sketch: the persistent input `Mode` stays **two-way** (`mode.rs` keeps the one-shot `:` out of persistent state), and the three-way distinction lives on the per-submission channel where the transient `:` belongs. Resolved at submit time (Simple+`:` → `AdvancedOneShot`; Advanced `:` is a no-op), threaded through `Action::ExecuteDsl` → worker, **output-only** (no executor branches its *effect* on it — Amendment 3 forbids behavioural mode dependence). The worker builds the teaching echo (+ category-3 expansion data — ADR-0038) for DSL-form commands in advanced/one-shot mode and returns it; the App renders it beneath `[ok]`. Co-located with execution because the echo's harder forms (resolved auto-names, generated `shortid`s, conversion counts) are worker-computed facts, and gating on mode means the work happens only when shown. Alternatives weighed + rejected: widening `Mode` (conflates transient/persistent state); App-side gating with the worker always emitting echo data (computes unconditionally, doesn't generalise, re-opens the render-side framing ruled against). Scope: channel + resolution rule only — the renderer/catalogue/`Value → SQL-literal` are ADR-0038, the `ALTER COLUMN` gap-fill is the ADR-0035 amendment - [ADR-0038 — The DSL → SQL teaching echo](0038-dsl-to-sql-teaching-echo.md) — **Accepted** (design agreed 2026-05-27; **fully implemented + verified** — every catalogue row in §7 Buckets A + B and the §6 category-3 prose round-trips per line through the advanced walker per §1, and the §4 de-emphasised styled-runs polish is wired: handoff-46 `04c8e42` shipped the channel + create-table slice, handoff-47 `90479cb` the full Bucket A expansion + a skeleton contract-gap fix (dropped per-column `DEFAULT`/`CHECK`), `275c726` the Bucket B resolved-name + multi-statement renderers (auto- and user-named `add index`, positional `drop index`, `add`/`drop relationship` in both selector forms, `drop column --cascade`, `add relationship --create-fk`), `e6ad1ae` the last category-3 line — the `change column --dont-convert` *caveat* (shortid + transform notes were already surfaced via pre-existing `client_side.*` keys), and `2aab457` the §4 styled-runs polish: a new `OutputKind::TeachingEcho` custom rendering branch (dimmed `Executing SQL:` prefix + the SQL re-lexed in advanced mode for token-class colouring, same as the input echo) plus a new `OutputStyleClass::Hint` for every cat-3 prose line — caveat *and* the existing illuminating notes, user-confirmed broader scope), **realises ADR-0030 §10** (the teaching bridge) — the Phase-5 echo **ADR-0035 §12 forward-referenced** — building on **ADR-0037** (the `SubmissionMode` gate) and **ADR-0035 Amendment 2** (standard-first dialect + `ALTER COLUMN` gap-fill). When a **DSL-form** command runs in advanced/one-shot mode, the worker emits the equivalent SQL beneath `[ok]` as a de-emphasised styled `OutputLine` (ADR-0028); the App renders it. **Defining invariant — the copy-paste contract:** every echoed line is *runnable advanced-mode SQL* (round-trip-tested: parse the echo → same-effect command; a planned "copy the echo" affordance depends on it). **Type vocabulary = the playground's own keywords** (`serial`/`shortid`/…, accepted by `from_sql_name`, decision (a)); **statement shape = the standard-first dialect** (Am2). **DML uses substituted literals, not `?`** (per-type `Value → SQL-literal`, round-trip-safe; `blob` moot — no literal syntax exists; auto-gen columns omitted to match `do_insert` + X4). **Firing reality — a DDL + `show data` feature:** in advanced mode `insert`/`update`/`delete … where` are SQL-first (`Sql*` = already SQL = nothing to echo per §10); only DSL-*only* spellings echo (DDL + `show data` + the `delete`/`update … --all-rows` fall-throughs — the latter via **ADR-0033 Amendment 4**, a bug-fix folded in here that reverses Amendment 3's `update … --all-rows` misparse). **Three-category framework** for "what happens beyond the literal SQL": **(1) engine-implementation-hiding** (the rebuild, rowid PK, non-PK `serial` MAX+1) — *never surfaced*; **(2) decomposable into advanced SQL** (`drop column --cascade`, `--create-fk` relationship) — *shown as the runnable multi-line sequence, one statement per line*; **(3) playground type-behaviour with no SQL-expressible form** (`shortid` generation — no `shortid()`; type-conversion transforms — no `USING`) — *de-emphasised prose expansion from the worker's `client_side.*` notes*. Carries the **full catalogue** (Buckets A single-statement / B resolved-name + multi-line / C no-echo) mapping every DSL-form command to its echo. OOS: reverse SQL→DSL echo (§13 OOS-5), app commands / `show table` / `explain` / `replay`, a `blob` literal, the column-level UNIQUE/CHECK drop residual (Bucket C until Am2's gap closes), and surfacing any category-1 engine internal -- [ADR-0039 — EXPLAIN over advanced-mode SQL queries](0039-explain-over-advanced-sql.md) — **Accepted** (decision recorded 2026-05-27; **implementation deferred** as a follow-up to the ADR-0037/0038 echo effort — not in that pass), **supersedes ADR-0030 §13 OOS-2**. Lets `explain` wrap the advanced SQL commands (`Select`/`SqlInsert`/`SqlUpdate`/`SqlDelete`) in addition to the DSL `ShowData`/`Update`/`Delete` it already covers (ADR-0028), running `EXPLAIN QUERY PLAN` over the validated SQL text through the existing ADR-0028 span-styled plan tree (advanced mode only; DSL `explain` unchanged in both modes). Reframes OOS-2 as a *deferred* scope exclusion (per ADR-0000's new out-of-scope discipline), not a principled rejection — surfaced 2026-05-27 while characterising advanced-mode explain (suspected a bug; it was OOS-2 as written). Self-contained, orthogonal to the echo (the echo renders SQL *from* DSL; this explains SQL the user *wrote*). OOS (deferred): EXPLAIN of DDL (no query plan exists). Built test-first when picked up +- [ADR-0039 — EXPLAIN over advanced-mode SQL queries](0039-explain-over-advanced-sql.md) — **Accepted** (2026-05-27), **implemented 2026-05-30 (issue #7)**, **supersedes ADR-0030 §13 OOS-2**. Lets `explain` wrap the advanced SQL commands (`Select`/`SqlInsert`/`SqlUpdate`/`SqlDelete`, plus `with`/CTE which builds a `Select`) in addition to the DSL `ShowData`/`Update`/`Delete` it already covers (ADR-0028), running `EXPLAIN QUERY PLAN` over the validated SQL text through the existing ADR-0028 span-styled plan tree (advanced mode only; DSL `explain` unchanged in both modes). Implemented via a second `Advanced` `explain` CommandNode (`EXPLAIN_SQL`) registered under the shared `explain` entry word — reusing the established `insert`/`update`/`delete` shared-word dispatch (`decide`: SQL-first / DSL-fallback), so `explain show data …` and DSL-only `--all-rows` still reach the DSL node; rejected a `DynamicSubgrammar` mode-gate (its resolution cache key omits `mode`). `build_explain_sql` slices the inner SQL off the source (excludes `explain`) and reuses the existing SQL builders; `do_explain_plan` runs the carried text verbatim, no params. Advanced `explain update`/`delete` now route through SQL (identical plan, full SQL syntax); DSL-explain tests pinned to simple mode. Reframed OOS-2 as a *deferred* exclusion (per ADR-0000's out-of-scope discipline), not a rejection. OOS (deferred): EXPLAIN of DDL (no query plan exists) diff --git a/docs/requirements.md b/docs/requirements.md index 4d60d77..4da2876 100644 --- a/docs/requirements.md +++ b/docs/requirements.md @@ -509,7 +509,11 @@ handoff-14 cleanup; 449 after B2/C2.) *(Implemented per ADR-0028: the `explain` prefix over `show data` / `update` / `delete`, with a span-styled plan tree. `EXPLAIN QUERY PLAN` never executes, so explaining a - destructive `update` / `delete` is safe.)* + destructive `update` / `delete` is safe. Extended 2026-05-30, + issue #7 / ADR-0039: `explain` now also wraps advanced-mode + SQL — `select` / `with` / `insert` / `update` / `delete` — via + a second `Advanced` `explain` CommandNode on the shared entry + word, reusing the same plan-tree renderer.)* - [x] **QA2** Plan rendering specifics — tree layout, annotation taxonomy, colour scheme. Implemented per ADR-0028 (§3–§6): a box-drawing tree, the substring-pattern taxonomy, and the diff --git a/src/db.rs b/src/db.rs index ed29d2a..9b09c94 100644 --- a/src/db.rs +++ b/src/db.rs @@ -9211,6 +9211,16 @@ fn do_explain_plan(conn: &Connection, query: &Command) -> Result (sql.clone(), Vec::new()), other => { // The grammar only ever wraps the three explainable // commands; a different inner command means a @@ -12436,6 +12446,13 @@ mod tests { .expect("inner command parse") } + /// Advanced-mode counterpart of `parse_inner` (ADR-0039): parses + /// a bare SQL command (the inner of an `explain`). + fn parse_inner_adv(sql: &str) -> Command { + crate::dsl::parser::parse_command_in_mode(sql, crate::mode::Mode::Advanced) + .expect("inner SQL command parse") + } + #[tokio::test] async fn explain_show_data_returns_a_scan_plan() { let db = db(); @@ -12587,6 +12604,155 @@ mod tests { assert!(result.is_err(), "explaining a missing table should fail"); } + // --- ADR-0039: explain over advanced-mode SQL ------------- + + #[tokio::test] + async fn explain_sql_select_returns_a_scan_plan() { + let db = db(); + people_table(&db).await; + let plan = db + .explain_query_plan(parse_inner_adv("select * from People where Name = 'Bob'")) + .await + .unwrap(); + assert!(!plan.rows.is_empty(), "a plan has at least one node"); + assert!( + plan.rows.iter().any(|r| r.detail.contains("SCAN")), + "expected a SCAN node, got {:?}", + plan.rows, + ); + } + + #[tokio::test] + async fn explain_sql_select_uses_an_index_when_one_exists() { + let db = db(); + people_table(&db).await; + db.add_index(None, "People".to_string(), vec!["Age".to_string()], None) + .await + .unwrap(); + let plan = db + .explain_query_plan(parse_inner_adv("select * from People where Age = 35")) + .await + .unwrap(); + assert!( + plan.rows.iter().any(|r| r.detail.contains("USING INDEX")), + "expected an index search, got {:?}", + plan.rows, + ); + } + + #[tokio::test] + async fn explain_sql_delete_does_not_remove_any_rows() { + let db = db(); + people_table(&db).await; + db.explain_query_plan(parse_inner_adv("delete from People where Age = 35")) + .await + .unwrap(); + let data = db + .query_data("People".to_string(), None, None, None) + .await + .unwrap(); + assert_eq!(data.rows.len(), 4, "explain delete must not delete"); + } + + #[tokio::test] + async fn explain_sql_update_does_not_change_any_data() { + let db = db(); + people_table(&db).await; + db.explain_query_plan(parse_inner_adv( + "update People set Name = 'Zed' where Age = 35", + )) + .await + .unwrap(); + let data = db + .query_data("People".to_string(), None, None, None) + .await + .unwrap(); + assert!( + !data.rows.iter().flatten().any(|c| c.as_deref() == Some("Zed")), + "explain update must not modify rows", + ); + } + + #[tokio::test] + async fn explain_sql_with_cte_returns_a_plan() { + let db = db(); + people_table(&db).await; + let plan = db + .explain_query_plan(parse_inner_adv( + "with adults as (select * from People where Age = 35) select * from adults", + )) + .await + .unwrap(); + assert!(!plan.rows.is_empty(), "a WITH query has a plan"); + } + + #[tokio::test] + async fn explain_sql_insert_does_not_add_a_row() { + // EXPLAIN QUERY PLAN over an INSERT must not execute it + // (ADR-0039). A VALUES insert has a trivial plan (it may be + // empty), so we assert the call succeeds and the table is + // unchanged rather than asserting a node count. + let db = db(); + people_table(&db).await; + db.explain_query_plan(parse_inner_adv( + "insert into People (Name, Age, Active) values ('Zed', 1, true)", + )) + .await + .unwrap(); + let data = db + .query_data("People".to_string(), None, None, None) + .await + .unwrap(); + assert_eq!(data.rows.len(), 4, "explain insert must not insert"); + } + + #[tokio::test] + async fn explain_sql_insert_from_select_returns_a_plan() { + // INSERT … SELECT has a real query plan (the SELECT source), + // so this exercises the non-trivial insert-plan path. + let db = db(); + people_table(&db).await; + let plan = db + .explain_query_plan(parse_inner_adv( + "insert into People (Name, Age, Active) select Name, Age, Active from People where Age = 35", + )) + .await + .unwrap(); + assert!( + plan.rows.iter().any(|r| r.detail.contains("SCAN")), + "insert-from-select plans the SELECT source, got {:?}", + plan.rows, + ); + let data = db + .query_data("People".to_string(), None, None, None) + .await + .unwrap(); + assert_eq!(data.rows.len(), 4, "explain insert-select must not insert"); + } + + #[tokio::test] + async fn explain_sql_display_sql_is_the_verbatim_query() { + // The SQL path carries the user's text grammar-as-text, so + // the display SQL is verbatim (no canonicalisation), unlike + // the DSL path which synthesises canonical SQL. + let db = db(); + people_table(&db).await; + let plan = db + .explain_query_plan(parse_inner_adv("select * from People where Age = 35")) + .await + .unwrap(); + assert_eq!(plan.display_sql, "select * from People where Age = 35"); + } + + #[tokio::test] + async fn explain_sql_of_a_missing_table_is_an_error() { + let db = db(); + let result = db + .explain_query_plan(parse_inner_adv("select * from NoSuchTable")) + .await; + assert!(result.is_err(), "explaining a missing SQL table should fail"); + } + // --- column constraints at create-table (ADR-0029) ------ /// A `ColumnSpec` carrying the four constraint slots. diff --git a/src/dsl/grammar/data.rs b/src/dsl/grammar/data.rs index a9c08c2..c33e0ca 100644 --- a/src/dsl/grammar/data.rs +++ b/src/dsl/grammar/data.rs @@ -431,6 +431,46 @@ const EXPLAIN_CHOICES: &[Node] = &[ ]; const EXPLAIN_SHAPE: Node = Node::Choice(EXPLAIN_CHOICES); +// --- explain over advanced-mode SQL (ADR-0039) ------------------- +// +// The SQL inner mirrors the DSL inner above, but wraps the SQL +// command shapes (the same nodes the standalone `SELECT` / `WITH` / +// `SQL_*` commands use). This shape backs a *second* `explain` +// CommandNode (`EXPLAIN_SQL`, registered `Advanced`); the registry's +// shared-entry-word dispatch tries it first in advanced mode and +// falls back to the `Simple` DSL `EXPLAIN` when a branch can't match +// (e.g. `explain show data …`, or a DSL-only `--all-rows`). `select` +// and `with` are SQL-only, so they only ever resolve here. + +const EXPLAIN_SELECT_NODES: &[Node] = &[ + Node::Word(Word::keyword("select")), + Node::Subgrammar(&sql_select::SQL_SELECT_TAIL), +]; +const EXPLAIN_WITH_NODES: &[Node] = &[ + Node::Word(Word::keyword("with")), + Node::Subgrammar(&sql_select::SQL_WITH_TAIL), +]; +const EXPLAIN_SQL_INSERT_NODES: &[Node] = &[ + Node::Word(Word::keyword("insert")), + Node::Subgrammar(&sql_insert::SQL_INSERT_SHAPE), +]; +const EXPLAIN_SQL_UPDATE_NODES: &[Node] = &[ + Node::Word(Word::keyword("update")), + Node::Subgrammar(&sql_update::SQL_UPDATE_SHAPE), +]; +const EXPLAIN_SQL_DELETE_NODES: &[Node] = &[ + Node::Word(Word::keyword("delete")), + Node::Subgrammar(&sql_delete::SQL_DELETE_SHAPE), +]; +const EXPLAIN_SQL_CHOICES: &[Node] = &[ + Node::Seq(EXPLAIN_SELECT_NODES), + Node::Seq(EXPLAIN_WITH_NODES), + Node::Seq(EXPLAIN_SQL_INSERT_NODES), + Node::Seq(EXPLAIN_SQL_UPDATE_NODES), + Node::Seq(EXPLAIN_SQL_DELETE_NODES), +]; +const EXPLAIN_SQL_SHAPE: Node = Node::Choice(EXPLAIN_SQL_CHOICES); + // ================================================================= // select — SQL `SELECT` (advanced mode; ADR-0030 §6, ADR-0031) // ================================================================= @@ -876,6 +916,48 @@ fn build_explain(path: &MatchedPath, _source: &str) -> Result Result { + // Words in the path: [0] is the `explain` entry word, [1] is the + // inner entry keyword (select / with / insert / update / delete). + let inner_item = path + .items + .iter() + .filter(|i| matches!(i.kind, MatchedKind::Word(_))) + .nth(1) + .ok_or_else(|| ValidationError { + message_key: "parse.error_wrapper", + args: vec![("detail", "missing explain target".to_string())], + })?; + let inner_word = match &inner_item.kind { + MatchedKind::Word(w) => *w, + _ => unreachable!("filtered to Word above"), + }; + let inner_source = source[inner_item.span.0..].trim(); + let inner = match inner_word { + "select" | "with" => build_select(path, inner_source)?, + "insert" => build_sql_insert(path, inner_source)?, + "update" => build_sql_update(path, inner_source)?, + "delete" => build_sql_delete(path, inner_source)?, + _ => { + return Err(ValidationError { + message_key: "parse.error_wrapper", + args: vec![("detail", "unknown explain target".to_string())], + }); + } + }; + Ok(Command::Explain { + query: Box::new(inner), + }) +} + // ================================================================= // replay — `replay ` | `replay ''` // ================================================================= @@ -1317,6 +1399,25 @@ pub static EXPLAIN: CommandNode = CommandNode { help_id: Some("data.explain"), usage_ids: &["parse.usage.explain"],}; +/// `explain` over advanced-mode SQL (ADR-0039). +/// +/// The `Advanced` node of the shared `explain` entry word. Pairs with +/// the `Simple` DSL [`EXPLAIN`] node above: in advanced mode the +/// dispatcher tries this SQL node first and falls back to the DSL node +/// when no SQL branch matches (`explain show data …`, or a DSL-only +/// `--all-rows`); in simple mode only the DSL node is reachable. +pub static EXPLAIN_SQL: CommandNode = CommandNode { + entry: Word::keyword("explain"), + shape: EXPLAIN_SQL_SHAPE, + ast_builder: build_explain_sql, + // No `help_id` / `usage_ids` — this is the `Advanced` half of the + // shared `explain` entry word, so it defers to the `Simple` + // `EXPLAIN` node's help/usage (which now covers the SQL forms + // too). Mirrors the `SQL_INSERT`/`SQL_UPDATE`/`SQL_DELETE` + // precedent; otherwise `note_help` would print `explain` twice. + help_id: None, + usage_ids: &[],}; + /// SQL `SELECT` (ADR-0030 §6, ADR-0031, ADR-0032). /// /// Advanced mode only — gated by `grammar::is_advanced_only`. @@ -1401,10 +1502,17 @@ mod explain_tests { use super::Command; use crate::dsl::parser::parse_command; - /// Parse `input` and unwrap the `Command::Explain` wrapper, - /// returning the inner command. + /// Parse `input` in **simple** mode and unwrap the + /// `Command::Explain` wrapper, returning the inner command. + /// These cover the DSL-explain wrapping (ADR-0028); the + /// advanced-mode SQL wrapping (ADR-0039) is covered by + /// `explain_inner_adv` below. (`parse_command` defaults to + /// advanced, where `explain update`/`delete` now route to the + /// SQL path — so DSL-explain tests pin the mode explicitly.) fn explain_inner(input: &str) -> Command { - match parse_command(input).expect("explain should parse") { + match crate::dsl::parser::parse_command_in_mode(input, crate::mode::Mode::Simple) + .expect("explain should parse") + { Command::Explain { query } => *query, other => panic!("expected Command::Explain, got {other:?}"), } @@ -1450,8 +1558,15 @@ mod explain_tests { fn explain_of_an_incomplete_update_is_a_parse_error() { // A bare `update` still needs its `where` / `--all-rows` // (ADR-0028 §1: `explain` of an incomplete command is the - // same parse error the command alone would be). - assert!(parse_command("explain update Customers set Name='Bo'").is_err()); + // same parse error the command alone would be). Simple mode: + // in advanced mode a where-less SQL UPDATE is valid (ADR-0039). + assert!( + crate::dsl::parser::parse_command_in_mode( + "explain update Customers set Name='Bo'", + crate::mode::Mode::Simple, + ) + .is_err() + ); } #[test] @@ -1465,4 +1580,138 @@ mod explain_tests { assert!(parse_command("explain").is_err()); assert!(parse_command("explain show").is_err()); } + + // ---- ADR-0039: explain over advanced-mode SQL -------------- + + use crate::dsl::parser::parse_command_in_mode; + use crate::mode::Mode; + + /// Advanced-mode counterpart of `explain_inner`. + fn explain_inner_adv(input: &str) -> Command { + match parse_command_in_mode(input, Mode::Advanced) + .expect("advanced explain should parse") + { + Command::Explain { query } => *query, + other => panic!("expected Command::Explain, got {other:?}"), + } + } + + #[test] + fn explain_select_wraps_a_select_with_clean_sql() { + // The carried SQL must NOT include the `explain` prefix + // (ADR-0039) — `EXPLAIN QUERY PLAN` runs over the inner SQL. + match explain_inner_adv("explain select * from Customers") { + Command::Select { sql } => assert_eq!(sql, "select * from Customers"), + other => panic!("expected Select, got {other:?}"), + } + } + + #[test] + fn explain_with_cte_wraps_a_select() { + match explain_inner_adv( + "explain with recent as (select * from Orders) select * from recent", + ) { + Command::Select { sql } => { + assert!(sql.starts_with("with recent"), "clean inner sql: {sql}"); + } + other => panic!("expected Select, got {other:?}"), + } + } + + #[test] + fn explain_sql_insert_wraps_a_sql_insert() { + match explain_inner_adv("explain insert into Customers values (1, 'Bo')") { + Command::SqlInsert { sql, target_table, .. } => { + assert_eq!(target_table, "Customers"); + assert_eq!(sql, "insert into Customers values (1, 'Bo')"); + } + other => panic!("expected SqlInsert, got {other:?}"), + } + } + + #[test] + fn explain_sql_update_wraps_a_sql_update_with_clean_sql() { + match explain_inner_adv("explain update Customers set Name = 'Bo' where id = 1") { + Command::SqlUpdate { sql, target_table, .. } => { + assert_eq!(target_table, "Customers"); + assert_eq!(sql, "update Customers set Name = 'Bo' where id = 1"); + } + other => panic!("expected SqlUpdate, got {other:?}"), + } + } + + #[test] + fn explain_sql_delete_wraps_a_sql_delete() { + match explain_inner_adv("explain delete from Customers where id = 1") { + Command::SqlDelete { sql, target_table, .. } => { + assert_eq!(target_table, "Customers"); + assert_eq!(sql, "delete from Customers where id = 1"); + } + other => panic!("expected SqlDelete, got {other:?}"), + } + } + + #[test] + fn explain_update_with_all_rows_flag_falls_back_to_dsl_in_advanced() { + // `--all-rows` is DSL-only; the SQL update shape can't + // consume it, so the explain inner falls back to the DSL + // `Update` node — mirroring the top-level shared-word + // dispatch (ADR-0033). + assert!(matches!( + explain_inner_adv("explain update Customers set Name = 'Bo' --all-rows"), + Command::Update { .. } + )); + } + + #[test] + fn explain_show_data_still_uses_dsl_in_advanced() { + // `show data` has no SQL form; advanced `explain show data` + // falls back to the DSL inner. + assert!(matches!( + explain_inner_adv("explain show data Customers"), + Command::ShowData { .. } + )); + } + + #[test] + fn explain_select_is_rejected_in_simple_mode() { + // `select` is advanced-only, so `explain select` has no + // simple-mode form. + assert!(parse_command_in_mode("explain select * from Customers", Mode::Simple).is_err()); + } + + #[test] + fn explain_does_not_cover_ddl() { + // EXPLAIN QUERY PLAN applies to DML/queries only (ADR-0039 + // out of scope); there is no SQL DDL branch under explain. + assert!(parse_command_in_mode( + "explain create table T (id int)", + Mode::Advanced, + ) + .is_err()); + } + + #[test] + fn advanced_explain_completion_offers_the_sql_verbs() { + // After `explain ` in advanced mode the candidate list is the + // union across both `explain` CommandNodes: the SQL verbs + // (select/with/insert/update/delete) plus the DSL `show` + // (ADR-0039). The shared-entry-word completion already + // aggregates, so there is no UX gap. + use crate::completion::candidates_at_cursor_in_mode; + let schema = crate::completion::SchemaCache::default(); + let input = "explain "; + let completion = + candidates_at_cursor_in_mode(input, input.len(), &schema, Mode::Advanced) + .expect("explain offers candidates"); + let names: Vec<&str> = completion + .candidates + .iter() + .map(|c| c.text.as_str()) + .collect(); + for verb in ["select", "with", "insert", "update", "delete", "show"] { + assert!(names.contains(&verb), "expected `{verb}` in {names:?}"); + } + } } + diff --git a/src/dsl/grammar/mod.rs b/src/dsl/grammar/mod.rs index a886076..fccf472 100644 --- a/src/dsl/grammar/mod.rs +++ b/src/dsl/grammar/mod.rs @@ -634,6 +634,13 @@ pub static REGISTRY: &[(&CommandNode, CommandCategory)] = &[ (&data::SQL_INSERT, CommandCategory::Advanced), (&data::SQL_UPDATE, CommandCategory::Advanced), (&data::SQL_DELETE, CommandCategory::Advanced), + // Shared entry word `explain` (ADR-0039): the `Simple` DSL + // `data::EXPLAIN` (above) wraps `show data` / `update` / `delete`; + // this `Advanced` node wraps the SQL `select` / `with` / `insert` + // / `update` / `delete`. SQL-first / DSL-fallback in advanced mode + // (so `explain show data …` and DSL-only `--all-rows` still reach + // the DSL node); DSL-only in simple mode. + (&data::EXPLAIN_SQL, CommandCategory::Advanced), // Shared entry word `create` (ADR-0035 §2): the simple // `ddl::CREATE` (above) and these advanced SQL nodes. The // dispatcher tries the advanced candidates first in advanced mode @@ -782,4 +789,22 @@ mod usage_key_tests { Some("parse.usage.create_table"), ); } + + #[test] + fn no_two_registered_commands_share_a_help_id() { + // `note_help` emits one help block per `help_id: Some(_)` + // with no dedup, so a duplicate help_id prints the same + // command twice in `help`. Shared-entry-word `Advanced` + // nodes (SQL_INSERT, …, EXPLAIN_SQL) therefore carry + // `help_id: None` and defer to their `Simple` sibling. + let mut seen = std::collections::HashSet::new(); + for (command, _category) in super::REGISTRY { + if let Some(id) = command.help_id { + assert!( + seen.insert(id), + "duplicate help_id `{id}` in REGISTRY would print twice in `help`", + ); + } + } + } } diff --git a/src/friendly/strings/en-US.yaml b/src/friendly/strings/en-US.yaml index 1e5e31f..3e4b37d 100644 --- a/src/friendly/strings/en-US.yaml +++ b/src/friendly/strings/en-US.yaml @@ -321,6 +321,8 @@ help: explain show data | explain update ... | explain delete from ... — show how the database would run a query, without running it (safe even for update / delete) + explain (advanced mode) + — the same plan for the SQL you wrote # Type reference, appended after the command list. types_reference: | Types: text, int, real, decimal, bool, date, datetime, blob, serial, shortid @@ -534,6 +536,7 @@ parse: explain show data [where ] [limit ] explain update
set =[, ...] (where | --all-rows) explain delete from
(where | --all-rows) + explain (advanced mode) replay: "replay | replay ''" # SQL `SELECT` (advanced mode; ADR-0030 / ADR-0031). select: "select (* | [ as ][, ...]) from
[where ] [order by [ asc|desc][, ...]] [limit ]"