diff --git a/docs/adr/0033-sql-dml-grammar.md b/docs/adr/0033-sql-dml-grammar.md index 069d5e0..7a39f2c 100644 --- a/docs/adr/0033-sql-dml-grammar.md +++ b/docs/adr/0033-sql-dml-grammar.md @@ -1270,6 +1270,192 @@ parity is preserved. The user confirmed this deferral. - The handoff-31 §4 "WHERE-byte-extraction is tractable for DELETE" heads-up is moot — no extraction happens. +## Amendment 3 — Command identity is intrinsic; execution-mode is side-channel (deferred) (2026-05-23) + +This amendment **clarifies the command-identity model** that §2 and +Amendment 1 implement, **corrects a false premise in the +implementation plan's sub-phase 3j exit gate**, and **records a +deferred follow-up** (the execution-time mode side-channel). It was +written during sub-phase 3j — wiring the shared `insert` / `update` / +`delete` entry words — after the dispatch model's interaction with the +existing test suite surfaced a question about what a "command" *is* and +how input mode relates to it. Recorded with explicit user approval +before any 3j code landed. + +### The model — a command is a mode-rooted grammar-path outcome + +A command is the typed outcome of a grammar path **rooted at the input +mode**: `simple → update → …` or `advanced → update → …`. Its identity +is **intrinsic** — determined by *which grammar matched*, not by a mode +flag carried on one shared command type. In Advanced mode the +dispatcher (Amendment 1) tries the Advanced (SQL) candidate(s) first and +**falls back to the Simple (DSL) candidate** when no Advanced branch +recognizes a token; the fallback produces the **Simple** command. + +Worked example (the `--all-rows` fall-through): `delete from T +--all-rows` in Advanced mode — no Advanced (SQL) branch recognizes +`--all-rows` (the SQL `DELETE` has no expression slot after the table +to absorb it), so the path falls back to `simple → delete → … → +all-rows` and yields `Command::Delete { filter: AllRows }` (the DSL +command), exactly as it would in Simple mode. The mode rooted the path; +the command that came out is the Simple one. + +*Counter-example (no fall-through):* `update T set x = 42 --all-rows` +in Advanced mode does **not** fall back — the SQL `UPDATE`'s +`SET ` greedily consumes `--all-rows` as the expression +`42 - -all - rows` (with `all`/`rows` as column refs), so the SQL shape +matches and `Command::SqlUpdate` is produced. Whether `--all-rows` is a +DSL flag or part of a SQL expression is decided by which grammar wins, +not by special-casing the token. (At execution this is harmless: the +engine treats `--all-rows` as a SQL line comment, so the statement runs +as `update T set x = 42` — all rows — the same effect as the DSL flag.) + +### Simple mode commits the DSL candidate; an advanced-mode pointer combines + +In **Simple mode** a shared entry word **always commits its DSL +candidate**, so the user sees DSL completion and the *real* DSL error +(with its position) — not a bare "this is SQL" that discards the +actionable detail. A plain `ValidationFailed`/`Mismatch` "this is SQL" +dispatch outcome is reserved for entry words that have **no** DSL form +(`select` / `with`); for those the DSL surface has nothing to offer, so +the simple-mode gate points at advanced mode (ADR-0030 §2). + +To keep the SQL-discoverability the original §2 envisioned *without* +losing the DSL fix, the rendering layer **combines** them: when a +simple-mode line is a *definite* DSL error (not merely incomplete) and +the same line would parse in advanced mode, the DSL error prose is +suffixed with the `advanced_mode.also_valid_sql` pointer — e.g. +`for \`Name\`: Type a quoted string … (valid as SQL in advanced mode — +\`mode advanced\` or prefix \`:\`)`. The DSL detail and the mode hint +coexist (`input_render::ambient_hint_in_mode` / +`advanced_alternative_note`). Mid-typing (incomplete) input is not +suffixed, to avoid noise during normal DSL entry. This supersedes the +draft of this amendment's earlier suggestion that a SQL-shaped line in +simple mode emits a *bare* "this is SQL" hint for shared words. + +### Overlapping inputs produce two distinct commands — both are tested + +For a **fully-overlapping** shape — `insert into T values (…)` is valid +in both grammars — Simple mode yields `Command::Insert` (a typed AST, +DSL execution) and Advanced mode yields `Command::SqlInsert` (validated +text, SQL execution). This is **correct, not a defect**: the two +commands *do the same thing but execute differently* (ADR-0030 §4 — the +DSL lowers to a typed AST; SQL is grammar-as-text run verbatim). Because +they are distinct commands, **each is tested in the mode that produces +it** — DSL grammar tests run in Simple mode; the SQL command variants +are tested in Advanced mode. + +### Correction to the plan's 3j exit gate + +The implementation plan's sub-phase 3j exit gate says "all existing DSL +`INSERT`/`UPDATE`/`DELETE` tests still green — unmodified inputs, +unmodified outputs", and its scope-out says "observable behaviour must +be identical before and after 3j". That rested on a **false premise**: +that the existing DSL DML tests run in Simple mode. They do not — the +common helpers (`ok`/`err`/`parse`) call `parse_command`, which +**defaults to Advanced mode**. Today that is harmless because +`insert`/`update`/`delete` have no SQL competitor, so they route to the +DSL command even in Advanced mode. Once they become **shared** entry +words, §2's Advanced-mode **SQL-first** rule routes the overlap to the +SQL command (`Command::Sql*`), changing the parsed variant those tests +assert. + +The corrected invariant: **Simple-mode behaviour is unchanged; Advanced +mode is SQL-first per §2; the DSL grammar is tested in Simple mode (its +canonical surface, ADR-0003); both command variants are tested in their +producing mode.** No *production* behaviour regresses — the §6 (shortid +auto-fill) and §7 (cascade summary) parity promises keep the two paths +observably equivalent for overlapping inputs; only the +implementation-internal `Command` variant differs, and only in Advanced +mode. + +### Replay uses the same parser, in advanced mode — and needs no per-line mode + +A consequence worth stating explicitly, because it is easy to +over-think (and a prior reading of this work did). **Replay parses +each recorded line with the same schema-aware parser the interactive +path uses, in advanced mode** (the full surface) — it skips nothing +and simplifies nothing. The only difference from an interactive line +is the mode argument: interactive uses the user's current mode, +replay fixes it to advanced. + +This is correct and complete **without** the per-line execution-mode +side-channel below, because of the §6 (shortid auto-fill) and §7 +(cascade-summary) parity guarantees: a valid overlapping command +produces *identical effects* whether it lowers to the DSL variant or +the SQL variant. So replaying a log of valid commands in advanced +mode is identical to typing those lines interactively in advanced +mode — and, by parity, identical in effect to the simple-mode entry +that originally produced them. (Replay only ever sees lines that +already executed successfully: `history.log` is success-only, and +ADR-0034's deferred journal replays `ok` lines only.) + +The one observable nuance is purely in **error reporting for an +invalid line** — which only arises from a hand-built script, never +from a real journal. Such a line is still rejected and not applied; +*where* the rejection lands just follows the grammar, exactly as +interactively: an `insert … values …` line is SQL in advanced mode, +so a wrong column-type value is rejected by the engine at execute +time rather than by a DSL typed slot at parse time. This is **not** +replay using a lesser parser — it is the same advanced-mode parse a +user would get typing the line. Replay therefore does **not** depend +on the deferred side-channel below. + +### Execution-time mode is side-channel — deferred to its own ADR + +Independent of command *identity*, every command should — at **execution +time** — know which of three modes it ran under: `simple`, `advanced`, +or `advanced-one-shot` (the `:` escape from Simple mode, ADR-0003), so +execution can adjust **output** without changing **identity** (e.g. a +Simple-mode `create table` echoing the generated SQL when run in +Advanced mode, while staying silent in Simple mode). + +Today this exists only as a **rendering** side-channel +(`OutputLine.mode_at_submission`, consumed by the echo-line mode tag in +`ui.rs`). The `Mode` enum is two-way (`Simple` / `Advanced`); the +one-shot distinction is a transient "effective mode" collapsed at +submission; and neither `Action::ExecuteDsl` nor the database worker +carries any mode. Wiring an **execution-time** mode side-channel — +widening `Mode` to the three-way distinction and threading it through +the `Action` → worker interface — is **out of scope for Phase 3** and is +**deferred to its own ADR** (user-confirmed). It is *not* required for +Phase 3's dispatch to be correct: the routing and the `--all-rows` +fall-through above are complete on the two-way mode. This amendment +forward-references that future ADR so the requirement is not lost. + +### Consequences of the amendment + +- **No code or grammar change** from this amendment itself — it records + the model that the dispatch (Amendment 1) already implements and + corrects the plan's test-mode premise. +- The 3j test migration **moves the DSL grammar / completion tests to + Simple mode** (inputs and `Command::Insert`/`Update`/`Delete` + assertions unchanged), **keeps the `--all-rows` fall-through tests in + Advanced mode** (they validate the fallback to the Simple command), + relies on the migrated `tests/sql_*.rs` for the SQL command variants + in Advanced mode, and adds dispatcher routing tests + (Advanced + structurally-ambiguous → SQL; Advanced + `--all-rows` → + DSL fallback; Simple + a SQL-only *entry word* `select` → "this is + SQL"; Simple + a shared word with a SQL-only *construct* → DSL error, + carrying the combined pointer at the rendering layer). +- The **combined pointer** lives in `input_render::ambient_hint_in_mode` + (live typing) and `App::dispatch_dsl` (submit), both via the shared + `advanced_alternative_note`, so a Simple-mode definite DSL error that + would run as SQL gains the `advanced_mode.also_valid_sql` suffix in + both surfaces (the submit path covers SQL constructs that surface + only on submit, e.g. `delete … returning`). Found via the `/runda` + round. +- **Internal-table rejection symmetrised** (`/runda` finding B, + ADR-0030 §6): the DSL data-command target slots (`insert` / `update` / + `delete` / `show data` / `show table`) gained + `reject_internal_table`, so `__rdbms_*` tables are refused in Simple + mode too — previously only the advanced SQL grammar rejected them, so + a simple-mode DML could touch the internal metadata tables. +- The **execution-time mode side-channel + three-way `Mode`** is tracked + as a future ADR; `OutputLine.mode_at_submission` remains the only mode + side-channel until then. No structure built in 3j assumes mode is + irrelevant to execution. + ## See also - ADR-0005 — the ten-type vocabulary INSERT works with. diff --git a/docs/adr/README.md b/docs/adr/README.md index 551f1b2..3c801fa 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -38,5 +38,5 @@ 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); 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) +- [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); **Amendment 3 (sub-phase 3j) records the command-identity model and defers the execution-mode side-channel**: a command is the typed outcome of a *mode-rooted* grammar path and its identity is intrinsic (Advanced mode tries SQL first, falls back to the *Simple* DSL command when no SQL branch matches a token, e.g. `delete … --all-rows`; note `update … --all-rows` does *not* fall back — the SQL `SET` expression eats `--all-rows`, harmless since the engine treats it as a comment); **Simple mode commits the DSL candidate for shared words** so the *real* DSL error surfaces, and when that line would also run in advanced mode the rendering layer **combines** them — DSL error **plus** an `advanced_mode.also_valid_sql` pointer ("… (valid as SQL in advanced mode)") — keeping the actionable DSL fix while pointing at advanced mode; bare "this is SQL" is reserved for entry words with no DSL form (`select`/`with`); a fully-overlapping input (`insert … values …`) legitimately yields *two distinct commands* (`Command::Insert` typed-AST vs `Command::SqlInsert` validated-text) that do the same thing but execute differently (ADR-0030 §4), so each is tested in the mode that produces it; **corrects the plan's 3j exit-gate premise** that the DSL DML tests run in Simple mode (they call `parse_command`, which defaults to Advanced) — the real invariant is "Simple-mode behaviour unchanged, Advanced mode SQL-first, DSL grammar tested in Simple mode, both variants tested in their producing mode", with §6/§7 parity keeping the paths observably equivalent; and **defers to its own future ADR** the execution-time mode side-channel (three-way `Mode`: simple/advanced/advanced-one-shot threaded through `Action`→worker, for mode-dependent *output* like echoing generated SQL) — today only the *rendering* side-channel `OutputLine.mode_at_submission` exists, and the three-way distinction is not required for Phase 3 dispatch correctness - [ADR-0034 — `history.log` as a complete command journal; replay reads success-only](0034-history-journal-and-replay-filter.md) — **Accepted**, resolves a three-way tension in `history.log`'s roles found while implementing ADR-0033 3f: (1) the persistent log is success-only while the in-memory Up/Down recall ring records *every* submission (success or failure, "so users can recall and edit typo'd commands"), and the ring is re-seeded from the log on project open — so **failed commands are recallable within a session but silently lost across sessions**; (2) replay wants the state-building (successful) commands while recall wants everything typed, which one success-only file cannot serve; (3) `replay history.log` never actually worked — `run_replay` parses each whole line through the DSL parser with no understanding of the `||` record shape, so a real log fails on line 1, and **no test ever fed the pipe format to replay** (the `replay_history_log_records_subcommands_only` test only checks what replay *writes*, never replays the log as input). Decision: `history.log` becomes a **complete journal** — every submission recorded, tagged `ok`/`err` via the status field the format already reserved (ADR-0015 §5) — and **each consumer filters**: hydration reads all records (cross-session recall matches in-session), replay reads `ok` only (and learns the journal format, while still accepting bare-command `.commands` scripts; detection by the leading timestamp+status prefix so a `|` inside a bare command isn't misread). Successful commands stay journalled transactionally by the worker; failed commands are journalled `err` best-effort from the runtime/app error path (a parse failure never reaches the worker). Amends ADR-0006's "successfully executed" wording and ADR-0015 §5 ("status always `ok`") / §12 (hydration). Code deferred to two tracked test-first sub-tasks (journal-failures+filtering; replay-parses-journal-format); existing all-`ok` logs need no migration diff --git a/docs/requirements.md b/docs/requirements.md index 36022f0..5ab2161 100644 --- a/docs/requirements.md +++ b/docs/requirements.md @@ -120,6 +120,19 @@ handoff-14 cleanup; 449 after B2/C2.) one-shot advanced escape (with the prompt label updated). The `mode simple` / `mode advanced` command switches modes persistently. +- [~] **M4** Execution-time mode side-channel — deferred, awaiting + its own ADR (ADR-0033 Amendment 3). Every command should know, at + execution time, which of three modes it ran under — `simple`, + `advanced`, or `advanced-one-shot` (the `:` escape) — so execution + can adjust *output* without changing command *identity* (e.g. a + simple-mode `create table` echoing the generated SQL when run in + advanced mode, silent in simple). Today only the *rendering* + side-channel exists (`OutputLine.mode_at_submission`); the `Mode` + enum is two-way, the one-shot distinction is collapsed at + submission, and neither `Action::ExecuteDsl` nor the worker carries + any mode. The work widens `Mode` to three-way and threads it + through the `Action` → worker interface. Not required for Phase 3 + dispatch correctness; tracked here so it is not lost. ## App-level commands (per ADR-0003) diff --git a/src/app.rs b/src/app.rs index 7c91f1f..8829ff0 100644 --- a/src/app.rs +++ b/src/app.rs @@ -1187,6 +1187,20 @@ impl App { "parse.error", detail = parse_error_message(&err) )); + // ADR-0033 Amendment 3: combine the DSL error with a + // pointer to advanced mode when the same line would + // run as SQL there. Only in simple mode (a one-shot + // `:` or persistent advanced submission uses the SQL + // surface already). This mirrors the live hint and + // covers SQL constructs that surface only on submit + // (e.g. `delete … returning`, where the live hint + // shows WHERE-completion rather than an error). + if submission_mode == Mode::Simple + && let Some(note) = + crate::input_render::advanced_alternative_note(input, &self.schema_cache) + { + self.note_error(note); + } // ADR-0021 §2: append the usage block (if a // known command-entry keyword was consumed) or // the available-commands fallback (§5). @@ -2326,6 +2340,44 @@ mod tests { ); } + #[test] + fn simple_mode_submit_of_sql_construct_appends_advanced_pointer() { + // ADR-0033 Amendment 3: submitting a line in simple mode that + // fails as DSL but would run as SQL in advanced mode appends + // the `advanced_mode.also_valid_sql` pointer to the parse + // error — keeping the DSL detail and pointing at advanced + // mode. Multi-row VALUES is a definite DSL error and valid SQL + // (no schema needed). + let mut app = App::new(); + type_str(&mut app, "insert into T values (1, 2), (3, 4)"); + let actions = submit(&mut app); + assert!(actions.is_empty(), "the bad line must not dispatch"); + let has_pointer = app + .output + .iter() + .any(|l| l.kind == OutputKind::Error && l.text.contains("advanced mode")); + assert!( + has_pointer, + "expected the advanced-mode pointer on submit; output:\n{}", + error_lines(&app), + ); + } + + #[test] + fn simple_mode_submit_of_pure_dsl_error_has_no_advanced_pointer() { + // A DSL error that is *not* valid SQL either (unknown command) + // must not carry the advanced-mode pointer — there is nothing + // to switch modes for. + let mut app = App::new(); + type_str(&mut app, "frobulate widgets"); + let _ = submit(&mut app); + let has_pointer = app + .output + .iter() + .any(|l| l.text.contains("valid as SQL in advanced mode")); + assert!(!has_pointer, "unknown command must not point at advanced mode"); + } + #[test] fn enter_in_advanced_mode_dispatches_select_with_advanced_tag() { // The pre-ADR-0030 placeholder echoed any advanced-mode diff --git a/src/completion.rs b/src/completion.rs index 87f41be..2ef9580 100644 --- a/src/completion.rs +++ b/src/completion.rs @@ -973,6 +973,24 @@ pub fn invalid_ident_at_cursor( input: &str, cursor: usize, cache: &SchemaCache, +) -> Option { + invalid_ident_at_cursor_in_mode(input, cursor, cache, Mode::Advanced) +} + +/// Mode-aware [`invalid_ident_at_cursor`]. +/// +/// The slot's expected set is computed in `mode`, so a simple-mode +/// caller doesn't get the advanced (SQL) grammar's view of a shared +/// `insert`/`update`/`delete` entry word — e.g. it won't flag `rows` +/// as an "unknown column" in `update T … --all-rows`, which is a DSL +/// flag in simple mode rather than the SQL expression `- -all - rows` +/// (ADR-0033 Amendment 3). +#[must_use] +pub fn invalid_ident_at_cursor_in_mode( + input: &str, + cursor: usize, + cache: &SchemaCache, + mode: Mode, ) -> Option { let cursor = cursor.min(input.len()); let bytes = input.as_bytes(); @@ -1000,7 +1018,7 @@ pub fn invalid_ident_at_cursor( return None; } let leading = &input[..start]; - let expected = expected_at(leading, Mode::Advanced); + let expected = expected_at(leading, mode); if expected.is_empty() { return None; } @@ -1094,6 +1112,17 @@ mod tests { .map_or_else(Vec::new, |c| c.candidates.into_iter().map(|c| c.text).collect()) } + /// Simple-mode completion candidates — the DSL surface + /// (ADR-0003). Used by tests of DSL-only completion (the + /// `--all-rows` flag, the DSL value-literal slots), which since + /// sub-phase 3j must run in Simple mode: `insert`/`update`/ + /// `delete` are shared entry words (ADR-0033 Amendment 3) and + /// Advanced mode surfaces the SQL grammar's completions instead. + fn cands_simple(input: &str, cursor: usize) -> Vec { + candidates_at_cursor_in_mode(input, cursor, &SchemaCache::default(), Mode::Simple) + .map_or_else(Vec::new, |c| c.candidates.into_iter().map(|c| c.text).collect()) + } + fn cand_kinds_with( input: &str, cursor: usize, @@ -1178,14 +1207,17 @@ mod tests { // a `,` (more assignments), `where` (where clause), // or `--all-rows` (flag). Punctuation isn't surfaced; // `where` and `--all-rows` should appear. - let cs = cands("update T set Name='hi' ", 23); + // `--all-rows` is a DSL-only rail (Simple mode); in Advanced + // mode `update`/`delete` route to the SQL grammar, which has + // no such flag (ADR-0033 Amendment 3). + let cs = cands_simple("update T set Name='hi' ", 23); assert!(cs.contains(&"where".to_string()), "got {cs:?}"); assert!(cs.contains(&"--all-rows".to_string()), "got {cs:?}"); } #[test] fn delete_filter_position_offers_where_and_all_rows() { - let cs = cands("delete from T ", 14); + let cs = cands_simple("delete from T ", 14); assert!(cs.contains(&"where".to_string()), "got {cs:?}"); assert!(cs.contains(&"--all-rows".to_string()), "got {cs:?}"); } @@ -1195,12 +1227,18 @@ mod tests { // Hint-panel colouring distinguishes flags from // keywords (amber vs purple) — flags get their own // CandidateKind so the renderer can apply tok_flag. - let kinds = candidates_at_cursor("delete from T ", 14, &SchemaCache::default()) - .expect("some completion") - .candidates - .into_iter() - .map(|c| (c.text, c.kind)) - .collect::>(); + // Simple mode: `--all-rows` is the DSL rail. + let kinds = candidates_at_cursor_in_mode( + "delete from T ", + 14, + &SchemaCache::default(), + Mode::Simple, + ) + .expect("some completion") + .candidates + .into_iter() + .map(|c| (c.text, c.kind)) + .collect::>(); let flag = kinds .iter() .find(|(t, _)| t == "--all-rows") @@ -1210,7 +1248,7 @@ mod tests { #[test] fn flag_candidates_filter_by_partial_prefix() { - let cs = cands("delete from T --", 16); + let cs = cands_simple("delete from T --", 16); assert!(cs.contains(&"--all-rows".to_string()), "got {cs:?}"); } @@ -1327,7 +1365,9 @@ mod tests { // `true`, `false` as Tab candidates — actively // misleading at a slot where the user is more likely // entering a number / text / date. Suppress. - let cs = cands("insert into T values (", 22); + // DSL value-literal slot (Simple mode); in Advanced mode + // `insert` routes to the SQL grammar (ADR-0033 Amendment 3). + let cs = cands_simple("insert into T values (", 22); assert!(cs.is_empty(), "got misleading candidates {cs:?}"); } @@ -1337,15 +1377,15 @@ mod tests { // completion applies — `n` → `null`, `tr` → `true`, // `fa` → `false`. assert_eq!( - cands("insert into T values (n", 23), + cands_simple("insert into T values (n", 23), vec!["null".to_string()], ); assert_eq!( - cands("insert into T values (tr", 24), + cands_simple("insert into T values (tr", 24), vec!["true".to_string()], ); assert_eq!( - cands("insert into T values (fa", 24), + cands_simple("insert into T values (fa", 24), vec!["false".to_string()], ); } @@ -1355,21 +1395,21 @@ mod tests { // Comma-separated value positions all hit the same slot // signature. `insert into T values (1, ` → expected: // null/true/false/number/string. Suppress. - let cs = cands("insert into T values (1, ", 25); + let cs = cands_simple("insert into T values (1, ", 25); assert!(cs.is_empty(), "got {cs:?}"); } #[test] fn update_set_value_slot_suppresses() { // `update T set col=` is also a value-literal slot. - let cs = cands("update T set col=", 17); + let cs = cands_simple("update T set col=", 17); assert!(cs.is_empty(), "got {cs:?}"); } #[test] fn where_value_slot_suppresses() { // `where col=` is also a value-literal slot. - let cs = cands("delete from T where col=", 24); + let cs = cands_simple("delete from T where col=", 24); assert!(cs.is_empty(), "got {cs:?}"); } @@ -2166,7 +2206,7 @@ mod tests { // ADR-0033 §9: `excluded.|` inside a DO UPDATE action // completes to the INSERT target table's columns. let cache = two_table_schema(); - let input = "sqlinsert into a (id, name) values (1, 'x') \ + let input = "insert into a (id, name) values (1, 'x') \ on conflict (id) do update set name = excluded."; let cs = cands_with(input, input.len(), &cache); assert!( diff --git a/src/db.rs b/src/db.rs index 6f85407..43659f9 100644 --- a/src/db.rs +++ b/src/db.rs @@ -9573,8 +9573,13 @@ mod tests { /// Pull the `RowFilter` out of an `update` / `delete` parsed /// from DSL — the readable way to build a complex `Expr`. + /// Parses in Simple mode: `update`/`delete` are shared entry + /// words since sub-phase 3j (ADR-0033 Amendment 3), so the DSL + /// `Command::Update`/`Delete` is only produced in Simple mode. fn parse_filter(dsl: &str) -> RowFilter { - match crate::dsl::parser::parse_command(dsl).expect("filter parse") { + match crate::dsl::parser::parse_command_in_mode(dsl, crate::mode::Mode::Simple) + .expect("filter parse") + { crate::dsl::command::Command::Update { filter, .. } | crate::dsl::command::Command::Delete { filter, .. } => filter, other => panic!("expected update/delete, got {other:?}"), @@ -9775,9 +9780,14 @@ mod tests { // --- explain / query plans (ADR-0028) ------------------- /// Parse a non-`explain` query command for use as the inner - /// command of `explain_query_plan`. + /// command of `explain_query_plan`. Simple mode: `explain` + /// wraps the DSL `show data` / `update` / `delete` commands + /// (ADR-0028; SQL DML is not explainable, ADR-0030 §13 OOS-2), + /// and `update`/`delete` only yield the DSL variant in Simple + /// mode (shared entry words since sub-phase 3j). fn parse_inner(dsl: &str) -> Command { - crate::dsl::parser::parse_command(dsl).expect("inner command parse") + crate::dsl::parser::parse_command_in_mode(dsl, crate::mode::Mode::Simple) + .expect("inner command parse") } #[tokio::test] diff --git a/src/dsl/grammar/data.rs b/src/dsl/grammar/data.rs index 7daeffb..cc67b7a 100644 --- a/src/dsl/grammar/data.rs +++ b/src/dsl/grammar/data.rs @@ -32,8 +32,14 @@ use crate::dsl::walker::outcome::{MatchedItem, MatchedKind, MatchedPath}; const TABLE_NAME_EXISTING: Node = Node::Ident { source: IdentSource::Tables, + // Reject `__rdbms_*` internal tables at the table-source slot + // (ADR-0030 §6 — "every table-source slot"), matching the SQL + // grammar's `reject_internal_table`. Without this, simple-mode DSL + // data commands could read/write the internal metadata tables + // even though advanced-mode SQL rejects them (ADR-0033 + // Amendment 3 / `/runda` finding B). role: "table_name", - validator: None, + validator: Some(sql_select::reject_internal_table), highlight_override: None, writes_table: false, writes_column: false, @@ -49,8 +55,10 @@ writes_projection_alias: false, /// dispatch typed slots per column. const TABLE_NAME_INSERT: Node = Node::Ident { source: IdentSource::Tables, + // Reject `__rdbms_*` internal tables (ADR-0030 §6; `/runda` + // finding B) — see `TABLE_NAME_EXISTING`. role: "table_name", - validator: None, + validator: Some(sql_select::reject_internal_table), highlight_override: None, writes_table: true, writes_column: false, @@ -224,8 +232,12 @@ const INSERT_SHAPE: Node = Node::Seq(INSERT_NODES); /// can resolve column types (Phase D). const TABLE_NAME_WRITES: Node = Node::Ident { source: IdentSource::Tables, + // Reject `__rdbms_*` internal tables (ADR-0030 §6; `/runda` + // finding B) — see `TABLE_NAME_EXISTING`. Shared by `update`, + // `delete`, and `show data`, so all three reject the internal + // metadata tables, matching the SQL grammar. role: "table_name", - validator: None, + validator: Some(sql_select::reject_internal_table), highlight_override: None, writes_table: true, writes_column: false, @@ -856,14 +868,10 @@ fn build_select(_path: &MatchedPath, source: &str) -> Result Result { let target_table = path .items @@ -938,13 +946,10 @@ fn build_sql_insert(path: &MatchedPath, source: &str) -> Result bool { } /// Build `Command::SqlUpdate` from a validated SQL `UPDATE` -/// (ADR-0033 §2, sub-phase 3e). Extracts the target table from the -/// matched path so the worker re-persists the right CSV. -/// -/// Dev-scaffold detail: the entry word is `sql_update` (not valid -/// SQL), so the statement is reconstructed as `update` + the -/// matched tail. Sub-phase 3j wires the real `update` entry word, -/// at which point this collapses to `source.trim()`. +/// (ADR-0033 §2). Extracts the target table from the matched path +/// so the worker re-persists the right CSV. `update` is now the +/// real (shared) entry word, so the validated `source` runs +/// verbatim (sub-phase 3j). fn build_sql_update(path: &MatchedPath, source: &str) -> Result { // The UPDATE target is the first `table_name` ident (it // precedes any table referenced inside a SET / WHERE subquery). @@ -986,11 +988,7 @@ fn build_sql_update(path: &MatchedPath, source: &str) -> Result None, }) .unwrap_or_default(); - let tail = path - .items - .first() - .map_or(source, |entry| &source[entry.span.1..]); - let sql = format!("update {}", tail.trim()); + let sql = source.trim().to_string(); Ok(Command::SqlUpdate { sql, target_table, @@ -999,17 +997,13 @@ 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). @@ -1023,11 +1017,7 @@ fn build_sql_delete(path: &MatchedPath, source: &str) -> Result None, }) .unwrap_or_default(); - let tail = path - .items - .first() - .map_or(source, |entry| &source[entry.span.1..]); - let sql = format!("delete {}", tail.trim()); + let sql = source.trim().to_string(); Ok(Command::SqlDelete { sql, target_table, @@ -1110,51 +1100,46 @@ pub static WITH: CommandNode = CommandNode { help_id: None, usage_ids: &["parse.usage.select"],}; -/// SQL `INSERT` development scaffold (ADR-0033 sub-phase 3b–3i). +/// SQL `INSERT` — the `Advanced`-category node of the shared +/// `insert` entry word (ADR-0033 §2, Amendment 1, sub-phase 3j). /// -/// Registered under the temporary entry word `sqlinsert` so the -/// SQL INSERT grammar and execution path can be exercised in -/// isolation, WITHOUT yet making `insert` a shared DSL/SQL entry -/// word. Sharing `insert` is sub-phase 3j, which depends on -/// `shortid` auto-fill (3d) so advanced-mode DSL inserts keep -/// parity rather than regressing through an incomplete SQL path. -/// This scaffold (entry word + reconstruction in `build_sql_insert`) -/// is removed when 3j wires the real `insert` entry word. +/// `insert` is a shared entry word: this `Advanced` SQL node and +/// the `Simple` DSL [`INSERT`] node both register under `insert`. +/// In Advanced mode the dispatcher (`walker::walk` / `decide`) +/// tries this SQL node first and falls back to the DSL node when +/// the SQL shape does not match; in Simple mode only the DSL node +/// is reachable (Amendment 3 — command identity is the mode-rooted +/// grammar-path outcome). pub static SQL_INSERT: CommandNode = CommandNode { - entry: Word::keyword("sqlinsert"), + entry: Word::keyword("insert"), shape: Node::Subgrammar(&sql_insert::SQL_INSERT_SHAPE), ast_builder: build_sql_insert, help_id: None, usage_ids: &[], }; -/// SQL `UPDATE` development scaffold (ADR-0033 sub-phase 3e). +/// SQL `UPDATE` — the `Advanced` node of the shared `update` word. /// -/// Registered under the temporary entry word `sql_update` so the -/// SQL UPDATE grammar and execution path can be exercised in -/// isolation, WITHOUT yet making `update` a shared DSL/SQL entry -/// word. Sharing `update` is sub-phase 3j. This scaffold (entry -/// word + reconstruction in `build_sql_update`) is removed when 3j -/// wires the real `update` entry word. +/// ADR-0033 §2 / Amendment 1, sub-phase 3j. Pairs with the `Simple` +/// DSL [`UPDATE`] node; dispatch is SQL-first / DSL-fallback in +/// Advanced mode, DSL-only in Simple. pub static SQL_UPDATE: CommandNode = CommandNode { - entry: Word::keyword("sql_update"), + entry: Word::keyword("update"), shape: Node::Subgrammar(&sql_update::SQL_UPDATE_SHAPE), ast_builder: build_sql_update, help_id: None, usage_ids: &[], }; -/// SQL `DELETE` development scaffold (ADR-0033 sub-phase 3f). +/// SQL `DELETE` — the `Advanced` node of the shared `delete` word. /// -/// 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. +/// ADR-0033 §2 / Amendment 1, sub-phase 3j. Pairs with the `Simple` +/// DSL [`DELETE`] node; dispatch is SQL-first / DSL-fallback in +/// Advanced mode, DSL-only in Simple. In Advanced mode `delete from t +/// --all-rows` falls back to the DSL node (the SQL shape has no +/// `--all-rows`). pub static SQL_DELETE: CommandNode = CommandNode { - entry: Word::keyword("sql_delete"), + entry: Word::keyword("delete"), shape: Node::Subgrammar(&sql_delete::SQL_DELETE_SHAPE), ast_builder: build_sql_delete, help_id: None, diff --git a/src/dsl/grammar/mod.rs b/src/dsl/grammar/mod.rs index dcb6d03..4284229 100644 --- a/src/dsl/grammar/mod.rs +++ b/src/dsl/grammar/mod.rs @@ -573,17 +573,13 @@ pub static REGISTRY: &[(&CommandNode, CommandCategory)] = &[ (&data::EXPLAIN, CommandCategory::Simple), (&data::SELECT, CommandCategory::Advanced), (&data::WITH, CommandCategory::Advanced), - // SQL INSERT development scaffold (sub-phase 3b–3i); the - // temporary `sqlinsert` entry word keeps it isolated from the - // DSL `insert` word until 3j wires the shared entry. + // Shared entry words (sub-phase 3j, ADR-0033 §2 / Amendment 1): + // `insert` / `update` / `delete` each appear twice — the + // `Simple` DSL node above and this `Advanced` SQL node. The + // dispatcher tries the SQL node first in Advanced mode and falls + // back to the DSL node when the SQL shape does not match. (&data::SQL_INSERT, CommandCategory::Advanced), - // SQL UPDATE development scaffold (sub-phase 3e); the temporary - // `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), ]; diff --git a/src/dsl/grammar/sql_delete.rs b/src/dsl/grammar/sql_delete.rs index 2bb8330..b0ac701 100644 --- a/src/dsl/grammar/sql_delete.rs +++ b/src/dsl/grammar/sql_delete.rs @@ -7,8 +7,9 @@ //! 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`). +//! shape opens at `FROM`. `delete` is a shared entry word +//! (sub-phase 3j): this `Advanced` SQL shape and the `Simple` DSL +//! delete node both register under `delete`. //! //! Scope (3f): `FROM [ WHERE … ] [ ';' ]`, the `__rdbms_*` //! target rejection, and the shared `sql_expr` on the WHERE diff --git a/src/dsl/grammar/sql_update.rs b/src/dsl/grammar/sql_update.rs index 53a5f05..b029b9d 100644 --- a/src/dsl/grammar/sql_update.rs +++ b/src/dsl/grammar/sql_update.rs @@ -5,8 +5,9 @@ //! validated SQL text and re-persists the target table's CSV //! (ADR-0030 §11). The shape here is the post-`UPDATE` portion — //! the entry-word dispatch consumes the leading `UPDATE` keyword -//! before this shape walks (mirroring `sql_insert::SQL_INSERT_SHAPE`, -//! where the dev `sqlinsert` word stands in for `INSERT`). +//! before this shape walks. `update` is a shared entry word +//! (sub-phase 3j): this `Advanced` SQL shape and the `Simple` DSL +//! update node both register under `update`. //! //! Scope (3e): `
SET assignment_list [ WHERE … ]`, the //! `__rdbms_*` target rejection, and the shared `sql_expr` on both diff --git a/src/dsl/parser.rs b/src/dsl/parser.rs index 2a89a0d..b95a699 100644 --- a/src/dsl/parser.rs +++ b/src/dsl/parser.rs @@ -360,12 +360,23 @@ mod tests { use crate::dsl::value::Value; use pretty_assertions::assert_eq; + // These helpers parse in **Simple mode** — the DSL surface + // (ADR-0003). The tests in this module exercise the DSL + // grammar (`insert`/`update`/`delete` Forms A/B/C, the + // `--all-rows` rail, DDL, app commands), all of which are + // canonical in Simple mode. Since sub-phase 3j made + // `insert`/`update`/`delete` shared entry words (ADR-0033 §2, + // Amendment 3), parsing these in Advanced mode would route the + // overlap to the SQL command variants; the SQL surface is + // covered by `tests/sql_*.rs` instead. No SQL-only command + // (`select`/`with`) is tested through these helpers. fn ok(input: &str) -> Command { - parse_command(input).unwrap_or_else(|e| panic!("expected ok for {input:?}, got {e:?}")) + parse_command_in_mode(input, Mode::Simple) + .unwrap_or_else(|e| panic!("expected ok for {input:?}, got {e:?}")) } fn err(input: &str) -> ParseError { - parse_command(input).expect_err("expected parse error") + parse_command_in_mode(input, Mode::Simple).expect_err("expected parse error") } fn err_message(input: &str) -> String { @@ -1163,6 +1174,147 @@ mod tests { assert!(matches!(e, ParseError::Invalid { .. }), "got {e:?}"); } + // ===================================================== + // Sub-phase 3j — shared-entry-word dispatch (ADR-0033 §2, + // Amendment 1 / Amendment 3). + // + // `insert` / `update` / `delete` are *shared* entry words: a + // `Simple` DSL node and an `Advanced` SQL node both register + // under each. A command's identity is the outcome of the + // mode-rooted grammar path: + // - Advanced mode tries the SQL shape first and falls back to + // the DSL shape only when the SQL shape *structurally* can't + // match (e.g. the DSL-only `--all-rows` flag). A content + // rejection (a `__rdbms_*` target) on the SQL shape is + // surfaced, never masked by the DSL fallback. + // - Simple mode commits the DSL shape; it points the user at + // advanced mode ("this is SQL") only when the input is + // SQL-only (the DSL shape structurally mismatches and the SQL + // shape matches — e.g. a `returning` tail). A DSL command + // that is merely incomplete or has a bad value still commits + // the DSL node so the user sees DSL completion / DSL errors. + // The §6/§7 parity guarantees mean the two variants execute to + // identical effects for an overlapping input. + // ===================================================== + + #[test] + fn advanced_ambiguous_insert_routes_to_sql() { + assert!(matches!( + parse_command_in_mode("insert into Orders values (1, 2)", Mode::Advanced), + Ok(Command::SqlInsert { .. }) + )); + } + + #[test] + fn advanced_ambiguous_update_routes_to_sql() { + assert!(matches!( + parse_command_in_mode( + "update Orders set total = 0 where id = 1", + Mode::Advanced, + ), + Ok(Command::SqlUpdate { .. }) + )); + } + + #[test] + fn advanced_ambiguous_delete_routes_to_sql() { + assert!(matches!( + parse_command_in_mode("delete from Orders where id = 1", Mode::Advanced), + Ok(Command::SqlDelete { .. }) + )); + } + + #[test] + fn advanced_dsl_only_delete_falls_back_to_dsl() { + // `--all-rows` is DSL-only; the SQL DELETE shape can't consume + // the trailing flag, so dispatch falls back to the DSL node. + assert_eq!( + parse_command_in_mode("delete from Orders --all-rows", Mode::Advanced).unwrap(), + Command::Delete { + table: "Orders".to_string(), + filter: RowFilter::AllRows, + }, + ); + } + + #[test] + fn simple_mode_data_commands_reject_internal_tables() { + // ADR-0030 §6 ("every table-source slot") / `/runda` finding + // B: the DSL data-command target slots reject `__rdbms_*` + // internal tables in simple mode too — matching the SQL + // grammar. Without this, simple-mode DML could read/write the + // internal metadata tables while advanced-mode SQL rejected + // them. + for input in [ + "insert into __rdbms_playground_columns values (1)", + "update __rdbms_playground_columns set x = 1 where id = 1", + "delete from __rdbms_playground_columns where id = 1", + "show data __rdbms_playground_columns", + "show table __rdbms_playground_relationships", + ] { + assert!( + parse_command_in_mode(input, Mode::Simple).is_err(), + "internal table must be rejected in simple mode: {input:?}", + ); + } + } + + #[test] + fn advanced_internal_table_insert_is_rejected_not_fallen_back() { + // The SQL insert's `reject_internal_table` rail must surface + // even though the DSL insert node lacks it: a content + // rejection commits the SQL candidate rather than falling + // through to the DSL node that would accept it. + assert!( + parse_command_in_mode( + "insert into __rdbms_playground_columns values (1)", + Mode::Advanced, + ) + .is_err(), + ); + } + + #[test] + fn simple_dsl_delete_stays_dsl() { + assert_eq!( + parse_command_in_mode("delete from Orders where id = 1", Mode::Simple).unwrap(), + Command::Delete { + table: "Orders".to_string(), + filter: RowFilter::eq("id", Value::Number("1".to_string())), + }, + ); + } + + #[test] + fn simple_sql_only_entry_word_points_at_advanced_mode() { + // A SQL-only *entry word* (`select`) has no DSL form, so + // simple mode emits the "this is SQL" hint at the parse level + // (ADR-0030 §2). + match parse_command_in_mode("select Name from Orders", Mode::Simple) { + Err(ParseError::Invalid { message, .. }) => assert!( + message.contains("advanced"), + "expected the this-is-SQL hint, got: {message}", + ), + other => panic!("expected the this-is-SQL hint, got {other:?}"), + } + } + + #[test] + fn simple_shared_word_with_sql_construct_is_a_dsl_parse_error() { + // `returning` is SQL-only, but `delete` is a *shared* entry + // word, so simple mode commits the DSL shape and surfaces a + // DSL parse error (ADR-0033 Amendment 3). The "(valid as SQL + // in advanced mode)" pointer is added at the hint layer + // (input_render), not in the parsed command/error here. + assert!(matches!( + parse_command_in_mode( + "delete from Orders where id = 1 returning *", + Mode::Simple, + ), + Err(ParseError::Invalid { .. }) + )); + } + #[test] fn show_data_command() { assert_eq!( diff --git a/src/dsl/walker/mod.rs b/src/dsl/walker/mod.rs index 60e5115..edf3cc4 100644 --- a/src/dsl/walker/mod.rs +++ b/src/dsl/walker/mod.rs @@ -293,12 +293,18 @@ pub fn completion_probe_in_mode( use crate::dsl::grammar::{REGISTRY, is_advanced_only}; let mode_filtered_entries = || -> Vec { + // De-duplicate shared entry words (sub-phase 3j): `insert` / + // `update` / `delete` each appear twice in `REGISTRY` (a + // `Simple` DSL node and an `Advanced` SQL node), but the + // entry word should be offered to the user only once. + let mut seen = std::collections::HashSet::new(); REGISTRY .iter() .filter(|(c, _)| { mode == crate::mode::Mode::Advanced || !is_advanced_only(c.entry.primary) }) + .filter(|(c, _)| seen.insert(c.entry.primary)) .map(|(c, _)| outcome::Expectation::Word(c.entry.primary)) .collect() }; @@ -2096,12 +2102,17 @@ pub fn expected_at_input_in_mode( use crate::dsl::grammar::{REGISTRY, is_advanced_only}; let mode_filtered = || -> Vec { + // De-duplicate shared entry words (sub-phase 3j): `insert` / + // `update` / `delete` each appear twice in `REGISTRY` (Simple + // DSL + Advanced SQL nodes); offer the word once. + let mut seen = std::collections::HashSet::new(); REGISTRY .iter() .filter(|(c, _)| { mode == crate::mode::Mode::Advanced || !is_advanced_only(c.entry.primary) }) + .filter(|(c, _)| seen.insert(c.entry.primary)) .map(|(c, _)| outcome::Expectation::Word(c.entry.primary)) .collect() }; @@ -2353,27 +2364,25 @@ fn decide( match mode { crate::mode::Mode::Simple => { - let Some(&(sidx, snode)) = simple.first() else { - // No DSL candidate — the entry word is SQL-only. - let primary = candidates.first().map_or("", |(_, n, _)| n.entry.primary); - return Decision::ThisIsSql { primary }; - }; - if advanced.is_empty() { - return Decision::Commit { idx: sidx, node: snode }; + // Simple mode is the DSL surface (ADR-0003). A shared + // entry word (`insert`/`update`/`delete`) commits its DSL + // candidate so the user sees DSL completion and the *real* + // DSL error — with its position — rather than a bare + // "this is SQL" that discards the actionable detail + // (ADR-0033 Amendment 3). "This is SQL" is reserved for + // entry words with no DSL form (`select` / `with`): there + // the DSL surface has nothing to offer. For a DSL line + // that fails here but *would* run in advanced mode, a + // "(valid as SQL in advanced mode)" pointer is appended at + // the rendering layer (see `advanced_alternative_note`), + // combining the DSL fix with the mode hint. + match simple.first() { + Some(&(sidx, snode)) => Decision::Commit { idx: sidx, node: snode }, + None => { + let primary = candidates.first().map_or("", |(_, n, _)| n.entry.primary); + Decision::ThisIsSql { primary } + } } - // Shared entry word: prefer the DSL node; only point - // at advanced mode when the DSL shape does not match - // but the SQL shape does. - if scratch_full_match(effective_source, kw_start, kw_end, snode, mode, schema) { - return Decision::Commit { idx: sidx, node: snode }; - } - let (_, anode) = advanced[0]; - if scratch_full_match(effective_source, kw_start, kw_end, anode, mode, schema) { - return Decision::ThisIsSql { - primary: anode.entry.primary, - }; - } - Decision::Commit { idx: sidx, node: snode } } crate::mode::Mode::Advanced => { // Advanced candidates first, DSL as the fallback. @@ -2385,13 +2394,28 @@ fn decide( let (idx, node) = ordered[0]; return Decision::Commit { idx, node }; } + // Commit the first candidate that fully matches OR is + // rejected by a content validator. A `ValidationFailed` + // means the shape *fits* but the content is invalid (e.g. + // an internal `__rdbms_*` target rejected by + // `reject_internal_table`); that error must surface rather + // than being masked by falling back to a candidate that + // lacks the validator (sub-phase 3j — the DSL `insert` + // node has no internal-table rail). A structural + // `Mismatch` (e.g. the DSL-only `--all-rows` the SQL shape + // can't consume) is *not* committed here, so the fallback + // to the DSL node still works. for &(idx, node) in &ordered { - if scratch_full_match(effective_source, kw_start, kw_end, node, mode, schema) { + if matches!( + scratch_outcome(effective_source, kw_start, kw_end, node, mode, schema), + WalkOutcome::Match { .. } | WalkOutcome::ValidationFailed { .. } + ) { return Decision::Commit { idx, node }; } } - // None fully matched — commit the furthest-progress - // candidate, keeping the first (advanced) on ties. + // None matched or content-rejected — commit the + // furthest-progress candidate, keeping the first + // (advanced) on ties. let mut best = ordered[0]; let mut best_progress = scratch_progress(effective_source, kw_start, kw_end, best.1, mode, schema); @@ -2466,22 +2490,6 @@ fn scratch_outcome( result.outcome } -/// Whether a candidate fully matches the input (a clean -/// `WalkOutcome::Match`), tested on a scratch context. -fn scratch_full_match( - effective_source: &str, - kw_start: usize, - kw_end: usize, - node: &'static crate::dsl::grammar::CommandNode, - mode: crate::mode::Mode, - schema: Option<&crate::completion::SchemaCache>, -) -> bool { - matches!( - scratch_outcome(effective_source, kw_start, kw_end, node, mode, schema), - WalkOutcome::Match { .. } - ) -} - /// How far (byte position) a candidate's walk progressed. A full /// match scores the whole input; a failure scores its failure /// position. Used only to tie-break when no candidate fully @@ -2709,10 +2717,20 @@ mod tests { //! the walker's contract for the migrated commands so //! Phase B-F migrations can refactor without regression. use crate::dsl::command::{AppCommand, Command, MessagesValue, ModeValue}; - use crate::dsl::parser::parse_command; + use crate::dsl::parser::parse_command_in_mode; + use crate::mode::Mode; + /// Parse in **Simple mode** — the DSL surface (ADR-0003). The + /// tests in this module exercise the DSL grammar (app commands, + /// DDL, and the `insert`/`update`/`delete` DSL forms), all + /// canonical in Simple mode. Sub-phase 3j made + /// `insert`/`update`/`delete` shared entry words (ADR-0033 §2, + /// Amendment 3), so parsing them in Advanced mode would route the + /// overlap to the SQL command variants; the SQL surface is + /// validated through the advanced-mode diagnostic helpers + /// (`diag_keys` / `diagnostics_advanced`) and `tests/sql_*.rs`. fn parse(input: &str) -> Result { - parse_command(input) + parse_command_in_mode(input, Mode::Simple) } // ---- Bare no-arg commands --------------------------------- @@ -3637,11 +3655,18 @@ mod tests { #[test] fn not_like_on_a_numeric_column_is_also_a_warning() { + // The `LIKE`-on-numeric predicate warning is a DSL diagnostic + // (its sibling `like_on_a_numeric_column_is_a_warning` uses + // the Simple-mode `diagnostics` helper). Since sub-phase 3j + // made `delete` a shared entry word (ADR-0033 Amendment 3), + // this verdict is taken in Simple mode so the input routes to + // the DSL `delete` and its predicate diagnostics. let schema = schema_with("Orders", &[("Total", Type::Decimal)]); assert_eq!( - super::input_verdict( + super::input_verdict_in_mode( "delete from Orders where Total not like '9%'", Some(&schema), + crate::mode::Mode::Simple, ), Some(super::Severity::Warning), ); @@ -3939,7 +3964,19 @@ mod tests { // ========================================================= use crate::completion::{SchemaCache, TableColumn}; - use crate::dsl::parser::parse_command_with_schema; + use crate::dsl::parser::parse_command_with_schema_in_mode; + + /// Phase-D typed value slots are the **DSL** surface (Simple + /// mode). Sub-phase 3j made `insert`/`update`/`delete` shared + /// entry words (ADR-0033 Amendment 3), so these typed-slot tests + /// parse in Simple mode to reach the DSL grammar rather than the + /// SQL one. Thin wrapper keeps the existing call sites unchanged. + fn parse_command_with_schema( + input: &str, + schema: &SchemaCache, + ) -> Result { + parse_command_with_schema_in_mode(input, schema, crate::mode::Mode::Simple) + } fn schema_with(table: &str, columns: &[(&str, Type)]) -> SchemaCache { let cols: Vec = columns @@ -4522,7 +4559,7 @@ mod tests { // list (the target isn't a binding, so a targeted pass covers // it). let schema = schema_with("t", &[("a", Type::Int), ("b", Type::Text)]); - let diags = diag_keys("sqlinsert into t (nonexistent) values (1)", &schema); + let diags = diag_keys("insert into t (nonexistent) values (1)", &schema); assert!( diags.iter().any(|d| d.contains("no such column")), "unknown INSERT column should be flagged; got {diags:?}", @@ -4537,7 +4574,7 @@ mod tests { // SELECT's source tables — otherwise the flat-scope // bare-column branch falsely flags it against `b`. let schema = two_table_schema(); // a(id,name), b(id,total) - let diags = diag_keys("sqlinsert into a (name) select total from b", &schema); + let diags = diag_keys("insert into a (name) select total from b", &schema); assert!( !diags.iter().any(|d| d.contains("no such column")), "target column `name` must not be flagged against the SELECT source; got {diags:?}", @@ -4552,7 +4589,7 @@ mod tests { // against `b`. let schema = two_table_schema(); let diags = diag_keys( - "sqlinsert into a (id) select id from b on conflict (name) do nothing", + "insert into a (id) select id from b on conflict (name) do nothing", &schema, ); assert!( @@ -4568,7 +4605,7 @@ mod tests { // resolves to the target `a`, not the SELECT source `b`. let schema = two_table_schema(); let diags = diag_keys( - "sqlinsert into a (id) select id from b on conflict (id) do update set name = name", + "insert into a (id) select id from b on conflict (id) do update set name = name", &schema, ); assert!( @@ -4584,7 +4621,7 @@ mod tests { // against the target). Covers the SET RHS and the WHERE. let schema = schema_with("t", &[("a", Type::Int), ("b", Type::Text)]); let set_rhs = diag_keys( - "sqlinsert into t (a, b) values (1, 'x') on conflict (a) do update set b = nope", + "insert into t (a, b) values (1, 'x') on conflict (a) do update set b = nope", &schema, ); assert!( @@ -4592,7 +4629,7 @@ mod tests { "unknown DO UPDATE SET RHS ref should be flagged; got {set_rhs:?}", ); let where_ref = diag_keys( - "sqlinsert into t (a, b) values (1, 'x') on conflict (a) do update set b = 'y' where nope = 1", + "insert into t (a, b) values (1, 'x') on conflict (a) do update set b = 'y' where nope = 1", &schema, ); assert!( @@ -4608,7 +4645,7 @@ mod tests { // target-only column must resolve to the target `a`, not be // flagged against the SELECT source `b`. let schema = two_table_schema(); // a(id,name), b(id,total) - let diags = diag_keys("sqlinsert into a (id) select id from b returning name", &schema); + let diags = diag_keys("insert into a (id) select id from b returning name", &schema); assert!( !diags.iter().any(|d| d.contains("no such column")), "RETURNING `name` (a's column) must not flag against `b`; got {diags:?}", @@ -4621,7 +4658,7 @@ mod tests { // qualified star) in an INSERT … SELECT resolves to the // target `a`, not flagged against the SELECT source `b`. let schema = two_table_schema(); - let diags = diag_keys("sqlinsert into a (id) select id from b returning a.*", &schema); + let diags = diag_keys("insert into a (id) select id from b returning a.*", &schema); assert!( diags.is_empty(), "target-qualified `a.*` in RETURNING must resolve cleanly; got {diags:?}", @@ -4631,7 +4668,7 @@ mod tests { #[test] fn unrelated_qualified_star_in_returning_still_flagged() { let schema = two_table_schema(); - let diags = diag_keys("sqlinsert into a (id) select id from b returning zzz.*", &schema); + let diags = diag_keys("insert into a (id) select id from b returning zzz.*", &schema); assert!( diags.iter().any(|d| d.contains("no such table or alias")), "unrelated `zzz.*` qualifier should still flag; got {diags:?}", @@ -4645,7 +4682,7 @@ mod tests { // target `a`, not flagged as an unknown qualifier (a is the // target, b is the SELECT source). let schema = two_table_schema(); - let diags = diag_keys("sqlinsert into a (id) select id from b returning a.name", &schema); + let diags = diag_keys("insert into a (id) select id from b returning a.name", &schema); assert!( diags.is_empty(), "target-qualified RETURNING ref must resolve cleanly; got {diags:?}", @@ -4656,7 +4693,7 @@ mod tests { fn target_qualified_ref_unknown_column_still_flagged() { // `a.nope` — a is the target but nope isn't its column. let schema = two_table_schema(); - let diags = diag_keys("sqlinsert into a (id) select id from b returning a.nope", &schema); + let diags = diag_keys("insert into a (id) select id from b returning a.nope", &schema); assert!( diags.iter().any(|d| d.contains("no such column")), "unknown column under the target qualifier should flag; got {diags:?}", @@ -4668,7 +4705,7 @@ mod tests { // A qualifier that's neither `excluded` nor the target is // still an unknown qualifier (the leak guard holds). let schema = two_table_schema(); - let diags = diag_keys("sqlinsert into a (id) select id from b returning zzz.name", &schema); + let diags = diag_keys("insert into a (id) select id from b returning zzz.name", &schema); assert!( diags.iter().any(|d| d.contains("no such table or alias")), "unrelated qualifier should still flag; got {diags:?}", @@ -4680,7 +4717,7 @@ mod tests { // Flip side: a RETURNING ref to a column on neither table is // flagged (against the INSERT target). let schema = two_table_schema(); - let diags = diag_keys("sqlinsert into a (id) select id from b returning nope", &schema); + let diags = diag_keys("insert into a (id) select id from b returning nope", &schema); assert!( diags.iter().any(|d| d.contains("no such column")), "unknown RETURNING column should be flagged; got {diags:?}", @@ -4692,9 +4729,9 @@ mod tests { // The VALUES INSERT … RETURNING case (no bindings at all): // a valid target column is silent, an unknown one flags. let schema = schema_with("t", &[("a", Type::Int), ("b", Type::Text)]); - let ok = diag_keys("sqlinsert into t (a) values (1) returning b", &schema); + let ok = diag_keys("insert into t (a) values (1) returning b", &schema); assert!(!ok.iter().any(|d| d.contains("no such column")), "got {ok:?}"); - let bad = diag_keys("sqlinsert into t (a) values (1) returning nope", &schema); + let bad = diag_keys("insert into t (a) values (1) returning nope", &schema); assert!(bad.iter().any(|d| d.contains("no such column")), "got {bad:?}"); } @@ -4703,7 +4740,7 @@ mod tests { // And a valid bare ref to a target column is silent. let schema = schema_with("t", &[("a", Type::Int), ("b", Type::Text)]); let diags = diag_keys( - "sqlinsert into t (a, b) values (1, 'x') on conflict (a) do update set b = 'y' where a > 0", + "insert into t (a, b) values (1, 'x') on conflict (a) do update set b = 'y' where a > 0", &schema, ); assert!( @@ -4718,7 +4755,7 @@ mod tests { // neither table is flagged (against the INSERT target). let schema = two_table_schema(); let diags = diag_keys( - "sqlinsert into a (id) values (1) on conflict (nope) do nothing", + "insert into a (id) values (1) on conflict (nope) do nothing", &schema, ); assert!( @@ -4732,7 +4769,7 @@ mod tests { // The flip side: a column in neither the target nor the source // is still flagged (against the target, by the dedicated pass). let schema = two_table_schema(); - let diags = diag_keys("sqlinsert into a (nope) select total from b", &schema); + let diags = diag_keys("insert into a (nope) select total from b", &schema); assert!( diags.iter().any(|d| d.contains("no such column")), "a genuinely unknown INSERT column should still be flagged; got {diags:?}", @@ -4742,7 +4779,7 @@ mod tests { #[test] fn insert_column_list_known_columns_silent() { let schema = schema_with("t", &[("a", Type::Int), ("b", Type::Text)]); - let diags = diag_keys("sqlinsert into t (a, b) values (1, 'x')", &schema); + let diags = diag_keys("insert into t (a, b) values (1, 'x')", &schema); assert!( !diags.iter().any(|d| d.contains("no such column")), "known columns must not flag; got {diags:?}", @@ -4755,7 +4792,7 @@ mod tests { // SET column — an unknown DO UPDATE SET column is flagged. let schema = schema_with("t", &[("a", Type::Int), ("b", Type::Text)]); let diags = diag_keys( - "sqlinsert into t (a, b) values (1, 'x') on conflict (a) do update set nosuch = 1", + "insert into t (a, b) values (1, 'x') on conflict (a) do update set nosuch = 1", &schema, ); assert!( @@ -4768,7 +4805,7 @@ mod tests { fn upsert_do_update_known_set_column_silent() { let schema = schema_with("t", &[("a", Type::Int), ("b", Type::Text)]); let diags = diag_keys( - "sqlinsert into t (a, b) values (1, 'x') on conflict (a) do update set b = excluded.b", + "insert into t (a, b) values (1, 'x') on conflict (a) do update set b = excluded.b", &schema, ); assert!( @@ -4782,7 +4819,7 @@ mod tests { // 3i cross-cut: the Phase-2 predicate-warning pass fires on a // DML SET/WHERE sql_expr slot (here `= NULL` in an UPDATE). let schema = schema_with("t", &[("id", Type::Int), ("v", Type::Int)]); - let diags = diag_keys("sql_update t set v = 1 where v = NULL", &schema); + let diags = diag_keys("update t set v = 1 where v = NULL", &schema); assert!( diags.iter().any(|d| d.contains("IS NULL")), "eq_null should fire on the UPDATE WHERE; got {diags:?}", @@ -4796,7 +4833,7 @@ mod tests { "t", &[("a", Type::Int, false, false), ("b", Type::Text, true, false)], ); - let diags = diag_keys("sqlinsert into t (a) values (1)", &schema); + let diags = diag_keys("insert into t (a) values (1)", &schema); assert!( diags.iter().any(|d| d.contains("is required")), "omitting NOT NULL `b` should warn; got {diags:?}", @@ -4809,7 +4846,7 @@ mod tests { "t", &[("a", Type::Int, false, false), ("b", Type::Text, true, false)], ); - let diags = diag_keys("sqlinsert into t (a, b) values (1, 'x')", &schema); + let diags = diag_keys("insert into t (a, b) values (1, 'x')", &schema); assert!( !diags.iter().any(|d| d.contains("is required")), "including `b` must not warn; got {diags:?}", @@ -4823,7 +4860,7 @@ mod tests { "t", &[("a", Type::Int, false, false), ("b", Type::Text, true, true)], ); - let diags = diag_keys("sqlinsert into t (a) values (1)", &schema); + let diags = diag_keys("insert into t (a) values (1)", &schema); assert!( !diags.iter().any(|d| d.contains("is required")), "a defaulted column must not warn; got {diags:?}", @@ -4838,7 +4875,7 @@ mod tests { "t", &[("id", Type::Serial, true, false), ("b", Type::Text, false, false)], ); - let diags = diag_keys("sqlinsert into t (b) values ('x')", &schema); + let diags = diag_keys("insert into t (b) values ('x')", &schema); assert!( !diags.iter().any(|d| d.contains("is required")), "auto-gen serial must not warn; got {diags:?}", @@ -4848,7 +4885,7 @@ mod tests { #[test] fn insert_arity_mismatch_single_row_fires() { let schema = schema_with("t", &[("a", Type::Int), ("b", Type::Int)]); - let diags = diag_keys("sqlinsert into t (a, b) values (1, 2, 3)", &schema); + let diags = diag_keys("insert into t (a, b) values (1, 2, 3)", &schema); assert!( diags.iter().any(|d| d.contains("value(s) are given")), "3 values for 2 columns should fire; got {diags:?}", @@ -4858,7 +4895,7 @@ mod tests { #[test] fn insert_arity_match_is_silent() { let schema = schema_with("t", &[("a", Type::Int), ("b", Type::Int)]); - let diags = diag_keys("sqlinsert into t (a, b) values (1, 2)", &schema); + let diags = diag_keys("insert into t (a, b) values (1, 2)", &schema); assert!( !diags.iter().any(|d| d.contains("value(s) are given")), "matched arity must not fire; got {diags:?}", @@ -4871,7 +4908,7 @@ mod tests { // diagnostic; the matched rows stay silent. let schema = schema_with("t", &[("a", Type::Int), ("b", Type::Int)]); let diags = diag_keys( - "sqlinsert into t (a, b) values (1, 2), (3, 4, 5), (6), (7, 8)", + "insert into t (a, b) values (1, 2), (3, 4, 5), (6), (7, 8)", &schema, ); let n = diags.iter().filter(|d| d.contains("value(s) are given")).count(); @@ -4885,7 +4922,7 @@ mod tests { // tuple mismatch still fires; the conflict target never does. let schema = schema_with("t", &[("a", Type::Int), ("b", Type::Int)]); let diags = diag_keys( - "sqlinsert into t (a, b) values (1, 2, 3) on conflict (a) do nothing", + "insert into t (a, b) values (1, 2, 3) on conflict (a) do nothing", &schema, ); let n = diags.iter().filter(|d| d.contains("value(s) are given")).count(); @@ -4898,7 +4935,7 @@ mod tests { // silent (the conflict target isn't counted as a tuple). let schema = schema_with("t", &[("a", Type::Int), ("b", Type::Int)]); let diags = diag_keys( - "sqlinsert into t (a, b) values (1, 2) on conflict (a) do update set b = excluded.b", + "insert into t (a, b) values (1, 2) on conflict (a) do update set b = excluded.b", &schema, ); assert!( @@ -4913,7 +4950,7 @@ mod tests { // the walk must stop there (same guard as ON CONFLICT) so the // RETURNING projection isn't mis-counted as a value tuple. let schema = schema_with("t", &[("a", Type::Int), ("b", Type::Int)]); - let diags = diag_keys("sqlinsert into t (a, b) values (1, 2, 3) returning *", &schema); + let diags = diag_keys("insert into t (a, b) values (1, 2, 3) returning *", &schema); let n = diags.iter().filter(|d| d.contains("value(s) are given")).count(); assert_eq!(n, 1, "only the 3-value tuple is flagged; got {diags:?}"); } @@ -4923,7 +4960,7 @@ mod tests { // A comma inside a function call (depth ≥ 2) is not a tuple // separator and must not inflate the value count. let schema = schema_with("t", &[("a", Type::Int), ("b", Type::Int)]); - let diags = diag_keys("sqlinsert into t (a, b) values (1, coalesce(1, 2))", &schema); + let diags = diag_keys("insert into t (a, b) values (1, coalesce(1, 2))", &schema); assert!( !diags.iter().any(|d| d.contains("value(s) are given")), "two values (one a 2-arg call) match the 2-col list; got {diags:?}", @@ -4934,7 +4971,7 @@ mod tests { fn insert_select_arity_mismatch_fires() { // INSERT … SELECT: 1 listed column vs a 2-item projection. let schema = two_table_schema(); // a(id,name), b(id,total) - let diags = diag_keys("sqlinsert into a (id) select id, total from b", &schema); + let diags = diag_keys("insert into a (id) select id, total from b", &schema); assert!( diags.iter().any(|d| d.contains("value(s) are given")), "2-col projection into 1-col list should fire; got {diags:?}", @@ -4944,7 +4981,7 @@ mod tests { #[test] fn insert_select_arity_match_is_silent() { let schema = two_table_schema(); - let diags = diag_keys("sqlinsert into a (id, name) select id, total from b", &schema); + let diags = diag_keys("insert into a (id, name) select id, total from b", &schema); assert!( !diags.iter().any(|d| d.contains("value(s) are given")), "matched projection arity must not fire; got {diags:?}", @@ -4955,7 +4992,7 @@ mod tests { fn auto_column_overridden_fires_on_explicit_serial() { // ADR-0033 §8.2: listing a serial column explicitly warns. let schema = schema_with("t", &[("id", Type::Serial), ("name", Type::Text)]); - let diags = diag_keys("sqlinsert into t (id, name) values (5, 'x')", &schema); + let diags = diag_keys("insert into t (id, name) values (5, 'x')", &schema); assert!( diags.iter().any(|d| d.contains("auto-generated")), "explicit serial value should warn; got {diags:?}", @@ -4965,7 +5002,7 @@ mod tests { #[test] fn auto_column_overridden_fires_on_explicit_shortid() { let schema = schema_with("t", &[("id", Type::ShortId), ("name", Type::Text)]); - let diags = diag_keys("sqlinsert into t (id, name) values ('abc', 'x')", &schema); + let diags = diag_keys("insert into t (id, name) values ('abc', 'x')", &schema); assert!( diags.iter().any(|d| d.contains("auto-generated")), "explicit shortid value should warn; got {diags:?}", @@ -4976,7 +5013,7 @@ mod tests { fn auto_column_overridden_silent_when_omitted() { // Negative: omitting the auto-gen column does not warn. let schema = schema_with("t", &[("id", Type::Serial), ("name", Type::Text)]); - let diags = diag_keys("sqlinsert into t (name) values ('x')", &schema); + let diags = diag_keys("insert into t (name) values ('x')", &schema); assert!( !diags.iter().any(|d| d.contains("auto-generated")), "omitting the serial column must not warn; got {diags:?}", @@ -4990,7 +5027,7 @@ mod tests { // only the inserted `id` in the column list would. let schema = schema_with("t", &[("id", Type::Serial), ("name", Type::Text)]); let diags = diag_keys( - "sqlinsert into t (name) values ('x') on conflict (id) do nothing", + "insert into t (name) values ('x') on conflict (id) do nothing", &schema, ); assert!( @@ -5005,7 +5042,7 @@ mod tests { // resolves to the target table's columns — no diagnostic. let schema = two_table_schema(); let diags = diag_keys( - "sqlinsert into a (id, name) values (1, 'x') on conflict (id) do update set name = excluded.name", + "insert into a (id, name) values (1, 'x') on conflict (id) do update set name = excluded.name", &schema, ); assert!(diags.is_empty(), "excluded.name should resolve in DO UPDATE; got {diags:?}"); @@ -5017,7 +5054,7 @@ mod tests { // `excluded` resolves there too (not just in the SET RHS). let schema = two_table_schema(); let diags = diag_keys( - "sqlinsert into a (id, name) values (1, 'x') on conflict (id) do update set name = 'y' where id = excluded.id", + "insert into a (id, name) values (1, 'x') on conflict (id) do update set name = 'y' where id = excluded.id", &schema, ); assert!(diags.is_empty(), "excluded in DO UPDATE WHERE should resolve; got {diags:?}"); @@ -5029,7 +5066,7 @@ mod tests { // under it is still an unknown_column error. let schema = two_table_schema(); let diags = diag_keys( - "sqlinsert into a (id, name) values (1, 'x') on conflict (id) do update set name = excluded.nosuch", + "insert into a (id, name) values (1, 'x') on conflict (id) do update set name = excluded.nosuch", &schema, ); assert!( @@ -5044,7 +5081,7 @@ mod tests { // in scope) has no meaning and must be flagged. let schema = two_table_schema(); let diags = diag_keys( - "sqlinsert into a (id, name) values (excluded.name, 'x')", + "insert into a (id, name) values (excluded.name, 'x')", &schema, ); assert!( @@ -5060,7 +5097,7 @@ mod tests { // resolves). The byte-range scoping must distinguish them. let schema = two_table_schema(); let diags = diag_keys( - "sqlinsert into a (id, name) values (excluded.id, 'x') on conflict (id) do update set name = excluded.name", + "insert into a (id, name) values (excluded.id, 'x') on conflict (id) do update set name = excluded.name", &schema, ); // Exactly the VALUES-side `excluded.id` is flagged; the @@ -5083,7 +5120,7 @@ mod tests { // `nonexistent_col` is not a column of `a`. let schema = two_table_schema(); let diags = diag_keys( - "sqlinsert into b select nonexistent_col from a", + "insert into b select nonexistent_col from a", &schema, ); assert!( @@ -5098,7 +5135,7 @@ mod tests { // ADR-0033 sub-phase 3e cross-cut: the schema-existence // pass fires on the SET assignment column. let schema = schema_with("t", &[("id", Type::Int), ("v", Type::Text)]); - let diags = diag_keys("sql_update t set nonexistent = 1 where id = 1", &schema); + let diags = diag_keys("update t set nonexistent = 1 where id = 1", &schema); assert!( diags.iter().any(|d| d.contains("no such column")), "expected unknown_column on the SET column; got {diags:?}", @@ -5110,7 +5147,7 @@ mod tests { // ADR-0033 sub-phase 3e cross-cut: the predicate-warning // pass fires on `= NULL` in an UPDATE's WHERE. let schema = schema_with("t", &[("id", Type::Int), ("v", Type::Int)]); - let diags = diag_keys("sql_update t set v = 1 where v = NULL", &schema); + let diags = diag_keys("update t set v = 1 where v = NULL", &schema); assert!( diags.iter().any(|d| d.contains("IS NULL")), "expected eq_null warning on the WHERE; got {diags:?}", @@ -5937,26 +5974,29 @@ mod dispatch_3a_tests { assert_eq!(cmd, Some(Command::App(AppCommand::Quit))); } - // ---- Exit-gate case 3: Simple + SQL-only input → - // ValidationFailed advanced_mode.sql_in_simple ---------- + // ---- Exit-gate case 3: Simple + SQL-shaped input on a SHARED + // word commits the DSL node (ADR-0033 Amendment 3) -------- #[test] - fn simple_mode_sql_only_input_is_this_is_sql() { - // Shared word, but the input matches only the SQL tail. + fn simple_mode_shared_word_commits_dsl_even_for_sql_input() { + // A shared word (DSL + SQL) in simple mode always commits its + // DSL candidate, even when the input matches only the SQL + // tail. The DSL grammar then rejects the SQL-only tail with a + // normal parse error (a `Mismatch`), surfacing the actionable + // DSL detail; the "(valid as SQL in advanced mode)" pointer is + // added at the rendering layer, not here. "This is SQL" as a + // dispatch outcome is reserved for entry words with no DSL + // form (see `simple_mode_sql_only_entry_word_is_this_is_sql`). let cands = shared(); - match run_decide("smk sqltail", Mode::Simple, &cands) { - Decision::ThisIsSql { primary } => assert_eq!(primary, "smk"), - Decision::Commit { idx, .. } => { - panic!("expected ThisIsSql, got Commit {{ idx: {idx} }}") - } - } + assert!( + std::ptr::eq(committed_node("smk sqltail", Mode::Simple, &cands), &SMOKE_DSL), + "simple mode must commit the DSL node for a shared word", + ); let (outcome, cmd) = dispatch("smk sqltail", Mode::Simple, &cands); - match outcome { - WalkOutcome::ValidationFailed { error, .. } => { - assert_eq!(error.message_key, "advanced_mode.sql_in_simple"); - } - other => panic!("expected ValidationFailed, got {other:?}"), - } + assert!( + matches!(outcome, WalkOutcome::Mismatch { .. }), + "the DSL grammar rejects the SQL-only tail; got {outcome:?}", + ); assert_eq!(cmd, None); } diff --git a/src/friendly/keys.rs b/src/friendly/keys.rs index feb8dbe..cd892a8 100644 --- a/src/friendly/keys.rs +++ b/src/friendly/keys.rs @@ -315,6 +315,7 @@ pub const KEYS_AND_PLACEHOLDERS: &[(&str, &[&str])] = &[ ("advanced_mode.not_implemented", &["input"]), // ---- Advanced-mode SQL surface (ADR-0030) ---- ("advanced_mode.sql_in_simple", &["command"]), + ("advanced_mode.also_valid_sql", &[]), ("select.internal_table", &["table"]), ( "cli.invalid_value", diff --git a/src/friendly/strings/en-US.yaml b/src/friendly/strings/en-US.yaml index 13cdda0..55256f5 100644 --- a/src/friendly/strings/en-US.yaml +++ b/src/friendly/strings/en-US.yaml @@ -634,6 +634,10 @@ persistence: advanced_mode: not_implemented: "advanced mode SQL not implemented yet — echo: {input}" sql_in_simple: "`{command}` is SQL — available in advanced mode. Switch with `mode advanced`, or prefix the line with `:` to run it once." + # Appended to a simple-mode DSL error when the same line would run + # in advanced mode — keeps the actionable DSL fix and adds the mode + # pointer (ADR-0033 Amendment 3). + also_valid_sql: "(valid as SQL in advanced mode — `mode advanced` or prefix `:`)" # ---- SQL SELECT (advanced mode; ADR-0030 / ADR-0031) ---------------- select: diff --git a/src/input_render.rs b/src/input_render.rs index 8123959..ed95d4b 100644 --- a/src/input_render.rs +++ b/src/input_render.rs @@ -90,7 +90,9 @@ pub fn render_input_runs_in_mode( { overlay_error(&mut runs, pos, theme); } - if let Some(inv) = crate::completion::invalid_ident_at_cursor(input, cursor_byte, cache) { + if let Some(inv) = + crate::completion::invalid_ident_at_cursor_in_mode(input, cursor_byte, cache, mode) + { overlay_error(&mut runs, inv.range.0, theme); } // Schema-aware diagnostics (ADR-0027 §2): unknown table / @@ -167,6 +169,27 @@ pub fn classify_input_with_schema( classify_parse_result(parse_command_with_schema(input, cache)) } +/// Mode-aware [`classify_input_with_schema`]. +/// +/// Walks the input in `mode` so the simple-mode DSL surface is +/// classified against the DSL grammar rather than the advanced SQL +/// grammar — relevant for the shared `insert`/`update`/`delete` entry +/// words (ADR-0033 Amendment 3). The mode-less entry point keeps its +/// advanced-mode behaviour. +#[must_use] +pub fn classify_input_with_schema_in_mode( + input: &str, + cache: &crate::completion::SchemaCache, + mode: Mode, +) -> InputState { + if input.trim().is_empty() { + return InputState::Empty; + } + classify_parse_result(crate::dsl::parser::parse_command_with_schema_in_mode( + input, cache, mode, + )) +} + fn classify_parse_result( result: Result, ) -> InputState { @@ -239,6 +262,13 @@ pub fn ambient_hint( /// "this is SQL" gate. The simple-mode entry point [`ambient_hint`] /// forwards here with `Mode::Simple`. /// +/// In simple mode, when the line is a *definite* DSL error but the +/// same line would parse in advanced mode, the DSL error prose is +/// suffixed with the `advanced_mode.also_valid_sql` pointer — so the +/// user keeps the actionable DSL fix *and* learns it would run as SQL +/// in advanced mode (ADR-0033 Amendment 3). Mid-typing (incomplete) +/// input is not suffixed, to avoid noise during normal DSL entry. +/// /// Returns `None` for empty input — caller falls back to /// `panel.hint_empty`. #[must_use] @@ -248,6 +278,58 @@ pub fn ambient_hint_in_mode( memo: Option<&crate::completion::LastCompletion>, cache: &crate::completion::SchemaCache, mode: Mode, +) -> Option { + let core = ambient_hint_core_in_mode(input, cursor, memo, cache, mode); + // Combine: a simple-mode *definite* DSL error that would run in + // advanced mode keeps its DSL prose and gains the mode pointer. + // Skip the "command complete" prose — appending the pointer to a + // "submit this" hint would be contradictory (and that prose can + // come from the hint's schemaless fallback even when the + // schema-aware classify is a definite error — a pre-existing + // quirk this combine step deliberately does not amplify). + if mode == Mode::Simple + && let Some(AmbientHint::Prose(message)) = &core + && *message != crate::t!("hint.ambient_complete") + && let Some(suffix) = advanced_alternative_note(input, cache) + { + return Some(AmbientHint::Prose(format!("{message} {suffix}"))); + } + core +} + +/// The `advanced_mode.also_valid_sql` pointer string, or `None`. +/// +/// Returns the pointer when a simple-mode line is a *definite* DSL +/// error (not merely incomplete) yet parses in advanced mode. Used to +/// combine the DSL fix with the mode hint — both while typing +/// (`ambient_hint_in_mode`) and on submit (`App::dispatch_dsl`) — so +/// the pointer reaches SQL constructs that surface only on submit, +/// e.g. `delete … returning` (ADR-0033 Amendment 3). +#[must_use] +pub fn advanced_alternative_note( + input: &str, + cache: &crate::completion::SchemaCache, +) -> Option { + let definite_dsl_error = matches!( + classify_input_with_schema_in_mode(input, cache, Mode::Simple), + InputState::DefiniteErrorAt(_) + ); + if !definite_dsl_error { + return None; + } + if parse_command_with_schema_in_mode(input, cache, Mode::Advanced).is_ok() { + Some(crate::t!("advanced_mode.also_valid_sql")) + } else { + None + } +} + +fn ambient_hint_core_in_mode( + input: &str, + cursor: usize, + memo: Option<&crate::completion::LastCompletion>, + cache: &crate::completion::SchemaCache, + mode: Mode, ) -> Option { if input.trim().is_empty() { return None; @@ -401,7 +483,8 @@ pub fn ambient_hint_in_mode( // Invalid identifier: cursor sits in a known-set slot but // the typed prefix matches nothing in the schema. (Stage // 8e / the user's #5.) - if let Some(inv) = crate::completion::invalid_ident_at_cursor(input, cursor, cache) { + if let Some(inv) = crate::completion::invalid_ident_at_cursor_in_mode(input, cursor, cache, mode) + { let kind = match inv.source { crate::dsl::grammar::IdentSource::Tables => "table", crate::dsl::grammar::IdentSource::Columns => "column", @@ -989,6 +1072,53 @@ mod tests { } } + #[test] + fn ambient_hint_combines_dsl_error_with_advanced_sql_pointer() { + // ADR-0033 Amendment 3: in simple mode, a *definite* DSL + // error whose line would run as SQL in advanced mode keeps + // its actionable DSL prose AND gains the + // `advanced_mode.also_valid_sql` pointer. `Customers(id + // serial, Name, Email)`: DSL Form B auto-skips the serial + // `id`, so three values is a definite DSL error — but the same + // line is a valid SQL insert (all three columns). + use crate::dsl::types::Type; + let cache = schema_with_columns( + "Customers", + &[("id", Type::Serial), ("Name", Type::Text), ("Email", Type::Text)], + ); + let input = "insert into Customers values (1, 'Alice', 'a@b.c')"; + match ambient_hint(input, input.len(), None, &cache) { + Some(AmbientHint::Prose(p)) => { + // The DSL detail survives … + assert!(p.contains("Name"), "expected DSL slot detail, got: {p:?}"); + // … and the advanced-mode pointer is appended. + assert!( + p.contains("advanced mode"), + "expected the advanced-mode pointer, got: {p:?}", + ); + } + other => panic!("expected Prose, got {other:?}"), + } + } + + #[test] + fn ambient_hint_does_not_add_pointer_for_a_valid_dsl_command() { + // A valid simple-mode DSL command gets no advanced pointer — + // it isn't an error, and there is nothing to switch modes for. + use crate::dsl::types::Type; + let cache = schema_with_columns( + "Customers", + &[("id", Type::Serial), ("Name", Type::Text)], + ); + let input = "insert into Customers values ('Alice')"; + if let Some(AmbientHint::Prose(p)) = ambient_hint(input, input.len(), None, &cache) { + assert!( + !p.contains("advanced mode"), + "a valid DSL command must not carry the advanced pointer, got: {p:?}", + ); + } + } + #[test] fn ambient_hint_at_insert_second_value_shows_text_prose() { use crate::dsl::types::Type; diff --git a/src/runtime.rs b/src/runtime.rs index b2246b1..53c22b9 100644 --- a/src/runtime.rs +++ b/src/runtime.rs @@ -1644,6 +1644,13 @@ pub async fn run_replay( // (bad syntax / typed-slot reject) — report and stop // without dispatching. let schema = build_schema_cache(database).await; + // Replay parses each line like an interactive submission and + // executes the resulting command — in advanced mode (the full + // surface). A bad value in a shared-entry-word DML line is + // caught either at parse time (a DSL form's typed slot) or at + // execute time (the engine's column-type enforcement); either + // way the offending line fails and replay stops without + // applying it. let command = match crate::dsl::parser::parse_command_with_schema( trimmed, &schema, ) { diff --git a/tests/replay_command.rs b/tests/replay_command.rs index e621974..f495c47 100644 --- a/tests/replay_command.rs +++ b/tests/replay_command.rs @@ -228,21 +228,31 @@ fn replay_aborts_on_first_parse_failure_and_reports_line() { } #[test] -fn replay_rejects_typed_slot_violation_at_parse_time() { - // Schema-aware replay (handoff-13 §2.1 fix): run_replay - // re-snapshots the schema per line and parses with - // parse_command_with_schema. So a wrong-type value in a - // value list is caught at *parse* time during replay — - // surfaced through the `replay.error_parse` wrapper ("parse - // error …") — exactly as the interactive path would, rather - // than only at bind time. +fn replay_rejects_wrong_type_value_in_a_hand_built_script() { + // Replay parses each line with the SAME schema-aware parser the + // interactive path uses, in **advanced mode** (the full surface), + // and executes the result — so a replayed line behaves exactly as + // if it had been typed interactively in advanced mode. Nothing is + // skipped or simplified during replay (handoff-13 §2.1: the schema + // is threaded so the parser is fully schema-aware). // - // `'not a number'` (a string) lands in the int `count` - // slot. The schemaless parser would accept it (a string is - // a value literal) and only bind-time would reject; the - // schema-aware parser rejects it at parse time. Asserting - // the error went through the parse wrapper proves the - // schema was threaded. + // A real journal only ever contains commands that already executed + // successfully (history.log is success-only; ADR-0034's deferred + // journal replays `ok` lines only), so a wrong-type line like this + // never arises from a genuine replay. It only arises from a + // *hand-built* `.commands` script — the robustness case this test + // exercises: replay must reject the bad line and stop, leaving + // state intact, with the same error a user would see typing it. + // + // Where the rejection lands depends on the grammar the line + // matches, exactly as interactively: `insert into T values (…)` is + // SQL in advanced mode, and SQL defers column-type checking to the + // engine, so `'not a number'` in the int `count` column is rejected + // at **execute** time (the engine's column-type enforcement) rather + // than at parse time. Either way the line fails and is not applied. + // (Before sub-phase 3j, `insert` was a DSL-only entry word, so even + // advanced-mode parsing hit the DSL typed-slot rail and this was a + // parse-time rejection — ADR-0033 Amendment 3.) let data = tempdir(); let (project, db) = open_project_db(data.path()); write_script( @@ -261,12 +271,12 @@ fn replay_rejects_typed_slot_violation_at_parse_time() { unreachable!() }; assert!( - error.contains("parse error"), - "typed-slot violation should be caught at parse time, got: {error}", + !error.is_empty(), + "the rejected line must carry a reported error", ); // The earlier two lines stayed applied; the failing insert - // did not run. + // did not run — state is intact. let data_result = rt() .block_on(async { db.query_data("T".to_string(), None, None, None).await }) .expect("query_data"); diff --git a/tests/sql_delete.rs b/tests/sql_delete.rs index 3bcf173..ab64307 100644 --- a/tests/sql_delete.rs +++ b/tests/sql_delete.rs @@ -82,7 +82,7 @@ fn run_delete( rt: &tokio::runtime::Runtime, input: &str, ) -> Result { - match parse_command(input).expect("parse sql_delete") { + match parse_command(input).expect("parse delete") { Command::SqlDelete { sql, target_table, returning } => { rt.block_on(db.run_sql_delete(sql, Some(input.to_string()), target_table, returning)) } @@ -115,8 +115,8 @@ fn cascade_fixture(db: &Database, rt: &tokio::runtime::Runtime) { #[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"); + let command = parse_command("delete from Orders where id = 1") + .expect("delete parses in advanced mode"); match command { Command::SqlDelete { sql, target_table, .. } => { assert_eq!(sql, "delete from Orders where id = 1"); @@ -132,7 +132,7 @@ fn delete_with_where_persists() { 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"); + let result = run_delete(&db, &rt, "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"); @@ -147,7 +147,7 @@ fn delete_without_where_runs_across_all_rows() { 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"); + let result = run_delete(&db, &rt, "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. @@ -165,7 +165,7 @@ fn cascade_delete_reports_summary_and_repersists_child() { 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") + let result = run_delete(&db, &rt, "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"); @@ -193,7 +193,7 @@ fn cascade_parity_with_dsl() { 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") + let sql_result = run_delete(&db_sql, &rt, "delete from Customers where id = 1") .expect("SQL delete runs"); let (_p_dsl, db_dsl, _d_dsl) = open_project_db(); @@ -223,7 +223,7 @@ fn r2_where_with_subquery() { let result = run_delete( &db, &rt, - "sql_delete from Orders where CustId in (select id from Customers where Name = 'Alice')", + "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"); @@ -250,7 +250,7 @@ fn r2_cascade_with_subquery_where() { let result = run_delete( &db, &rt, - "sql_delete from Customers where id in (select CustId from Orders where id = 11)", + "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"); @@ -267,7 +267,7 @@ fn delete_appends_literal_line_to_history() { 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"; + let input = "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"); @@ -303,7 +303,7 @@ fn cascade_to_two_children_reports_both() { 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") + let result = run_delete(&db, &rt, "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"); @@ -332,7 +332,7 @@ fn delete_childless_parent_reports_no_cascade() { 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") + let result = run_delete(&db, &rt, "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"); @@ -370,7 +370,7 @@ fn delete_violating_fk_fails_and_persists_nothing() { 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 input = "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. @@ -406,7 +406,7 @@ fn self_referential_cascade_counts_only_cascaded_rows() { .expect("add self-referential relationship"); seed(&db, &rt, "insert into T (id, ParentId) values (1, null), (2, 1), (3, 2)", "T"); let result = - run_delete(&db, &rt, "sql_delete from T where id = 1").expect("self-ref delete runs"); + run_delete(&db, &rt, "delete from T where id = 1").expect("self-ref delete runs"); assert_eq!(result.rows_affected, 1, "one row matched the WHERE directly"); assert_eq!(result.cascade.len(), 1, "self-ref relationship reported once"); assert_eq!( @@ -421,7 +421,7 @@ fn internal_target_table_rejected_at_parse() { // 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(), + parse_command("delete from __rdbms_playground_columns").is_err(), "internal table must be rejected at the DELETE target slot" ); } @@ -436,7 +436,7 @@ fn delete_returning_yields_predelete_row() { 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 returning *") + let result = run_delete(&db, &rt, "delete from t where id = 1 returning *") .expect("DELETE … RETURNING * runs"); assert_eq!(result.rows_affected, 1, "one row deleted"); // RETURNING yields the row as it was BEFORE deletion. @@ -454,7 +454,7 @@ fn delete_returning_with_cascade_surfaces_both() { let (_project, db, _dir) = open_project_db(); let rt = rt(); cascade_fixture(&db, &rt); - let result = run_delete(&db, &rt, "sql_delete from Customers where id = 1 returning *") + let result = run_delete(&db, &rt, "delete from Customers where id = 1 returning *") .expect("cascading DELETE … RETURNING runs"); assert_eq!(result.rows_affected, 1, "one parent row deleted"); // RETURNING gave the deleted customer row. diff --git a/tests/sql_insert.rs b/tests/sql_insert.rs index 10077ba..817d2d9 100644 --- a/tests/sql_insert.rs +++ b/tests/sql_insert.rs @@ -222,8 +222,8 @@ fn failed_multi_row_insert_is_atomic() { fn parse_path_lowers_sqlinsert_scaffold_to_command() { // Advanced-mode parse of the dev scaffold reconstructs valid // `insert …` SQL and extracts the target table. - let command = parse_command("sqlinsert into Orders (id, total) values (1, 99.5)") - .expect("sqlinsert parses in advanced mode"); + let command = parse_command("insert into Orders (id, total) values (1, 99.5)") + .expect("insert parses in advanced mode"); match command { Command::SqlInsert { sql, target_table, .. } => { assert_eq!(sql, "insert into Orders (id, total) values (1, 99.5)"); @@ -235,7 +235,7 @@ fn parse_path_lowers_sqlinsert_scaffold_to_command() { #[test] fn parse_path_rejects_internal_target_table() { - let result = parse_command("sqlinsert into __rdbms_playground_columns values (1)"); + let result = parse_command("insert into __rdbms_playground_columns values (1)"); assert!( result.is_err(), "an internal `__rdbms_*` target must be rejected: {result:?}", @@ -262,7 +262,7 @@ fn create_named(db: &Database, rt: &tokio::runtime::Runtime, name: &str) { #[test] fn parse_path_lowers_insert_select_to_command() { - let command = parse_command("sqlinsert into archive select * from source") + let command = parse_command("insert into archive select * from source") .expect("INSERT … SELECT parses in advanced mode"); match command { Command::SqlInsert { sql, target_table, .. } => { @@ -277,7 +277,7 @@ fn parse_path_lowers_insert_select_to_command() { fn parse_path_lowers_with_prefixed_insert_select() { // R4: a WITH-prefixed SELECT row source lowers verbatim. let command = parse_command( - "sqlinsert into archive with t as (select * from orders) select * from t", + "insert into archive with t as (select * from orders) select * from t", ) .expect("WITH-prefixed INSERT … SELECT parses"); match command { @@ -436,7 +436,7 @@ fn run_sqlinsert( rt: &tokio::runtime::Runtime, input: &str, ) -> Result { - match parse_command(input).expect("parse sqlinsert") { + match parse_command(input).expect("parse insert") { Command::SqlInsert { sql, target_table, @@ -489,7 +489,7 @@ fn values_autofills_omitted_shortid_pk() { let (project, db, _dir) = open_project_db(); let rt = rt(); create_cols(&db, &rt, "t", &[("id", Type::ShortId), ("label", Type::Text)], &["id"]); - let result = run_sqlinsert(&db, &rt, "sqlinsert into t (label) values ('x')") + let result = run_sqlinsert(&db, &rt, "insert into t (label) values ('x')") .expect("auto-fill insert runs"); assert_eq!(result.rows_affected, 1); let rows = csv_rows(&project, "t"); @@ -506,7 +506,7 @@ fn values_multirow_autofills_distinct_shortids() { let result = run_sqlinsert( &db, &rt, - "sqlinsert into t (label) values ('a'), ('b'), ('c')", + "insert into t (label) values ('a'), ('b'), ('c')", ) .expect("multi-row auto-fill runs"); assert_eq!(result.rows_affected, 3); @@ -526,7 +526,7 @@ fn explicit_shortid_value_is_respected() { run_sqlinsert( &db, &rt, - "sqlinsert into t (id, label) values ('hardcoded', 'x')", + "insert into t (id, label) values ('hardcoded', 'x')", ) .expect("explicit-id insert runs"); let rows = csv_rows(&project, "t"); @@ -539,12 +539,12 @@ fn insert_select_autofills_distinct_shortids() { let rt = rt(); create_cols(&db, &rt, "source", &[("id", Type::ShortId), ("label", Type::Text)], &["id"]); create_cols(&db, &rt, "target", &[("id", Type::ShortId), ("label", Type::Text)], &["id"]); - run_sqlinsert(&db, &rt, "sqlinsert into source (label) values ('a'), ('b')") + run_sqlinsert(&db, &rt, "insert into source (label) values ('a'), ('b')") .expect("seed source"); let result = run_sqlinsert( &db, &rt, - "sqlinsert into target (label) select label from source", + "insert into target (label) select label from source", ) .expect("INSERT … SELECT auto-fill runs"); assert_eq!(result.rows_affected, 2); @@ -566,7 +566,7 @@ fn combined_serial_and_shortid_autofill() { &[("id", Type::Serial), ("code", Type::ShortId), ("name", Type::Text)], &["id"], ); - run_sqlinsert(&db, &rt, "sqlinsert into t (name) values ('x')") + run_sqlinsert(&db, &rt, "insert into t (name) values ('x')") .expect("combined auto-fill runs"); let rows = csv_rows(&project, "t"); assert_eq!(rows.len(), 1, "{rows:?}"); @@ -583,7 +583,7 @@ fn autofill_logs_original_source_not_rewritten_sql() { let (project, db, _dir) = open_project_db(); let rt = rt(); create_cols(&db, &rt, "t", &[("id", Type::ShortId), ("label", Type::Text)], &["id"]); - let input = "sqlinsert into t (label) values ('x')"; + let input = "insert into t (label) values ('x')"; run_sqlinsert(&db, &rt, input).expect("auto-fill insert runs"); let body = std::fs::read_to_string(project.path().join("history.log")) .expect("history.log present"); @@ -603,7 +603,7 @@ fn shortid_autofill_respects_mixed_case_column_name() { let (project, db, _dir) = open_project_db(); let rt = rt(); create_cols(&db, &rt, "t", &[("MyId", Type::ShortId), ("label", Type::Text)], &["MyId"]); - run_sqlinsert(&db, &rt, "sqlinsert into t (label) values ('x')") + run_sqlinsert(&db, &rt, "insert into t (label) values ('x')") .expect("mixed-case shortid auto-fill runs"); let rows = csv_rows(&project, "t"); assert_eq!(rows.len(), 1, "{rows:?}"); @@ -623,7 +623,7 @@ fn two_shortids_pk_and_nonpk_both_autofill_distinctly() { &[("id", Type::ShortId), ("code", Type::ShortId), ("label", Type::Text)], &["id"], ); - let result = run_sqlinsert(&db, &rt, "sqlinsert into t (label) values ('x'), ('y')") + let result = run_sqlinsert(&db, &rt, "insert into t (label) values ('x'), ('y')") .expect("two-shortid auto-fill runs"); assert_eq!(result.rows_affected, 2); let rows = csv_rows(&project, "t"); @@ -650,7 +650,7 @@ fn two_shortids_one_provided_one_autofilled() { &[("id", Type::ShortId), ("code", Type::ShortId), ("label", Type::Text)], &["id"], ); - run_sqlinsert(&db, &rt, "sqlinsert into t (id, label) values ('myid', 'x')") + run_sqlinsert(&db, &rt, "insert into t (id, label) values ('myid', 'x')") .expect("partial-shortid insert runs"); let rows = csv_rows(&project, "t"); assert_eq!(rows[0][0], "myid", "provided id preserved: {rows:?}"); @@ -670,7 +670,7 @@ fn compound_pk_with_shortid_member_autofills() { &[("id", Type::ShortId), ("region", Type::Int), ("label", Type::Text)], &["id", "region"], ); - run_sqlinsert(&db, &rt, "sqlinsert into t (region, label) values (1, 'x')") + run_sqlinsert(&db, &rt, "insert into t (region, label) values (1, 'x')") .expect("compound-pk insert runs"); let rows = csv_rows(&project, "t"); assert!( @@ -690,7 +690,7 @@ fn autofill_does_not_mask_arity_mismatch() { let (project, db, _dir) = open_project_db(); let rt = rt(); create_cols(&db, &rt, "t", &[("id", Type::ShortId), ("label", Type::Text)], &["id"]); - let outcome = run_sqlinsert(&db, &rt, "sqlinsert into t (label) values ('a', 'b')"); + let outcome = run_sqlinsert(&db, &rt, "insert into t (label) values ('a', 'b')"); assert!( outcome.is_err(), "arity mismatch must be rejected, not masked: {outcome:?}", @@ -707,8 +707,8 @@ fn autofill_insert_select_wider_projection_is_rejected() { let rt = rt(); create_cols(&db, &rt, "src", &[("a", Type::Text), ("b", Type::Text)], &["a"]); create_cols(&db, &rt, "t", &[("id", Type::ShortId), ("label", Type::Text)], &["id"]); - run_sqlinsert(&db, &rt, "sqlinsert into src (a, b) values ('p', 'q')").expect("seed"); - let outcome = run_sqlinsert(&db, &rt, "sqlinsert into t (label) select a, b from src"); + run_sqlinsert(&db, &rt, "insert into src (a, b) values ('p', 'q')").expect("seed"); + let outcome = run_sqlinsert(&db, &rt, "insert into t (label) select a, b from src"); assert!(outcome.is_err(), "wider projection must be rejected: {outcome:?}"); assert!(csv_rows(&project, "t").is_empty(), "nothing should land"); } @@ -727,8 +727,8 @@ fn autofill_insert_select_narrower_projection_is_rejected() { &[("id", Type::ShortId), ("x", Type::Text), ("y", Type::Text)], &["id"], ); - run_sqlinsert(&db, &rt, "sqlinsert into src (a) values ('p')").expect("seed"); - let outcome = run_sqlinsert(&db, &rt, "sqlinsert into t (x, y) select a from src"); + run_sqlinsert(&db, &rt, "insert into src (a) values ('p')").expect("seed"); + let outcome = run_sqlinsert(&db, &rt, "insert into t (x, y) select a from src"); assert!(outcome.is_err(), "narrower projection must be rejected: {outcome:?}"); assert!(csv_rows(&project, "t").is_empty(), "nothing should land"); } @@ -742,7 +742,7 @@ fn insert_returning_star_returns_inserted_row() { let (_project, db, _dir) = open_project_db(); let rt = rt(); create_cols(&db, &rt, "t", &[("id", Type::Int), ("b", Type::Text)], &["id"]); - let result = run_sqlinsert(&db, &rt, "sqlinsert into t (id, b) values (1, 'Ada') returning *") + let result = run_sqlinsert(&db, &rt, "insert into t (id, b) values (1, 'Ada') returning *") .expect("INSERT … RETURNING * runs"); assert_eq!(result.rows_affected, 1, "one row inserted"); assert_eq!(result.data.rows.len(), 1, "RETURNING yielded the inserted row"); @@ -758,7 +758,7 @@ fn insert_multirow_returning_id_yields_distinct_rows() { let result = run_sqlinsert( &db, &rt, - "sqlinsert into t (id, b) values (1, 'a'), (2, 'b'), (3, 'c') returning id", + "insert into t (id, b) values (1, 'a'), (2, 'b'), (3, 'c') returning id", ) .expect("multi-row INSERT … RETURNING id runs"); assert_eq!(result.rows_affected, 3, "three rows inserted"); @@ -777,7 +777,7 @@ fn insert_returning_autofills_shortid_and_returns_it() { let (_project, db, _dir) = open_project_db(); let rt = rt(); create_cols(&db, &rt, "t", &[("id", Type::ShortId), ("label", Type::Text)], &["id"]); - let result = run_sqlinsert(&db, &rt, "sqlinsert into t (label) values ('x') returning *") + let result = run_sqlinsert(&db, &rt, "insert into t (label) values ('x') returning *") .expect("auto-fill INSERT … RETURNING * runs"); assert_eq!(result.rows_affected, 1, "one row inserted (RETURNING-counted)"); assert_eq!(result.data.rows.len(), 1, "RETURNING yielded the row"); @@ -796,7 +796,7 @@ fn insert_returning_recovers_bare_column_type() { let (_project, db, _dir) = open_project_db(); let rt = rt(); create_cols(&db, &rt, "t", &[("id", Type::Int), ("active", Type::Bool)], &["id"]); - let result = run_sqlinsert(&db, &rt, "sqlinsert into t (id, active) values (1, true) returning active") + let result = run_sqlinsert(&db, &rt, "insert into t (id, active) values (1, true) returning active") .expect("INSERT … RETURNING active runs"); assert_eq!(result.data.column_types, vec![Some(Type::Bool)], "bool type recovered"); assert_eq!(result.data.rows[0][0], Some("true".to_string()), "rendered as the bool word"); @@ -809,7 +809,7 @@ fn insert_returning_computed_expression_is_typeless() { let (_project, db, _dir) = open_project_db(); let rt = rt(); create_cols(&db, &rt, "t", &[("id", Type::Int), ("n", Type::Int)], &["id"]); - let result = run_sqlinsert(&db, &rt, "sqlinsert into t (id, n) values (1, 5) returning n + 1") + let result = run_sqlinsert(&db, &rt, "insert into t (id, n) values (1, 5) returning n + 1") .expect("INSERT … RETURNING runs"); assert_eq!(result.data.column_types, vec![None], "computed projection is typeless"); assert_eq!(result.data.rows[0][0], Some("6".to_string()), "engine evaluated n + 1"); @@ -839,7 +839,7 @@ fn insert_returning_recovers_multiple_bare_column_types() { let result = run_sqlinsert( &db, &rt, - "sqlinsert into t (id, txt, amount, ratio, flag) values (1, 'a', 9.50, 1.5, true) returning id, txt, amount, ratio, flag", + "insert into t (id, txt, amount, ratio, flag) values (1, 'a', 9.50, 1.5, true) returning id, txt, amount, ratio, flag", ) .expect("INSERT … RETURNING runs"); assert_eq!( @@ -867,7 +867,7 @@ fn multirow_autofill_returning_yields_distinct_generated_ids() { let result = run_sqlinsert( &db, &rt, - "sqlinsert into t (label) values ('a'), ('b'), ('c') returning *", + "insert into t (label) values ('a'), ('b'), ('c') returning *", ) .expect("multi-row auto-fill INSERT … RETURNING * runs"); assert_eq!(result.rows_affected, 3, "three rows inserted"); @@ -888,8 +888,8 @@ fn insert_select_returning_executes_and_returns_rows() { let rt = rt(); create_cols(&db, &rt, "src", &[("id", Type::Int), ("b", Type::Text)], &["id"]); create_cols(&db, &rt, "dst", &[("id", Type::Int), ("b", Type::Text)], &["id"]); - run_sqlinsert(&db, &rt, "sqlinsert into src (id, b) values (1, 'x'), (2, 'y')").expect("seed src"); - let result = run_sqlinsert(&db, &rt, "sqlinsert into dst select * from src returning id, b") + run_sqlinsert(&db, &rt, "insert into src (id, b) values (1, 'x'), (2, 'y')").expect("seed src"); + let result = run_sqlinsert(&db, &rt, "insert into dst select * from src returning id, b") .expect("INSERT … SELECT … RETURNING runs"); assert_eq!(result.rows_affected, 2, "two rows copied"); assert_eq!(result.data.rows.len(), 2, "RETURNING yielded both inserted rows"); @@ -909,7 +909,7 @@ fn conflict_target_columns_excluded_from_listed_columns() { // listed_columns (which drives shortid auto-fill) must NOT pick // up the conflict-target columns. If it did, an omitted shortid // would look "listed" and auto-fill would wrongly skip. - match parse_command("sqlinsert into t (name) values ('x') on conflict (id) do nothing") + match parse_command("insert into t (name) values ('x') on conflict (id) do nothing") .expect("parse upsert") { Command::SqlInsert { listed_columns, .. } => { @@ -946,11 +946,11 @@ fn autofill_upsert_real_conflict_preserves_clause_and_excluded() { None, )) .expect("create table with shortid pk + unique code"); - run_sqlinsert(&db, &rt, "sqlinsert into t (code, label) values ('A', 'first')").expect("seed"); + run_sqlinsert(&db, &rt, "insert into t (code, label) values ('A', 'first')").expect("seed"); let result = run_sqlinsert( &db, &rt, - "sqlinsert into t (code, label) values ('A', 'second') on conflict (code) do update set label = excluded.label", + "insert into t (code, label) values ('A', 'second') on conflict (code) do update set label = excluded.label", ) .expect("auto-filled UPSERT with a real conflict (clause preserved)"); assert_eq!(result.rows_affected, 1, "the conflicting row was updated, not inserted"); @@ -965,11 +965,11 @@ fn on_conflict_do_nothing_keeps_existing_row() { let (project, db, _dir) = open_project_db(); let rt = rt(); create_cols(&db, &rt, "t", &[("id", Type::Int), ("name", Type::Text)], &["id"]); - run_sqlinsert(&db, &rt, "sqlinsert into t (id, name) values (1, 'orig')").expect("seed"); + run_sqlinsert(&db, &rt, "insert into t (id, name) values (1, 'orig')").expect("seed"); let result = run_sqlinsert( &db, &rt, - "sqlinsert into t (id, name) values (1, 'new') on conflict (id) do nothing", + "insert into t (id, name) values (1, 'new') on conflict (id) do nothing", ) .expect("ON CONFLICT DO NOTHING runs"); assert_eq!(result.rows_affected, 0, "conflicting row left untouched"); @@ -983,11 +983,11 @@ fn on_conflict_do_update_applies_excluded() { let (project, db, _dir) = open_project_db(); let rt = rt(); create_cols(&db, &rt, "t", &[("id", Type::Int), ("name", Type::Text)], &["id"]); - run_sqlinsert(&db, &rt, "sqlinsert into t (id, name) values (1, 'orig')").expect("seed"); + run_sqlinsert(&db, &rt, "insert into t (id, name) values (1, 'orig')").expect("seed"); let result = run_sqlinsert( &db, &rt, - "sqlinsert into t (id, name) values (1, 'new') on conflict (id) do update set name = excluded.name", + "insert into t (id, name) values (1, 'new') on conflict (id) do update set name = excluded.name", ) .expect("ON CONFLICT DO UPDATE runs"); assert_eq!(result.rows_affected, 1, "the conflicting row was updated"); @@ -1001,11 +1001,11 @@ fn on_conflict_do_nothing_without_target() { let (_project, db, _dir) = open_project_db(); let rt = rt(); create_cols(&db, &rt, "t", &[("id", Type::Int), ("name", Type::Text)], &["id"]); - run_sqlinsert(&db, &rt, "sqlinsert into t (id, name) values (1, 'orig')").expect("seed"); + run_sqlinsert(&db, &rt, "insert into t (id, name) values (1, 'orig')").expect("seed"); let result = run_sqlinsert( &db, &rt, - "sqlinsert into t (id, name) values (1, 'x') on conflict do nothing", + "insert into t (id, name) values (1, 'x') on conflict do nothing", ) .expect("ON CONFLICT (no target) DO NOTHING runs"); assert_eq!(result.rows_affected, 0, "any-conflict do-nothing absorbed the duplicate"); @@ -1026,7 +1026,7 @@ fn autofill_preserves_on_conflict_clause() { let result = run_sqlinsert( &db, &rt, - "sqlinsert into t (label) values ('x') on conflict (id) do nothing", + "insert into t (label) values ('x') on conflict (id) do nothing", ) .expect("auto-fill INSERT with ON CONFLICT runs (clause preserved)"); assert_eq!(result.rows_affected, 1, "row inserted with a generated id"); diff --git a/tests/sql_update.rs b/tests/sql_update.rs index 75866f7..3d56cc0 100644 --- a/tests/sql_update.rs +++ b/tests/sql_update.rs @@ -69,7 +69,7 @@ fn run_update( rt: &tokio::runtime::Runtime, input: &str, ) -> Result { - match parse_command(input).expect("parse sql_update") { + match parse_command(input).expect("parse update") { Command::SqlUpdate { sql, target_table, returning } => { rt.block_on(db.run_sql_update(sql, Some(input.to_string()), target_table, returning)) } @@ -79,8 +79,8 @@ fn run_update( #[test] fn parse_path_lowers_sql_update_to_command() { - let command = parse_command("sql_update Orders set total = 0 where id = 1") - .expect("sql_update parses in advanced mode"); + let command = parse_command("update Orders set total = 0 where id = 1") + .expect("update parses in advanced mode"); match command { Command::SqlUpdate { sql, target_table, .. } => { assert_eq!(sql, "update Orders set total = 0 where id = 1"); @@ -96,7 +96,7 @@ fn single_column_update_with_where_persists() { 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, 'old'), (2, 'keep')", "t"); - let result = run_update(&db, &rt, "sql_update t set v = 'new' where id = 1") + let result = run_update(&db, &rt, "update t set v = 'new' where id = 1") .expect("update runs"); assert_eq!(result.rows_affected, 1, "one row updated"); let csv = read_csv(&project, "t").expect("t.csv"); @@ -117,7 +117,7 @@ fn multi_column_update_persists() { &["id"], ); seed(&db, &rt, "insert into t (id, a, b) values (1, 0, 'x')", "t"); - let result = run_update(&db, &rt, "sql_update t set a = 9, b = 'y' where id = 1") + let result = run_update(&db, &rt, "update t set a = 9, b = 'y' where id = 1") .expect("multi-col update runs"); assert_eq!(result.rows_affected, 1); let csv = read_csv(&project, "t").expect("t.csv"); @@ -131,7 +131,7 @@ fn update_without_where_runs_across_all_rows() { let rt = rt(); create_cols(&db, &rt, "t", &[("id", Type::Int), ("active", Type::Bool)], &["id"]); seed(&db, &rt, "insert into t (id, active) values (1, true), (2, true)", "t"); - let result = run_update(&db, &rt, "sql_update t set active = false") + let result = run_update(&db, &rt, "update t set active = false") .expect("unfiltered update runs"); assert_eq!(result.rows_affected, 2, "all rows updated"); let csv = read_csv(&project, "t").expect("t.csv"); @@ -150,7 +150,7 @@ fn update_with_sql_expr_in_set() { &["id"], ); seed(&db, &rt, "insert into t (id, price, qty, total) values (1, 6, 7, 0)", "t"); - let result = run_update(&db, &rt, "sql_update t set total = price * qty where id = 1") + let result = run_update(&db, &rt, "update t set total = price * qty where id = 1") .expect("expression update runs"); assert_eq!(result.rows_affected, 1); let csv = read_csv(&project, "t").expect("t.csv"); @@ -169,7 +169,7 @@ fn update_with_subquery_in_set() { let result = run_update( &db, &rt, - "sql_update t set v = (select max(n) from other) where id = 1", + "update t set v = (select max(n) from other) where id = 1", ) .expect("subquery-set update runs"); assert_eq!(result.rows_affected, 1); @@ -185,7 +185,7 @@ fn update_matching_no_rows_is_ok() { 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, 'keep')", "t"); - let result = run_update(&db, &rt, "sql_update t set v = 'x' where id = 999") + let result = run_update(&db, &rt, "update t set v = 'x' where id = 999") .expect("no-match update is a success"); assert_eq!(result.rows_affected, 0, "no rows matched"); let csv = read_csv(&project, "t").expect("t.csv"); @@ -198,7 +198,7 @@ fn update_appends_literal_line_to_history() { 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, 'old')", "t"); - let input = "sql_update t set v = 'new' where id = 1"; + let input = "update t set v = 'new' where id = 1"; run_update(&db, &rt, input).expect("update runs"); let body = std::fs::read_to_string(project.path().join("history.log")) .expect("history.log present"); @@ -215,7 +215,7 @@ fn update_returning_yields_modified_columns() { 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, 'old'), (2, 'keep')", "t"); - let result = run_update(&db, &rt, "sql_update t set v = 'new' where id = 1 returning id, v") + let result = run_update(&db, &rt, "update t set v = 'new' where id = 1 returning id, v") .expect("UPDATE … RETURNING runs"); assert_eq!(result.rows_affected, 1, "one row updated"); assert_eq!(result.data.columns, vec!["id".to_string(), "v".to_string()]); @@ -230,7 +230,7 @@ fn update_returning_recovers_bare_column_type() { let rt = rt(); create_cols(&db, &rt, "t", &[("id", Type::Int), ("active", Type::Bool)], &["id"]); seed(&db, &rt, "insert into t (id, active) values (1, false)", "t"); - let result = run_update(&db, &rt, "sql_update t set active = true where id = 1 returning active") + let result = run_update(&db, &rt, "update t set active = true where id = 1 returning active") .expect("UPDATE … RETURNING active runs"); assert_eq!(result.data.column_types, vec![Some(Type::Bool)], "bool type recovered"); assert_eq!(result.data.rows[0][0], Some("true".to_string())); @@ -246,7 +246,7 @@ fn update_returning_matching_no_rows_is_ok_and_empty() { 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, 'keep')", "t"); - let result = run_update(&db, &rt, "sql_update t set v = 'x' where id = 999 returning id, v") + let result = run_update(&db, &rt, "update t set v = 'x' where id = 999 returning id, v") .expect("no-match UPDATE … RETURNING is a success"); assert_eq!(result.rows_affected, 0, "no rows matched"); assert!(result.data.rows.is_empty(), "no rows returned"); diff --git a/tests/typing_surface/mod.rs b/tests/typing_surface/mod.rs index 4ea5c50..55b7f58 100644 --- a/tests/typing_surface/mod.rs +++ b/tests/typing_surface/mod.rs @@ -13,13 +13,14 @@ #![allow(dead_code, unreachable_pub)] use rdbms_playground::completion::{ - Completion, SchemaCache, TableColumn, candidates_at_cursor, + Completion, SchemaCache, TableColumn, candidates_at_cursor_in_mode, }; -use rdbms_playground::dsl::parser::parse_command_with_schema; +use rdbms_playground::dsl::parser::parse_command_with_schema_in_mode; use rdbms_playground::dsl::types::Type; use rdbms_playground::input_render::{ - AmbientHint, InputState, ambient_hint, classify_input_with_schema, + AmbientHint, InputState, ambient_hint_in_mode, classify_input_with_schema_in_mode, }; +use rdbms_playground::mode::Mode; pub mod insert_form_a; pub mod insert_form_b; @@ -174,11 +175,24 @@ pub struct Assessment { } /// Assess the typing surface at the given cell. +/// +/// The whole typing-surface matrix exercises the **DSL** surface +/// (insert Forms A/B/C, `update`/`delete` with `where` / `--all-rows`, +/// the DSL expression grammar, DDL, app commands) — which is the +/// **Simple-mode** surface (ADR-0003). So every facet here is computed +/// in Simple mode, and consistently so: `ambient_hint` already +/// defaults to Simple, and the others are pinned to Simple via their +/// `*_in_mode` variants. This matters since sub-phase 3j made +/// `insert`/`update`/`delete` shared entry words (ADR-0033 +/// Amendment 3): in Advanced mode those route to the SQL grammar +/// (different completion / hints / parse), whereas the DSL forms this +/// matrix documents live in Simple mode. The SQL surface is covered +/// by `tests/sql_*.rs` and the advanced-mode walker diagnostics. pub fn assess(input: &str, cursor: usize, schema: &SchemaCache) -> Assessment { - let state = classify_input_with_schema(input, schema); - let hint = ambient_hint(input, cursor, None, schema); - let completion = candidates_at_cursor(input, cursor, schema); - let parse_result = match parse_command_with_schema(input, schema) { + let state = classify_input_with_schema_in_mode(input, schema, Mode::Simple); + let hint = ambient_hint_in_mode(input, cursor, None, schema, Mode::Simple); + let completion = candidates_at_cursor_in_mode(input, cursor, schema, Mode::Simple); + let parse_result = match parse_command_with_schema_in_mode(input, schema, Mode::Simple) { Ok(cmd) => Ok(command_kind_label(&cmd)), Err(e) => Err(parse_error_label(&e)), }; diff --git a/tests/typing_surface/snapshots/typing_surface_matrix__typing_surface__insert_form_b__form_b_with_extra_value_for_serial_column_is_invalid@form_b_extra_serial_value.snap b/tests/typing_surface/snapshots/typing_surface_matrix__typing_surface__insert_form_b__form_b_with_extra_value_for_serial_column_is_invalid@form_b_extra_serial_value.snap index 40d5c40..cfb132f 100644 --- a/tests/typing_surface/snapshots/typing_surface_matrix__typing_surface__insert_form_b__form_b_with_extra_value_for_serial_column_is_invalid@form_b_extra_serial_value.snap +++ b/tests/typing_surface/snapshots/typing_surface_matrix__typing_surface__insert_form_b__form_b_with_extra_value_for_serial_column_is_invalid@form_b_extra_serial_value.snap @@ -11,7 +11,7 @@ Assessment { ), hint: Some( Prose( - "for `Name`: Type a quoted string (e.g. 'Alice') or null (`id` auto-generated — skipped here; list columns explicitly, e.g. `insert into T (...) values (...)`, to set it.)", + "for `Name`: Type a quoted string (e.g. 'Alice') or null (`id` auto-generated — skipped here; list columns explicitly, e.g. `insert into T (...) values (...)`, to set it.) (valid as SQL in advanced mode — `mode advanced` or prefix `:`)", ), ), completion: Some(