From 49ea03b0d51e6e58858f0b66e6015ea4f6b135b6 Mon Sep 17 00:00:00 2001 From: "claude@clouddev1" Date: Tue, 26 May 2026 22:48:46 +0000 Subject: [PATCH] =?UTF-8?q?feat:=20ADR-0036=20Phase=203a=20=E2=80=94=20liv?= =?UTF-8?q?e=20typed-slot=20hints=20+=20highlighting=20for=20SQL=20SET=20v?= =?UTF-8?q?alues?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wire the DSL's column-typed value slots into the advanced-mode SQL UPDATE/UPSERT `SET col = ` value position so a learner gets the same per-column hint ("for `Email`: type a quoted string") and live numeric- shape mismatch highlight the simple-mode DSL gives. Discriminate literal-vs-expression with a boundary-aware lookahead (shared::SET_VALUE), NOT the naive `Choice(typed-slot, sql_expr)` the ADR originally sketched: the walker's Choice is first-match-wins with no backtrack, so a typed slot would greedily match the leading `1` of `1 + 2` and commit, regressing valid SQL (e.g. the existing `values (1, 1 + 2)` test). The lookahead peeks the whole value position: a literal routes to the typed slot only when it fills the position up to the next `,`/`)`/`;`/`where`/`returning`/end; everything else falls through to the full sql_expr grammar unchanged. The SET column ident gets `writes_column: true` so `current_column` drives the slot + hint. Scope: Phase 3a covers UPDATE's assignment list and INSERT's ON CONFLICT DO UPDATE SET. Phase 3b (INSERT VALUES — needs a per-position grammar restructure + multi-row) is deferred. Records ADR-0036 Amendment 1 with the mechanism correction + the 3a/3b split. Tests: 1939 passing (+5), 0 failed, 0 skipped, 1 ignored; clippy clean. --- docs/adr/0036-typed-dml-values-vs-verbatim.md | 89 +++++++++++-- docs/adr/README.md | 2 +- src/dsl/grammar/shared.rs | 111 ++++++++++++++++ src/dsl/grammar/sql_insert.rs | 24 ++-- src/dsl/grammar/sql_update.rs | 27 ++-- tests/sql_insert.rs | 35 +++++ tests/sql_update.rs | 120 ++++++++++++++++++ 7 files changed, 376 insertions(+), 32 deletions(-) diff --git a/docs/adr/0036-typed-dml-values-vs-verbatim.md b/docs/adr/0036-typed-dml-values-vs-verbatim.md index fd859ee..58896d8 100644 --- a/docs/adr/0036-typed-dml-values-vs-verbatim.md +++ b/docs/adr/0036-typed-dml-values-vs-verbatim.md @@ -16,8 +16,12 @@ validation + offending-value retention; the same capture-at-parse technique on the SET assignment list — `capture_set_literals` in `data.rs` — classifying each top-level RHS literal-vs-expression, validating literals in `do_sql_update`, and reading them in `user_value_for_column`; `WHERE` is not -validated, execution stays verbatim). Phase 3 (completion -hinting/highlighting — the only part needing a grammar change) pending. +validated, execution stays verbatim). **Phase 3a implemented 2026-05-26** +— live typed-slot hints + numeric-shape highlighting for advanced-mode +`UPDATE`/UPSERT `SET col = ` value positions, via a +**boundary-aware lookahead** (not the naive `Choice` this ADR originally +sketched in §5 — see **Amendment 1**). Phase 3b (`INSERT … VALUES` typed +slots — needs a per-position grammar restructure + multi-row) pending. **Augments** **ADR-0030 §4** and **ADR-0033 §10** — it does **not** supersede them and does **not** change the execution model. Advanced-mode @@ -265,13 +269,17 @@ execution), only its `Result` is used. verbatim update; `user_value_for_column` reads them so a constraint error names the offending value. `WHERE` is deliberately not validated (§2). - **Phase 3 — completion hinting / highlighting.** This is the *only* - part that needs a grammar change: a `Choice(typed-literal-slot, - sql_expr)` at each value position (reusing the DSL's live - `column_value_list` / `TypedValueSlot`s — `data.rs:141`/`189`/`269`), - so the column type drives a live hint and a mismatch highlights while - typing. When Phase 3 lands, the typed slot supersedes Phase 1's - classification of literals (the validation/enrichment built on top is - unaffected — that is the only throwaway, by design). + part that needs a grammar change: a typed-literal slot vs `sql_expr` at + each value position (reusing the DSL's live `column_value_list` / + `TypedValueSlot`s — `data.rs:141`/`189`/`269`), so the column type + drives a live hint and a mismatch highlights while typing. When Phase 3 + lands, the typed slot supersedes Phase 1/2's classification of literals + (the validation/enrichment built on top is unaffected — that is the + only throwaway, by design). **The literal-vs-expression discriminator + is a boundary-aware *lookahead*, not a naive `Choice(typed-slot, + sql_expr)` — see Amendment 1, which corrects this section's mechanism + and splits Phase 3 into 3a (`SET`, implemented 2026-05-26) and 3b + (`VALUES`, pending).** ### 6. Non-goals @@ -315,6 +323,69 @@ execution), only its `Result` is used. validation/enrichment built on it is permanent; only the detection is provisional — a deliberate, documented small throwaway. +## Amendment 1 — Phase 3 mechanism is a boundary-aware lookahead, not a naive `Choice`; Phase 3 split into 3a/3b (2026-05-26) + +**Status:** Accepted (agreed with the user in conversation, 2026-05-26). +**Phase 3a implemented the same day.** + +§5 Phase 3 sketched the mechanism as "a `Choice(typed-literal-slot, +sql_expr)` at each value position." Implementation found that sketch is +**wrong as written** and would regress valid SQL, so it is corrected here. + +**Why the naive `Choice` is broken.** The walker's `Node::Choice` is +first-match-wins with **no cross-branch backtracking** once a branch has +committed a `Matched` (a later failure in the enclosing `Seq` does not +re-enter the Choice). At, say, an `int` column, the value `1 + 2`: + +- Branch 0 (the typed slot) matches just the `1` and commits, leaving + `+ 2` dangling — the enclosing tuple/assignment then fails on `+`. +- The Choice never falls through to `sql_expr`, so a **valid, currently + parsing SQL expression is rejected**. + +This is not hypothetical: `tests/sql_insert.rs::sql_insert_expression_value_is_not_validated_and_runs` +exercises exactly `values (1, 1 + 2)`. Putting `sql_expr` first instead +makes the typed slot unreachable (sql_expr matches bare literals too), +defeating the purpose. So the discriminator must know whether a literal +**fills the whole value position** before choosing the typed slot. + +**The correction.** Discriminate by a **boundary-aware lookahead** +(`shared::SET_VALUE` → `set_value_node`): peek the value position and +route to the column-typed slot only when it is empty, a partial string +still being typed, or a single complete literal token whose next token is +a position boundary (`,` / `)` / `;` / `where` / `returning` / end); +otherwise route to `Subgrammar(sql_expr)`. The empty case still routes to +the slot so the per-column hint shows while the cursor sits right after +`=`. A leading sign folds into the literal (the slot's `NumberLit` uses +the same `consume_number_literal` that eats a `-`), so signed literals get +typed treatment too. `Node::Lookahead` already exists and is used the same +way by `insert_first_paren` (`data.rs`). The validation/enrichment from +Phases 1–2 is unchanged; only the *live-feedback detection* uses this +lookahead — consistent with §5's note that Phase 3's detection is the one +deliberate throwaway. + +**Phase 3 split into 3a + 3b.** The two halves differ structurally: + +- **Phase 3a (implemented) — `UPDATE` / UPSERT `SET col = `.** Low + risk: the preceding `SET` column ident gets `writes_column: true` so + `current_column` (and the `pending_value_column` hint framing) is set + per assignment; the RHS becomes `shared::SET_VALUE`. Covers both + `sql_update`'s assignment list and `sql_insert`'s `ON CONFLICT … DO + UPDATE SET`. Mismatch examples now caught **live** (e.g. `set k = 3.14` + at an `int` column), matching what simple mode already does — earlier, + better feedback than Phase 2's execution-time catch. +- **Phase 3b (pending) — `INSERT … VALUES (…)`.** Harder: the values list + is `Repeated(VALUE_EXPR)` with **no per-position column identity**, and + multi-row `values (..),(..)` must be handled. It needs the DSL-style + per-position restructure (a `DynamicSubgrammar` emitting one + boundary-aware position per column), tracked as its own step. + +**Known limitation (both phases, matches the DSL).** `date` / `shortid` / +`datetime` **format** is still not validated at parse — those slots accept +any quoted string; the format is checked at bind/execution time (Phase 2). +So the live highlight catches *numeric-shape* mismatches (`int`/`decimal`/ +`bool`), not malformed dates. The column-type **hint** still shows for +every type. + ## See also - ADR-0030 §4 / ADR-0033 §10 — the execute-path this ADR **augments** diff --git a/docs/adr/README.md b/docs/adr/README.md index fac6bae..106664b 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -41,4 +41,4 @@ This directory contains the project's ADRs, recorded per - [ADR-0033 — The full SQL DML grammar (`INSERT` / `UPDATE` / `DELETE`)](0033-sql-dml-grammar.md) — **Accepted** (implemented + verified through sub-phase 3k, 2026-05-23; phase-exit report `docs/handoff/20260523-phase-3-verification.md`), 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; **implemented 2026-05-24** (plan `docs/plans/20260524-adr-0034-history-journal.md`); **Amendment 1 (2026-05-24): replay filters out app-lifecycle commands** — a working `replay history.log` (the §3 fix) exposed that the journal also records `save as`/`load`/`new`/`export`/`import`/`rebuild`/`mode` (which would panic the worker dispatch or abort the replay), so replay now re-applies **only** schema/data write commands and **skips** every `Command::App` + nested `Command::Replay`; **all skips continue** (never abort — reversing the prior nested-`replay` refusal, so a journal containing a once-run `replay` needs no hand-editing, and the infinite-loop footgun is closed by construction), with a `[skip]` **warning** on `import` and nested-`replay` skips (their omission can leave replayed state incomplete) and silent skips for the rest; `replay.error_nested` removed, `replay.skipped_import`/`replay.skipped_replay` added, `ReplayCompleted` carries `warnings` - [ADR-0035 — Advanced-mode SQL DDL](0035-advanced-mode-sql-ddl.md) — **Accepted** (design agreed 2026-05-24; validated end-to-end by sub-phases 4a/4a.2/4a.3/4b `CREATE TABLE` (incl. foreign keys) + 4c `DROP TABLE [IF EXISTS]` + 4d `CREATE [UNIQUE] INDEX` / `DROP INDEX [IF EXISTS]` + 4e `ALTER TABLE` add/drop/rename column + 4f `ALTER TABLE … ALTER COLUMN TYPE` + 4g `ALTER TABLE` add/drop constraint + add FK + 4h `ALTER TABLE … RENAME TO` + 4i verification sweep (completion merge + simple/advanced completion colour + describe of table-level constraints + self-ref FK indicator + CREATE-TABLE help/usage), implemented 2026-05-25/26 — **Phase 4 complete**; **Amendment 1, 2026-05-26**: drop a composite UNIQUE via a derived, engine-neutral `unique_` name that reuses the existing `DROP CONSTRAINT ` grammar — no new syntax, no metadata, §4g anonymity intact; `describe` shows the name; dropping a UNIQUE-covered *column* now refuses with that name + the drop command), **Phase 4** of the ADR-0030 roadmap (peer of 0031/0032/0033) and **clarifies ADR-0030 §4**. Advanced-mode `CREATE`/`DROP`/`ALTER TABLE` + `CREATE`/`DROP INDEX` get their **own per-statement commands** (`SqlCreateTable`/`SqlAlterTable`/`SqlDropTable`/`SqlCreateIndex`/`SqlDropIndex`), like DML's `Sql*` set — but unlike DML they **execute *structurally*, not verbatim** (raw execution would lose the playground's types, named relationships, and `STRICT`; "verbatim" was a DML convenience, not a rule). Handlers **reuse the low-level schema/metadata helpers** where the operation matches simple mode and **stand alone where the SQL surface is richer** (clarity over forced refactoring); simple mode is untouched (additive). Dispatch: `create`/`drop` reuse ADR-0033 Amendment 1's category-grouped mode-aware dispatch (SQL-first, simple fallback); `alter` is a new advanced-only entry word. Full surface (no pre-emptive cuts, `Q4`): `CREATE TABLE` with column + table constraints, single/compound `PRIMARY KEY`, inline + table-level `FOREIGN KEY` → **named relationships** (one statement = one command = **one undo step**, ADR-0006); `ALTER TABLE` add/drop/rename column, `ALTER COLUMN TYPE`, add/drop constraint, add FK, **`RENAME TO`** (advanced-only table rename — new low-level op renaming the table + its CSV + the relationship and table-CHECK metadata, closing the rename half of `C1`); `CREATE [UNIQUE] INDEX` / `DROP INDEX`. Type slot accepts the ten playground keywords **and** standard-SQL aliases (`integer`→`int`, `varchar`→`text`, `timestamp`→`datetime`, …; length args accepted-and-ignored; no engine type names in/out — ADR-0030 §5). `CHECK`/`DEFAULT` reuse ADR-0031 `sql_expr`. **Pre-implementation `/runda` refinements (2026-05-24, user-confirmed):** `CREATE TABLE`/`DROP TABLE` **admit `IF [NOT] EXISTS`** (no-op-that-succeeds-with-a-note — a near-universal cross-vendor idiom, reclassified *into* scope, not engine-specific); `INTEGER PRIMARY KEY` maps to a **plain `int`** PK, *not* auto-increment (`serial` stays the sole auto-increment type). **Column-type-conversion is unified** (ADR-0017 engine, mode-appropriate policy): clean auto-converts and incompatible/own-type-static cases refuse in both modes, but a **lossy** change refuses-by-default in simple mode (`--force-conversion` opts in) while advanced mode **performs it with a loss note** and relies on **`undo` as the safety net** — no force flag, no dropping to simple mode (a payoff of shipping ADR-0006 first). OOS: views/triggers/txn-control/PRAGMA/etc. (ADR-0030 §3), the Postgres `USING` clause, and the DSL→SQL teaching echo (ADR-0030 Phase 5). Sub-phases 4a–4i, plus **4a.2** (per-column `CHECK`/`DEFAULT` via raw `sql_expr` text — `sql_expr` is validate-only, no `Expr` AST — + composite `UNIQUE(a,b)`; no new internal table) and **4a.3** (table-level/multi-column `CHECK`, landed via the new `__rdbms_playground_table_checks` metadata table because SQLite has no PRAGMA for CHECK; the builder tells a table-level CHECK from a column-level one by element position) and **4b** (foreign keys — inline `REFERENCES` + table-level `FOREIGN KEY` → ADR-0013 named relationships in the create transaction, one undo step; self-references + bare `REFERENCES ` supported, user-confirmed) and **4c** (`DROP TABLE [IF EXISTS]` → `SqlDropTable`, reusing `do_drop_table`; `IF EXISTS` is a no-op-with-note via `DropOutcome::Skipped`) and **4d** (`CREATE [UNIQUE] INDEX [IF NOT EXISTS] [] ON (cols)` → `SqlCreateIndex` and `DROP INDEX [IF EXISTS] ` → `SqlDropIndex`, reusing `do_add_index`/`do_drop_index`; **`CREATE UNIQUE INDEX` admitted** — ADR-0025 **Amendment 1** — via an additive `IndexSchema.unique` flag that round-trips through `project.yaml` and rebuild, with `[unique]` markers in the structure view + items panel, while simple-mode `add unique index` stays deferred; `IF [NOT] EXISTS` reuses the 4c skip path; `create`/`drop` each gain a *second* advanced node, exercising the all-candidates dispatch) and **4e** (`ALTER TABLE` add/drop/rename column → `SqlAlterTable`; `alter` is a new advanced-**only** entry word, runtime-decomposed to the existing `do_add_column`/`do_drop_column`/`do_rename_column` — no new worker layer; `do_add_column` extended to consume raw `default_sql`/`check_sql` so ADD COLUMN reaches CREATE-TABLE constraint parity; drop/rename refuse a column any CHECK references (table-level AND column-level, incl. a column's own self-check on rename) — the 4a.3 deferral, via a raw-CHECK-text tokenizer in the shared executors, so it guards both surfaces and fixes a latent rename-drift bug; SQL DROP COLUMN refuses an index-covered column with no `--cascade` spelling; the column executors + `do_add_index` gained an internal-`__rdbms_*`-table guard — all user-confirmed) and **4f** (`ALTER TABLE … ALTER COLUMN TYPE` → a fourth `AlterTableAction`, runtime-decomposed to the existing `change_column_type` with `ChangeColumnMode::ForceConversion` — which **is** the §7 advanced policy: lossy converts *with a note* (no force flag), incompatible + ADR-0017 static refusals (`↔ blob`, same-type, `date ↔ datetime`, non-`int → serial`) still refuse, while **`int → serial` is allowed** (auto-fills nulls + UNIQUE, ADR-0018 §8 — the §7 "→serial refused" summary is looser than the code); the builder discriminates the fourth branch by the **`type` keyword** (unique — ADD COLUMN's type is an ident), the type slot reuses `SQL_TYPE`; the internal-`__rdbms_*` guard was folded into `do_change_column_type`, closing the simple `change column` exposure too — user-confirmed) and **4g** (`ALTER TABLE … ADD [CONSTRAINT ] (CHECK | UNIQUE | FOREIGN KEY)` + `DROP CONSTRAINT `; ADD = CHECK + composite UNIQUE + FK, with `ADD PRIMARY KEY` and a *named* UNIQUE refused — composite UNIQUE is anonymous in our model; each ADD reuses a low-level path (table-CHECK/UNIQUE rebuild with a dry-run guard; FK → `add_relationship`, bare `REFERENCES

` → parent single-PK), DROP CONSTRAINT resolves the name to a table-CHECK then a child-side FK; **named table-CHECKs round-trip** via a nullable `name` column on `__rdbms_playground_table_checks` (**rebuild-only** arrival — pre-4g projects gain it on `rebuild`, a named add on an un-upgraded project is refused with a friendly "rebuild first" message) *and* a `project.yaml` `check_constraints` extension to an `{expr, name}` mapping (the bare-string form still reads); the internal-`__rdbms_*` guard was folded into `do_add_constraint`/`do_add_relationship`, completing that guard class — all user-confirmed) and **4h** (`ALTER TABLE … RENAME TO` — the one genuinely new low-level op, `do_rename_table`: a native engine rename plus one-transaction reconciliation of every metadata row naming the table (`__rdbms_playground_columns`, **both ends** of `__rdbms_playground_relationships`, `__rdbms_playground_table_checks`), the CSV file (the existing rewrite+delete path — no new persistence method), and **CHECK text that qualifies a column with the old table name** (`T.age`→`U.age`, a planning-`/runda` finding — the engine rewrites the live CHECK but the stored text would drift and break a fresh rebuild; `rewrite_check_table_qualifier` keeps them in step); grammar splits the `rename` verb into one branch with an inner Choice on a distinct second keyword (`column` vs `to`), the new-name slot mirroring the `CREATE TABLE` name slot; refuses same-name / existing-target / `__rdbms_*` / non-existent, with **case-insensitive** collision checks behind an engine-neutral pre-check (a finished-slice `/runda` finding — the engine matches names case-insensitively); auto-named indexes *and* relationships keep their stale names (only table-name columns update — §6 scope); one undo step; advanced-only, closing the rename half of `C1` — all user-confirmed) and **4i** (the verification sweep that completes Phase 4: the shared-entry-word completion merge + the simple-vs-advanced completion colour-when-mixed with Both→Advanced→Simple block ordering; `describe` of table-level composite UNIQUE + table CHECK; the self-ref FK pre-submit indicator fix; and the CREATE-TABLE help/usage skeleton refresh). **All of Phase 4 (4a–4i) is shipped.** Each sub-phase has exit + DA gates -- [ADR-0036 — Value validation for advanced-mode DML](0036-typed-dml-values-vs-verbatim.md) — **Accepted** (design agreed + `/runda`'d 2026-05-26; mechanism then **deliberately narrowed** from "bind literals via the DSL path" to surgical **"validate-and-retain, execute verbatim"** after the user resisted consolidating the modes and a concrete auto-fill difference confirmed even the single-row literal case isn't identical across modes; **Phases 1–2 implemented** 2026-05-26 — `INSERT … VALUES` and `UPDATE … SET` literal validation + offending-value retention, capture-at-parse with no grammar change; Phase 3 (completion hinting/highlighting) pending). **Augments — does NOT supersede — ADR-0030 §4 / ADR-0033 §10**: execution stays verbatim, ADR-0033 Amendment 3's two-command identity (`Insert` vs `SqlInsert`) **stands**. The problem (investigated 2026-05-26; characterization test `sql_insert.rs::sql_dml_skips_app_level_value_validation_that_the_dsl_enforces` proves it): advanced-mode SQL DML gets **none** of the DSL's value feedback — a malformed `date` like `2025/01/15` is silently written, and the offending value is missing from constraint errors — because literal values are spliced into text and discarded (only `STRICT` storage types check them). **Fix (surgical): validate each literal value against its column type before the verbatim insert, and retain it for error reporting — sharing only the per-type validators (`Value::bind_for_column`/`validate_date`/`shortid::validate`), nothing else.** No binding, no statement reconstruction, no auto-fill change, no command-identity collapse — because the two gaps are closed by validation + retention alone, and executing the user's own text is already safe. The literal set = `NULL`/boolean/string/**signed**-numeric; arithmetic/functions/subqueries/column-refs are expressions (skipped — the engine evaluates them). `WHERE` not validated (it's an expression in general; motivation met by `VALUES`/`SET`); `SELECT`/`INSERT … SELECT`/`RETURNING`/`ON CONFLICT` need no special handling since execution is untouched. Phased: **Phase 1** capture-at-parse + validate + retain for `INSERT … VALUES` (no grammar change, no reparse — closes both proven gaps); **Phase 2** `UPDATE … SET` literals; **Phase 3** completion hinting/highlighting (the only part needing a grammar change — `Choice(typed-literal-slot, sql_expr)` reusing the DSL `TypedValueSlot`s at `data.rs:141`/`189`/`269`; supersedes only Phase 1's literal *detection*, not the validation/enrichment on top). Non-goals: binding/reconstruction, collapsing command identity (Am3 stands), changing `serial`/`shortid` auto-fill (`requirements.md` **X4**, a separate possible-bug), a structural `SELECT`, a full SQL-expression AST. Embodies `requirements.md` **X5** (share a *mechanic*, not a *command*); the neutral "that value" safety net (ADR-0035 Amendment 1) stays correct for genuinely-computed values +- [ADR-0036 — Value validation for advanced-mode DML](0036-typed-dml-values-vs-verbatim.md) — **Accepted** (design agreed + `/runda`'d 2026-05-26; mechanism then **deliberately narrowed** from "bind literals via the DSL path" to surgical **"validate-and-retain, execute verbatim"** after the user resisted consolidating the modes and a concrete auto-fill difference confirmed even the single-row literal case isn't identical across modes; **Phases 1–2 implemented** 2026-05-26 — `INSERT … VALUES` and `UPDATE … SET` literal validation + offending-value retention, capture-at-parse with no grammar change; **Phase 3a implemented** 2026-05-26 — live typed-slot hints + numeric-shape highlighting for `UPDATE`/UPSERT `SET col = ` via a boundary-aware lookahead (Amendment 1 corrects this ADR's naive-`Choice` sketch); Phase 3b (`INSERT … VALUES` typed slots) pending). **Augments — does NOT supersede — ADR-0030 §4 / ADR-0033 §10**: execution stays verbatim, ADR-0033 Amendment 3's two-command identity (`Insert` vs `SqlInsert`) **stands**. The problem (investigated 2026-05-26; characterization test `sql_insert.rs::sql_dml_skips_app_level_value_validation_that_the_dsl_enforces` proves it): advanced-mode SQL DML gets **none** of the DSL's value feedback — a malformed `date` like `2025/01/15` is silently written, and the offending value is missing from constraint errors — because literal values are spliced into text and discarded (only `STRICT` storage types check them). **Fix (surgical): validate each literal value against its column type before the verbatim insert, and retain it for error reporting — sharing only the per-type validators (`Value::bind_for_column`/`validate_date`/`shortid::validate`), nothing else.** No binding, no statement reconstruction, no auto-fill change, no command-identity collapse — because the two gaps are closed by validation + retention alone, and executing the user's own text is already safe. The literal set = `NULL`/boolean/string/**signed**-numeric; arithmetic/functions/subqueries/column-refs are expressions (skipped — the engine evaluates them). `WHERE` not validated (it's an expression in general; motivation met by `VALUES`/`SET`); `SELECT`/`INSERT … SELECT`/`RETURNING`/`ON CONFLICT` need no special handling since execution is untouched. Phased: **Phase 1** capture-at-parse + validate + retain for `INSERT … VALUES` (no grammar change, no reparse — closes both proven gaps); **Phase 2** `UPDATE … SET` literals; **Phase 3** completion hinting/highlighting (the only part needing a grammar change — a typed-literal slot vs `sql_expr` reusing the DSL `TypedValueSlot`s at `data.rs:141`/`189`/`269`, discriminated by a **boundary-aware lookahead** not a naive `Choice` per **Amendment 1**; split into **3a** `SET` (done) and **3b** `VALUES` (pending); supersedes only Phase 1/2's literal *detection*, not the validation/enrichment on top). Non-goals: binding/reconstruction, collapsing command identity (Am3 stands), changing `serial`/`shortid` auto-fill (`requirements.md` **X4**, a separate possible-bug), a structural `SELECT`, a full SQL-expression AST. Embodies `requirements.md` **X5** (share a *mechanic*, not a *command*); the neutral "that value" safety net (ADR-0035 Amendment 1) stays correct for genuinely-computed values diff --git a/src/dsl/grammar/shared.rs b/src/dsl/grammar/shared.rs index f2692d8..56e7721 100644 --- a/src/dsl/grammar/shared.rs +++ b/src/dsl/grammar/shared.rs @@ -456,3 +456,114 @@ pub fn column_value_list(ctx: &WalkContext) -> Node { } Node::Seq(Box::leak(children.into_boxed_slice())) } + +// ================================================================= +// Advanced-mode SQL `SET col = ` value slot (ADR-0036 Phase 3a) +// ================================================================= +// +// A SQL UPDATE / UPSERT `SET col = ` value position routes a +// *lone literal* (a string / number / `null` / `true` / `false` that +// fills the whole position up to the next `,` / `where` / `returning` +// / `;` / end-of-input) to the column-typed slot — so the learner gets +// the same per-column hint + numeric-shape highlight the DSL gives — +// and routes anything else (an expression: arithmetic, a literal- +// prefixed form like `1 + 2`, a function call, a scalar subquery, a +// column / `excluded` reference) to the full `sql_expr` grammar, +// unchanged (ADR-0030 §4). +// +// Discrimination is by *lookahead*, NOT `Choice([typed_slot, +// sql_expr])`. A naive Choice would let the typed slot greedily match +// the leading literal of `1 + 2` and commit, leaving `+ 2` dangling — +// the walker's `Choice` is first-match-wins with no cross-branch +// backtrack — turning a valid expression into a parse error. The +// lookahead peeks the whole value position first, so a literal routes +// to the typed slot only when it is the *entire* value (ADR-0036 +// Amendment 1). + +/// Lookahead factory for a SQL `SET col = ` value position +/// (ADR-0036 Phase 3a). Routes a lone literal to the column-typed slot +/// (live hint + highlight) and everything else to `sql_expr`. +fn set_value_node(_ctx: &WalkContext, source: &str, pos: usize) -> Node { + if set_rhs_is_lone_literal(source, pos) { + // The column type was resolved into `current_column` by the + // preceding `SET` column ident (`writes_column: true`), so the + // typed slot drives the per-column hint + highlight. + Node::DynamicSubgrammar(current_column_value) + } else { + // An expression — the engine evaluates it; keep the full + // `sql_expr` surface. Returned from a `fn` (not a `const`) so + // the reference doesn't enter the sql_expr ⇄ sql_select + // const-evaluation cycle (see the matching note in `data.rs`). + Node::Subgrammar(&crate::dsl::grammar::sql_expr::SQL_OR_EXPR) + } +} + +/// The SQL `SET col = ` value slot (ADR-0036 Phase 3a). Shared by +/// `sql_update`'s assignment list and the `sql_insert` UPSERT +/// `DO UPDATE SET`. +pub const SET_VALUE: Node = Node::Lookahead(set_value_node); + +/// True when the `SET` RHS starting at `pos` is empty, a partial string +/// still being typed, or exactly one complete literal token that fills +/// the value position (the next token is a position boundary). Such +/// positions route to the column-typed slot so its hint/highlight fire; +/// everything else is an expression and routes to `sql_expr`. +/// +/// An empty RHS counts as lone-literal so the typed-slot hint shows +/// while the cursor sits right after `=`. A signed number (`-5`) counts +/// too: `consume_number_literal` — the same consumer the slot's +/// `NumberLit` uses — folds the leading sign into the literal, so the +/// slot can match it. +fn set_rhs_is_lone_literal(source: &str, pos: usize) -> bool { + use crate::dsl::walker::lex_helpers::{ + consume_ident, consume_number_literal, consume_string_literal, skip_whitespace, + }; + let p = skip_whitespace(source, pos); + if p >= source.len() { + return true; // empty RHS — show the typed-slot hint + } + // A string literal — single-quoted (SQL strings). Complete (a + // boundary must follow) or a partial one still being typed (route + // to the slot so the hint persists while typing). + if source.as_bytes()[p] == b'\'' { + return match consume_string_literal(source, p) { + Some(((_, end), _)) => next_is_set_boundary(source, end), + None => true, // unterminated string, mid-typing + }; + } + // A number literal (incl. a leading sign) that fills the position. + if let Some((_, end)) = consume_number_literal(source, p) { + return next_is_set_boundary(source, end); + } + // `null` / `true` / `false` filling the position. + if let Some((s, e)) = consume_ident(source, p) { + let word = &source[s..e]; + if word.eq_ignore_ascii_case("null") + || word.eq_ignore_ascii_case("true") + || word.eq_ignore_ascii_case("false") + { + return next_is_set_boundary(source, e); + } + return false; // any other identifier → column ref / function → expression + } + false // punctuation / sign-then-space / `(` → expression +} + +/// True when the next non-whitespace token after `pos` ends a `SET` +/// value position: `,` (next assignment), `)` / `;` / end-of-input, or +/// a `where` / `returning` clause keyword. +fn next_is_set_boundary(source: &str, pos: usize) -> bool { + use crate::dsl::walker::lex_helpers::{consume_ident, skip_whitespace}; + let p = skip_whitespace(source, pos); + let Some(b) = source.as_bytes().get(p) else { + return true; // end of input + }; + if *b == b',' || *b == b';' || *b == b')' { + return true; + } + if let Some((s, e)) = consume_ident(source, p) { + let word = &source[s..e]; + return word.eq_ignore_ascii_case("where") || word.eq_ignore_ascii_case("returning"); + } + false +} diff --git a/src/dsl/grammar/sql_insert.rs b/src/dsl/grammar/sql_insert.rs index fac4b3d..3c04886 100644 --- a/src/dsl/grammar/sql_insert.rs +++ b/src/dsl/grammar/sql_insert.rs @@ -13,6 +13,7 @@ //! (3g), and `ON CONFLICT … ` UPSERT (3h) land in later //! sub-phases. +use crate::dsl::grammar::shared::SET_VALUE; use crate::dsl::grammar::sql_expr; use crate::dsl::grammar::sql_select::{ RETURNING_CLAUSE, SQL_SELECT_COMPOUND, WHERE_CLAUSE, reject_internal_table, @@ -146,30 +147,31 @@ const OPTIONAL_CONFLICT_TARGET: Node = Node::Optional(&Node::Seq(CONFLICT_TARGET /// The column on the left of one `DO UPDATE SET col = expr` /// assignment. Mirrors `sql_update`'s `ASSIGN_COLUMN` shape (same /// `update_set_column` role so it gets the same column completion / -/// diagnostics against the target table). +/// diagnostics against the target table). `writes_column: true` +/// resolves the column type into `current_column` so the RHS +/// `SET_VALUE` lookahead can dispatch the typed slot for a lone +/// literal (ADR-0036 Phase 3a). const UPSERT_SET_COLUMN: Node = Node::Ident { source: IdentSource::Columns, role: "update_set_column", validator: None, highlight_override: None, writes_table: false, - writes_column: false, + writes_column: true, writes_user_listed_column: false, writes_table_alias: false, writes_cte_name: false, writes_projection_alias: false, }; -/// `column '=' sql_expr` — the RHS reuses the shared expression -/// grammar (ADR-0031), so `excluded.col`, literals, operators, -/// `CASE`, and function calls are all admitted. `excluded` is the -/// would-have-been-inserted row (ADR-0033 §9); it parses as a +/// `column '=' ` — the RHS is the boundary-aware `SET_VALUE` +/// slot (ADR-0036 Phase 3a), shared with `sql_update`: a lone literal +/// routes to the column-typed slot (live hint + highlight) while an +/// expression — `excluded.col`, operators, `CASE`, function calls — +/// falls through to the full `sql_expr` grammar (ADR-0031). `excluded` +/// is the would-have-been-inserted row (ADR-0033 §9); it parses as a /// qualified ref via `sql_expr` and the engine resolves it. -static UPSERT_ASSIGNMENT_NODES: &[Node] = &[ - UPSERT_SET_COLUMN, - Node::Punct('='), - Node::Subgrammar(&sql_expr::SQL_OR_EXPR), -]; +static UPSERT_ASSIGNMENT_NODES: &[Node] = &[UPSERT_SET_COLUMN, Node::Punct('='), SET_VALUE]; static UPSERT_ASSIGNMENT: Node = Node::Seq(UPSERT_ASSIGNMENT_NODES); // `const` — used by value in `DO_UPDATE_NODES` (static-vs-const // rule: a `Node` referenced by value in a `static [...]` must be diff --git a/src/dsl/grammar/sql_update.rs b/src/dsl/grammar/sql_update.rs index b029b9d..0c5294c 100644 --- a/src/dsl/grammar/sql_update.rs +++ b/src/dsl/grammar/sql_update.rs @@ -15,7 +15,7 @@ //! `--all-rows` rail — a SQL `UPDATE` without `WHERE` runs as //! written (ADR-0030 §12). `RETURNING` (3g) lands later. -use crate::dsl::grammar::sql_expr; +use crate::dsl::grammar::shared::SET_VALUE; use crate::dsl::grammar::sql_select::{RETURNING_CLAUSE, WHERE_CLAUSE, reject_internal_table}; use crate::dsl::grammar::{IdentSource, Node, Word}; @@ -44,28 +44,33 @@ const TARGET_TABLE: Node = Node::Ident { }; /// The column on the left of one `SET col = expr` assignment. +/// +/// `writes_column: true` resolves the column's type into +/// `current_column` (and frames the value-slot hint via +/// `pending_value_column`), so the RHS `SET_VALUE` lookahead can +/// dispatch the column-typed slot for a lone literal (ADR-0036 +/// Phase 3a). const ASSIGN_COLUMN: Node = Node::Ident { source: IdentSource::Columns, role: "update_set_column", validator: None, highlight_override: None, writes_table: false, - writes_column: false, + writes_column: true, writes_user_listed_column: false, writes_table_alias: false, writes_cte_name: false, writes_projection_alias: false, }; -/// `column_name '=' sql_expr` — the RHS reuses the shared -/// expression grammar (ADR-0031), so literals, operators, `CASE`, -/// function calls, and scalar subqueries are all admitted; the -/// engine evaluates them at execution time. -static ASSIGNMENT_NODES: &[Node] = &[ - ASSIGN_COLUMN, - Node::Punct('='), - Node::Subgrammar(&sql_expr::SQL_OR_EXPR), -]; +/// `column_name '=' ` — the RHS is the boundary-aware +/// `SET_VALUE` slot (ADR-0036 Phase 3a): a lone literal routes to the +/// column-typed slot (live per-column hint + numeric-shape highlight, +/// shared with the DSL), while any expression — arithmetic, a +/// literal-prefixed form, `CASE`, function calls, scalar subqueries — +/// falls through to the full `sql_expr` grammar (ADR-0031), which the +/// engine evaluates at execution time. +static ASSIGNMENT_NODES: &[Node] = &[ASSIGN_COLUMN, Node::Punct('='), SET_VALUE]; static ASSIGNMENT: Node = Node::Seq(ASSIGNMENT_NODES); /// `assignment ( ',' assignment )*`. diff --git a/tests/sql_insert.rs b/tests/sql_insert.rs index dc56f04..ade2643 100644 --- a/tests/sql_insert.rs +++ b/tests/sql_insert.rs @@ -20,9 +20,12 @@ //! worker-level tests call `db.run_sql_insert` directly with the //! real reconstructed SQL. +use rdbms_playground::completion::{SchemaCache, TableColumn}; use rdbms_playground::db::{Database, DbError, InsertResult}; use rdbms_playground::dsl::{ColumnSpec, Command, Type, Value, parse_command}; use rdbms_playground::event::AppEvent; +use rdbms_playground::input_render::{AmbientHint, ambient_hint_in_mode}; +use rdbms_playground::mode::Mode; use rdbms_playground::persistence::Persistence; use rdbms_playground::project; use rdbms_playground::runtime::run_replay; @@ -1182,3 +1185,35 @@ fn sql_insert_natural_order_validates_against_schema_columns() { "natural-order insert validates the date against column `d`; events: {events:?}" ); } + +// ================================================================= +// ADR-0036 Phase 3a — live typed-slot hint for the UPSERT +// `ON CONFLICT … DO UPDATE SET col = ` value position. +// ================================================================= + +#[test] +fn advanced_upsert_do_update_set_offers_typed_slot_hint() { + // ADR-0036 Phase 3a: the `DO UPDATE SET col = ` value position + // shares the SQL UPDATE `SET` treatment, so it drives the same + // column-typed slot hint (boundary-aware lookahead → typed slot). + let mut cache = SchemaCache::default(); + let cols = vec![ + TableColumn { name: "id".to_string(), user_type: Type::Int, not_null: false, has_default: false }, + TableColumn { name: "Name".to_string(), user_type: Type::Text, not_null: false, has_default: false }, + ]; + cache.tables.push("Customers".to_string()); + cache.columns.push("id".to_string()); + cache.columns.push("Name".to_string()); + cache.table_columns.insert("Customers".to_string(), cols); + + let input = "insert into Customers (id, Name) values (1, 'x') on conflict (id) do update set Name="; + let hint = ambient_hint_in_mode(input, input.len(), None, &cache, Mode::Advanced); + let Some(AmbientHint::Prose(prose)) = hint else { + panic!("expected a Prose hint at the UPSERT SET value slot, got {hint:?}"); + }; + assert!(prose.contains("Name"), "hint names the column `Name`: {prose:?}"); + assert!( + prose.contains("quoted string"), + "text-column hint says `quoted string`: {prose:?}" + ); +} diff --git a/tests/sql_update.rs b/tests/sql_update.rs index a6a6717..586cc61 100644 --- a/tests/sql_update.rs +++ b/tests/sql_update.rs @@ -7,9 +7,14 @@ //! append `history.log`). A SQL `UPDATE` without `WHERE` runs //! across all rows with no rail (ADR-0030 §12). +use rdbms_playground::completion::{SchemaCache, TableColumn}; use rdbms_playground::db::{Database, DbError, UpdateResult}; use rdbms_playground::dsl::{ColumnSpec, Command, Type, Value, parse_command}; use rdbms_playground::event::AppEvent; +use rdbms_playground::input_render::{ + AmbientHint, InputState, ambient_hint_in_mode, classify_input_with_schema_in_mode, +}; +use rdbms_playground::mode::Mode; use rdbms_playground::persistence::Persistence; use rdbms_playground::project; use rdbms_playground::runtime::run_replay; @@ -368,3 +373,118 @@ fn update_returning_matching_no_rows_is_ok_and_empty() { assert!(result.data.rows.is_empty(), "no rows returned"); assert_eq!(result.data.columns, vec!["id".to_string(), "v".to_string()], "columns still present"); } + +// ================================================================= +// ADR-0036 Phase 3a — live typed-slot hints + highlighting for +// advanced-mode `SET col = ` (boundary-aware lookahead). +// ================================================================= + +/// Build a `SchemaCache` for the advanced-mode typing-surface tests +/// (mirrors `tests/typing_surface`'s `build_schema`). +fn schema_cache(tables: &[(&str, &[(&str, Type)])]) -> SchemaCache { + let mut cache = SchemaCache::default(); + for (table, cols) in tables { + let table_cols: Vec = cols + .iter() + .map(|(n, t)| TableColumn { + name: (*n).to_string(), + user_type: *t, + not_null: false, + has_default: false, + }) + .collect(); + cache.tables.push((*table).to_string()); + for c in &table_cols { + if !cache.columns.contains(&c.name) { + cache.columns.push(c.name.clone()); + } + } + cache.table_columns.insert((*table).to_string(), table_cols); + } + cache +} + +#[test] +fn advanced_update_set_value_offers_typed_slot_hint_for_column() { + // ADR-0036 Phase 3a: at a `SET col = ` value position the + // advanced-mode SQL UPDATE now drives the same column-typed slot + // hint the DSL gives — "for `Email`: type a quoted string …" — + // instead of the type-blind sql_expr surface. + let schema = schema_cache(&[( + "Customers", + &[("id", Type::Serial), ("Name", Type::Text), ("Email", Type::Text)], + )]); + let input = "update Customers set Email="; + let hint = ambient_hint_in_mode(input, input.len(), None, &schema, Mode::Advanced); + let Some(AmbientHint::Prose(prose)) = hint else { + panic!("expected a Prose hint at the typed value slot, got {hint:?}"); + }; + assert!(prose.contains("Email"), "hint names the column `Email`: {prose:?}"); + assert!( + prose.contains("quoted string"), + "text-column hint says `quoted string`: {prose:?}" + ); +} + +#[test] +fn advanced_update_set_date_value_hint_says_yyyy_mm_dd() { + let schema = schema_cache(&[("Things", &[("k", Type::Int), ("dt", Type::Date)])]); + let input = "update Things set dt="; + let hint = ambient_hint_in_mode(input, input.len(), None, &schema, Mode::Advanced); + let Some(AmbientHint::Prose(prose)) = hint else { + panic!("expected a Prose hint at the date value slot, got {hint:?}"); + }; + assert!( + prose.contains("YYYY-MM-DD"), + "date-column hint references the YYYY-MM-DD format: {prose:?}" + ); +} + +#[test] +fn advanced_update_set_int_value_type_mismatch_is_caught_live() { + // A decimal literal at an `int` column now fails to parse in + // advanced mode (the typed slot's integer validator fires while + // typing) — previously the verbatim sql_expr surface accepted it + // and only Phase 2's execution-time validation caught it. + let schema = schema_cache(&[("Things", &[("k", Type::Int), ("note", Type::Text)])]); + let bad = classify_input_with_schema_in_mode( + "update Things set k = 3.14 where k = 0", + &schema, + Mode::Advanced, + ); + assert!( + !matches!(bad, InputState::Valid), + "a decimal at an int column is rejected live (typed slot), got {bad:?}" + ); + // A well-formed integer literal still parses cleanly. + let ok = classify_input_with_schema_in_mode( + "update Things set k = 5 where k = 0", + &schema, + Mode::Advanced, + ); + assert!(matches!(ok, InputState::Valid), "a valid int literal parses: {ok:?}"); +} + +#[test] +fn advanced_update_set_expression_still_parses_via_sql_expr() { + // Regression guard: the boundary-aware lookahead must fall through + // to sql_expr for anything that is not a lone literal — arithmetic, + // a literal-prefixed expression, a function call, a scalar subquery. + // None of these may be stolen by the typed slot. + let schema = schema_cache(&[ + ("Things", &[("k", Type::Int), ("note", Type::Text)]), + ("other", &[("n", Type::Int)]), + ]); + for input in [ + "update Things set k = 3 + 2 where k = 0", // literal-prefixed expression + "update Things set k = (select max(n) from other) where k = 0", // scalar subquery + "update Things set note = upper(note) where k = 0", // function call + "update Things set k = -5 where k = 0", // signed number → sql_expr + ] { + let state = classify_input_with_schema_in_mode(input, &schema, Mode::Advanced); + assert!( + matches!(state, InputState::Valid), + "{input:?} must still parse via sql_expr, got {state:?}" + ); + } +}