docs(adr): design the DSL→SQL teaching echo (ADR-0038) + dependencies

Realises ADR-0030 §10 (the DSL→SQL teaching bridge) as a /runda'd design
set, before implementation:

- ADR-0037 (new): execution-time mode side-channel — SubmissionMode
  {Simple, Advanced, AdvancedOneShot} threaded Action→worker, output-only;
  redeems ADR-0033 Amendment 3's deferred follow-up. Replay stays silent.
- ADR-0038 (new): the teaching echo + full catalogue (Buckets A/B/C),
  the copy-paste round-trip contract, the three-category framework, and
  the Value→SQL-literal renderer. DDL + show-data centric (overlapping
  DML is SQL-first, so already SQL). Build-order deps recorded.
- ADR-0035 Amendment 2: standard-first dialect stance + ALTER COLUMN
  SET/DROP NOT NULL, SET/DROP DEFAULT, ISO SET DATA TYPE gap-fill.
- ADR-0033 Amendment 4: reclassifies the `update … --all-rows`
  non-fall-back as a bug; it now falls back to the DSL Update and echoes
  (keyed on adjacent `--`; spaced arithmetic preserved).
- ADR-0039 (new): EXPLAIN over advanced SQL — decision recorded, build
  deferred; supersedes ADR-0030 §13 OOS-2.
- ADR-0000: out-of-scope discipline (deferred vs rejected). README index
  updated for all of the above.

Reconcile CLAUDE.md: simple-mode column ops are implemented, not pending
(requirements.md C2/B2 already [x]).
This commit is contained in:
claude@clouddev1
2026-05-27 20:44:38 +00:00
parent 2bcc55f939
commit 9f15f386d5
9 changed files with 775 additions and 8 deletions
+77
View File
@@ -1476,6 +1476,83 @@ forward-references that future ADR so the requirement is not lost.
side-channel until then. No structure built in 3j assumes mode is
irrelevant to execution.
## Amendment 4 — `update … --all-rows` falls back to the DSL; Amendment 3's counter-example was a bug (2026-05-27)
Designing the DSL → SQL teaching echo (ADR-0038) re-examined Amendment 3's
**counter-example** — the claim that `update T set x = 42 --all-rows` in
Advanced mode does *not* fall back to the DSL because the SQL `UPDATE`'s
`SET <expr>` "greedily consumes `--all-rows` as the expression
`42 - -all - rows` (with `all`/`rows` as column refs)." **That is a bug,
not correct behaviour**, and this amendment reverses it. Recorded with
explicit user approval (2026-05-27).
### Why it is a bug
- The walker has **no `--` comment support** (it lexes two minus
operators); the engine *does* treat `--` as a line comment. So the
walker's parse (`42 - -all - rows`) **diverges from execution** (which
runs `update T set x = 42`).
- Absent columns literally named `all` and `rows`, the walker is
**silently accepting a `SET` expression over non-existent columns** —
precisely the case ADR-0027 says to flag ("mark error if we know it
will fail at runtime"). It only *appears* to succeed because the
engine's comment leniency masks the divergence.
- **Trailing SQL comments are not a supported feature** of the surface,
so relying on the engine to treat `--all-rows` as one is wrong.
So the SQL `UPDATE` shape should **not** match `… --all-rows`.
### Decision
The `--all-rows` token sequence makes the **SQL `UPDATE` shape fail**, so
dispatch **falls back to the DSL** `Command::Update { filter: AllRows }`
— restoring symmetry with `delete … --all-rows`, which already falls
back (the SQL `DELETE` has no slot to absorb the flag). Consequences:
- `update … --all-rows` in Advanced mode is the **DSL** command, runs all
rows via the safe DSL path, and (ADR-0038) gains the teaching echo
`UPDATE T SET … ` with the `WHERE` omitted.
- **No `--` comment feature is introduced**; the playground still does
not support trailing SQL comments (consistent with the rest of the
surface, engine-neutral).
### Mechanism — key on the **adjacent `--`**
The DSL `--all-rows` is matched by `Node::Flag("all-rows")` via
`consume_flag` (`walker/lex_helpers.rs`), which requires an **adjacent
`--`**. The SQL expression grammar has no flag node, so it consumes the
same source as `- -all - rows` (two minus operators, `all`/`rows` as
column refs) and the SQL `UPDATE` shape wins (SQL-first). Crucially,
`--all-rows` and a legitimate `- -all` tokenise *identically* once split,
so the **only** robust discriminator is the adjacency of the two dashes:
- **The fix:** the SQL expression treats an **adjacent `--`** as a
boundary it will not consume, so the SQL `UPDATE` shape **fails** there
and dispatch falls back to the DSL, whose `Node::Flag` matches
`--all-rows`. (Adjacency is what separates the DSL flag from real
arithmetic — see the preserved case below.)
Verified consequences (probed against the current grammar), which the
build must lock down test-first:
- `update T set x = 42 --all-rows` → DSL `Update { AllRows }` *(the goal;
inverts `sql_dml_e2e.rs::e2e_update_all_rows_in_advanced_does_not_fall_back_to_dsl`)*.
- `update T set x = 42 - -3` → **unchanged**, stays `SqlUpdate` (= 45):
the dashes are space-separated, not an adjacent `--`. This invariant is
non-negotiable and gets its own test.
- `update T set x = 42--3` and `update T set x = 42 --col` *(adjacent
`--` before a number or a real column)* → today they parse `Ok` as
`SqlUpdate`; after the fix the SQL expression stops at `--` and the DSL
flag grammar (which accepts only `all-rows`) rejects the rest, so they
become **parse errors**. This is a deliberate, acceptable behaviour
change for these contrived adjacent-dash inputs — the playground does
not support `--` line comments, so there is no valid reading of an
adjacent `--` here. *(User heads-up flagged 2026-05-27.)*
The existing `delete … --all-rows` fall-back is unaffected. Folded into
the ADR-0038 echo effort (the fix that makes `update … --all-rows`
echoable).
## See also
- ADR-0005 — the ten-type vocabulary INSERT works with.