380c4238ef
Sub-phase 3k of ADR-0033. Adds the Tier-3 end-to-end DML suite (tests/sql_dml_e2e.rs) and the cross-cut gap-fill tests, fills the verification matrix (every row a verified file::function), and produces the phase-exit report. - tests/sql_dml_e2e.rs: INSERT…SELECT cross-table, all-ten-type multi-row INSERT + RETURNING type recovery, UPDATE-with-subquery-in-SET, cascade DELETE, UPSERT round-trip, RETURNING x3, history.log replay, OOS rejections (full §13 table), validity-indicator-from-SQL-DML. - walker/mod.rs, highlight.rs, completion.rs, input_render.rs: inherited-diagnostic, DML-keyword highlight, INSERT INTO completion, and advanced-mode DML hint-panel cross-cuts. - Matrix correction (user-confirmed): predicate warnings fire on row-scoped DML slots; INSERT VALUES has no row scope (ADR-0033 §8.4). - Auto-snapshot row marked N/A (user-confirmed): ADR-0006 unimplemented for both paths; deferred. /runda round: added an advanced-mode DML hint-panel test (A6 was attributed to simple-mode prose under the §8 advanced heading); extended OOS coverage to the full ADR-0033 §13 table (OOS-5 INDEXED BY / OOS-6 multi-statement) + a trailing-semicolon guard. 1645 passing / 0 failing / 0 skipped / 1 ignored. Clippy clean.
1531 lines
75 KiB
Markdown
1531 lines
75 KiB
Markdown
# Implementation plan — ADR-0033 (ADR-0030 Phase 3)
|
||
|
||
**Plan author date:** 2026-05-20
|
||
**Driving ADR:** [ADR-0033 — The full SQL DML grammar (INSERT / UPDATE / DELETE)](../adr/0033-sql-dml-grammar.md)
|
||
**Parent ADR:** [ADR-0030 — Advanced mode: the standard-SQL surface](../adr/0030-advanced-mode-sql-surface.md)
|
||
|
||
## Purpose
|
||
|
||
ADR-0033 is the *decision*: what Phase 3 covers, how dispatch
|
||
works on shared entry words, what diagnostics land, what the
|
||
execute-path Command variants look like. This plan is the
|
||
*mechanics*: the build order (3a → 3k), the per-sub-phase exit
|
||
gates, the cross-cut verifications that prevent the kind of
|
||
silent gap Phase 1 shipped (ADR-0032 §11.6) and the kind of
|
||
matrix-attribution slippage Phase 2's first DA pass let through
|
||
(handoff-28 §3.4).
|
||
|
||
The plan does **not** re-litigate ADR-0033's decisions. Any
|
||
finding during implementation that suggests a decision should
|
||
change is escalated to the user and resolved by ADR amendment
|
||
(or a new ADR) before code proceeds — never by silent drift.
|
||
|
||
## Why this plan exists
|
||
|
||
Phase 2 closed clean but the closing DA review needed a second
|
||
pass to surface four production gaps. Phase 3 introduces a
|
||
structural risk Phase 2 didn't have: **a new walker capability
|
||
(`Node::Guard`) sits at the foundation**, and every later
|
||
sub-phase depends on it working. If 3a ships broken-but-believed-
|
||
green, 3b–3j build on broken scaffolding. The plan exists to
|
||
make 3a's verification rigorous and to keep every "X comes for
|
||
free" claim from ADR-0033 explicit and tested.
|
||
|
||
The three failure modes the plan is built to prevent:
|
||
|
||
- **The Guard mechanism shipping without proof that it fires
|
||
BEFORE the Seq commits.** R1 in ADR-0033's Open Implementation
|
||
Risks; 3a's DA gate makes this the make-or-break test.
|
||
- **A "free for free" claim shipping without a test that proves
|
||
the claim.** Every "X works for SQL DML automatically" in
|
||
ADR-0030/0031/0032/0033 needs an explicit cross-cut test.
|
||
- **Matrix attribution pointing at a DSL test for a SQL claim.**
|
||
Handoff-28 §3.4: rows pointing at a SQL surface must point at
|
||
a test whose INPUT is SQL syntax. Cross-checked at attribution
|
||
time, not at DA-review time.
|
||
|
||
## Working-mode reminder
|
||
|
||
Per `CLAUDE.md`'s "Working mode — solo with sub-agents by
|
||
default": this work proceeds in solo mode unless the user
|
||
explicitly invokes `/team`. The phased structure below maps to
|
||
solo-mode's Phase 1 → 5 discipline within each sub-phase:
|
||
|
||
- Requirements extraction (the sub-phase's scope below) is
|
||
Phase 1.
|
||
- Divergent exploration is rarely needed for individual
|
||
sub-phases here (ADR-0033 already selected the approach), but
|
||
if implementation reveals an ambiguity, it lands as a Phase-2
|
||
exploration before code proceeds.
|
||
- The DA hat is worn at every sub-phase gate. The DA critique
|
||
is **written** with specific findings BEFORE the verdict; a
|
||
clean PASS without listed findings is a process failure even
|
||
if the work is sound (handoff-28 §4.1; handoff-29 §4.1).
|
||
- Escalation rather than guessing applies throughout. "Out of
|
||
scope" / "non-blocking" require user confirmation
|
||
(handoff-29 §4.2).
|
||
|
||
## Standing authorizations and continuation policy
|
||
|
||
The user granted these authorizations for Phase 2; absent a
|
||
fresh statement to the contrary, they extend to Phase 3
|
||
because the work pattern is identical. They are recorded here
|
||
verbatim from `docs/plans/20260520-adr-0032-phase-2.md` so any
|
||
implementer (the original session, a future session, or a
|
||
fresh agent) operates on the same terms. **If the user has
|
||
narrowed these for Phase 3, the narrowest interpretation
|
||
wins.**
|
||
|
||
### 1. Walk the plan without interruption when no interruption is needed
|
||
|
||
The plan's sub-phases (3a–3k) have explicit, written exit
|
||
gates. Within a sub-phase, the implementer proceeds through
|
||
the build steps and verifies the gate without asking for
|
||
permission at each step. Between sub-phases, the implementer
|
||
proceeds to the next one without asking, *provided* the
|
||
previous gate is green.
|
||
|
||
The implementer does NOT interrupt to:
|
||
|
||
- Confirm "should I continue to the next sub-phase?"
|
||
- Confirm minor implementation choices that match surrounding
|
||
code (`CLAUDE.md`'s code-style discipline applies; pick the
|
||
consistent option and move on).
|
||
- Confirm test naming, file organisation, commit granularity
|
||
within a sub-phase.
|
||
- Report progress mid-step. A status update arrives at gate
|
||
boundaries — not every five minutes.
|
||
|
||
### 2. When the implementer MUST escalate
|
||
|
||
The escalation rule from `CLAUDE.md` ("Escalate ambiguity — do
|
||
not decide for the user") is **not** relaxed. Standing
|
||
authorization to walk uninterrupted is *not* authorization to
|
||
guess on design questions. The implementer escalates if:
|
||
|
||
- An exit gate cannot be made green by work in the current
|
||
sub-phase's scope — a test fails for a reason that suggests
|
||
the ADR is wrong, or a behaviour the gate requires turns out
|
||
to need machinery not in ADR-0033.
|
||
- An ambiguity arises that the ADR and this plan do not settle.
|
||
See "Open questions to escalate before code starts" at the
|
||
end of this plan — those are pre-identified. Anything
|
||
similar that surfaces is also escalation territory.
|
||
- An autonomous design decision is required (a choice outside
|
||
ADR-0033 §§1–13's settled list and the plan's per-sub-phase
|
||
scope). Per `CLAUDE.md`'s "Out of scope and non-blocking
|
||
require user confirmation": the default assumption is that
|
||
the work is in scope and is blocking unless the user has
|
||
said otherwise (handoff-29 §4.2 pin).
|
||
- A "real blocker" is encountered — environment, dependency,
|
||
or test-infrastructure failure that prevents progress and
|
||
cannot be resolved within the sub-phase. A flaky test is
|
||
not a real blocker — investigate per `CLAUDE.md`'s "no
|
||
excuses" rule on test infrastructure.
|
||
|
||
### 3. Commits are pre-authorized at standard granularity
|
||
|
||
The user has explicitly pre-authorized commits within this
|
||
plan's scope, overriding `CLAUDE.md`'s "All commits MUST be
|
||
confirmed by the user first" for this plan. The implementer:
|
||
|
||
- **Commits at natural break points within each sub-phase** and
|
||
always at sub-phase gate boundaries. Granular is better than
|
||
monolithic.
|
||
- **Writes detailed commit messages** in the project's existing
|
||
style — `area: short subject` first line, then a body
|
||
explaining what changed and why. Body should reference the
|
||
ADR section being implemented (e.g., "ADR-0033 §2 — Guard
|
||
mechanism, option (a) chosen") so a reader can trace each
|
||
commit back to a decision.
|
||
- **Includes NO AI attribution** of any form (`Co-Authored-By`,
|
||
generated-by lines, AI footers, …). This is a global rule
|
||
(`CLAUDE.md`); the standing authorization here is for
|
||
*committing*, not for relaxing attribution discipline.
|
||
- **Names the mechanism choice in 3a's commit message** —
|
||
option (a) / (b) / (c) per ADR-0033 §2. If 3a fell back to
|
||
(b) or (c), update ADR-0033 §2's "Default choice" sentence
|
||
in the same commit (per handoff-29 §3 step 6).
|
||
|
||
### 4. Pushes remain a user step
|
||
|
||
`CLAUDE.md`'s "Push is a user step — agents cannot and shall
|
||
never do it" is **not** overridden. The implementer commits
|
||
freely; the user pushes when they choose. Unpushed commits are
|
||
a normal working state, and the implementer never describes
|
||
work as blocked on a push.
|
||
|
||
### 5. End-of-plan handoff
|
||
|
||
At the end of 3k, after the verification report is produced
|
||
and the DA's final PASS verdict is recorded (with specific
|
||
critiques listed first, not a rubber-stamp), the implementer
|
||
**stops and surfaces a summary to the user** — exact test
|
||
counts, the filled cross-cut matrix, the requirements-to-test
|
||
mapping, any autonomous decisions made and their
|
||
justification. The user confirms acceptance before the plan
|
||
is considered done. This is the one explicit hand-back point.
|
||
|
||
If the user wants follow-up work — adjustments, polish,
|
||
additional tests — the implementer addresses it and re-runs
|
||
the gate before re-surfacing. The plan does not declare
|
||
itself complete; the user does.
|
||
|
||
## Sub-phase overview
|
||
|
||
| Sub-phase | Title | Test-suite gate |
|
||
|-----------|------------------------------------------------|----------------------------------------------------------------------------------|
|
||
| **3a** | `Node::Guard` + mode-gated Choice mechanism | Smoke-test grammar: 4 mode × shape combinations green; R1 invariant proven |
|
||
| **3b** | INSERT grammar + minimal execution | Single + multi-row VALUES end-to-end; CSV re-persisted; `__rdbms_*` rejected |
|
||
| **3c** | INSERT … SELECT | Compound row source + WITH-prefixed SELECT row source end-to-end |
|
||
| **3d** | `shortid` auto-fill (worker) | Auto-fill on VALUES (single/multi) + INSERT…SELECT; override warning path |
|
||
| **3e** | UPDATE grammar + execution | Multi-column SET, with/without WHERE; sql_expr in SET works |
|
||
| **3f** | DELETE grammar + execution + cascade summary | Cascade parity with DSL; WHERE-with-subquery case (R2) |
|
||
| **3g** | RETURNING | All three statement kinds; result-column type recovery via Phase-2 path |
|
||
| **3h** | UPSERT (`ON CONFLICT … DO NOTHING / DO UPDATE`) | Both branches; `excluded.col` resolves to target table's columns |
|
||
| **3i** | Diagnostics | Three new keys: positive + negative each; per-row arity emit verified |
|
||
| **3j** | Dispatch wiring on shared entry words | Existing DSL tests still green; SQL inputs route via shared CommandNode |
|
||
| **3k** | Verification sweep | Cross-cut matrix every row green; phase-exit verification report |
|
||
|
||
Each sub-phase ships independently: green tests, clippy clean,
|
||
a written DA gate review with specific critiques listed first,
|
||
and a commit (pre-authorized per §3 above).
|
||
|
||
---
|
||
|
||
## Sub-phase 3a — `Node::Guard` + mode-gated Choice mechanism
|
||
|
||
**This sub-phase is the foundation.** If the Guard mechanism
|
||
doesn't work cleanly, ADR-0033 §2's mitigation paths
|
||
(mechanism options (b) or (c)) get evaluated **before** any
|
||
DML grammar lands on top.
|
||
|
||
### Scope (in)
|
||
|
||
- Add a new walker node variant
|
||
`Node::Guard(fn(&WalkContext) -> Result<(), ValidationError>)`
|
||
to `src/dsl/grammar/mod.rs::Node` — zero-byte consumption,
|
||
fails the enclosing Seq with `ValidationFailed` when the
|
||
validator returns Err.
|
||
- Implement `walk_guard` in `src/dsl/walker/driver.rs`. Match
|
||
arm in `walk_node_inner`.
|
||
- Add the `reject_sql_in_simple_mode` guard function. Returns
|
||
`ValidationError { message_key: "advanced_mode.sql_in_simple",
|
||
args: … }` in Simple mode, `Ok(())` in Advanced.
|
||
- Add the catalog key `advanced_mode.sql_in_simple` with
|
||
engine-neutral wording matching the existing `select` /
|
||
`with` gating pattern (ADR-0030 §2).
|
||
- Build the smoke-test grammar: a single experimental
|
||
`CommandNode` whose `shape` is
|
||
`Choice([Seq[Guard(reject_sql_in_simple_mode),
|
||
SQL_marker_token], DSL_marker_token])` using two
|
||
distinguishable, non-DML marker tokens (e.g., synthetic
|
||
`__sql_marker` / `__dsl_marker`) so the smoke test exercises
|
||
the mechanism without depending on any DML grammar.
|
||
- Verify the four mode × input cases listed in Exit gate
|
||
below.
|
||
|
||
### Scope (out — explicit)
|
||
|
||
- Any real DML grammar — that is 3b onwards.
|
||
- Wiring `data::INSERT/UPDATE/DELETE` shapes — that is 3j.
|
||
- Any other guard-based gating beyond `reject_sql_in_simple_mode`
|
||
— the mechanism is reusable but only this guard is
|
||
required by this sub-phase.
|
||
|
||
### Build steps
|
||
|
||
1. Add `Node::Guard` variant. Cover the match-arm in every
|
||
exhaustive `match Node` in the walker / grammar / hint
|
||
helpers — clippy will surface incomplete arms.
|
||
2. Implement `walk_guard` in the driver. Push zero bytes;
|
||
return `WalkOutcome::ValidationFailed` from the Err path.
|
||
3. Author `reject_sql_in_simple_mode` in
|
||
`src/dsl/grammar/mod.rs` (or a sibling module if it fits
|
||
better there).
|
||
4. Author the catalog key + an engine-neutral i18n message.
|
||
5. Build the smoke-test grammar in a temporary file
|
||
(`src/dsl/grammar/_guard_smoke.rs` or similar; remove
|
||
before 3a's commit, OR keep it as a unit-test fixture in
|
||
the walker driver tests — implementer's choice as long as
|
||
it doesn't leak into the runtime CommandNode registry).
|
||
6. Author the four exit-gate tests.
|
||
7. Author the R1 invariant test (DA gate below).
|
||
8. `cargo test`, `cargo clippy --all-targets -- -D warnings`.
|
||
|
||
### Exit gate
|
||
|
||
**Required green tests:**
|
||
|
||
- **Smoke-test grammar — Simple mode + DSL-shape input:** the
|
||
DSL branch matches. (Baseline: Choice fall-through works.)
|
||
- **Smoke-test grammar — Advanced mode + SQL-shape input:**
|
||
the SQL branch matches.
|
||
- **Smoke-test grammar — Simple mode + SQL-shape input:**
|
||
`WalkOutcome::ValidationFailed` with
|
||
`message_key: "advanced_mode.sql_in_simple"`. (The Guard
|
||
fires; no other branch is attempted because the SQL marker
|
||
consumed bytes after the Guard pushed empty.)
|
||
- **Smoke-test grammar — Advanced mode + DSL-shape input:**
|
||
the DSL branch matches. (Choice fall-through works even
|
||
when the SQL branch is admissible — the Guard returned Ok
|
||
but the marker token didn't match the input, so the SQL
|
||
Seq fails NoMatch and Choice falls through to DSL.)
|
||
|
||
**R1 invariant test (this is the make-or-break case):**
|
||
|
||
- **Advanced mode + an input whose first tokens match the SQL
|
||
branch's Guard-then-marker pattern but the SQL branch's
|
||
*later* tokens fail, while the DSL branch's tokens would
|
||
match the full input.** The expected outcome is:
|
||
- If the Choice falls through to DSL → R1 mechanism (a)
|
||
works as designed; 3a is green.
|
||
- If the Choice commits to SQL and reports Failed → R1
|
||
mechanism (a) is broken in the same way ADR-0033 §2's
|
||
open risk anticipated; sub-phase REOPENS with mechanism
|
||
option (b) or (c).
|
||
|
||
Note: in 3a's smoke-test grammar the SQL and DSL markers
|
||
are different, so the simplest exercise of this invariant
|
||
uses a 2-token SQL branch (`SQL_marker_A`, `SQL_tail_X`)
|
||
and an input `SQL_marker_A SQL_tail_Y` where `SQL_tail_Y`
|
||
isn't admissible to the SQL branch but is admissible to
|
||
the DSL branch. The Choice MUST fall through.
|
||
|
||
**Other gates:**
|
||
|
||
- `cargo test` total: 1446 (baseline) + the new smoke-test +
|
||
R1 invariant tests; expect ≥ 1450 (rough order).
|
||
- Zero failures, one ignored (the unchanged doctest), zero
|
||
skipped.
|
||
- `cargo clippy --all-targets -- -D warnings` clean.
|
||
|
||
### DA gate (written)
|
||
|
||
The DA hat is worn explicitly here. Specific critiques listed
|
||
first, verdict last (handoff-29 §4.1 pin):
|
||
|
||
- **Does the Guard actually fire BEFORE the Seq commits?**
|
||
Read `walk_seq` / `walk_choice` in
|
||
`src/dsl/walker/driver.rs`. Trace the path: Choice picks
|
||
branch 0 → walk_seq enters → walk_guard runs → if Err,
|
||
Seq's outcome is Failed → does walk_choice see Failed and
|
||
fall through to branch 1, or does it commit on branch
|
||
0's "tried but failed"? The R1 invariant test above is
|
||
the empirical answer; the DA reads the code and confirms
|
||
the test's assertion matches the code path.
|
||
- **Is the Guard mechanism reusable beyond DML?** Phase 4
|
||
(DDL) is the likely future consumer. If yes, document the
|
||
pattern in `src/dsl/grammar/mod.rs` doc comments so the
|
||
next phase doesn't reinvent it.
|
||
- **Does `reject_sql_in_simple_mode`'s catalog key match the
|
||
existing `select` / `with` entry-word gate's wording
|
||
posture?** Inconsistent UX between entry-word gates and
|
||
branch gates is a real gap; the DA reads
|
||
`src/dsl/grammar/mod.rs::ADVANCED_ONLY_ENTRIES` and
|
||
confirms the catalog key + i18n string land at the same
|
||
user-facing message shape.
|
||
- **Did 3a introduce any structure ADR-0033 §2 doesn't
|
||
sanction?** A new node variant added; a new validator
|
||
signature shape; a new catalog key. If anything beyond
|
||
these landed, that's an autonomous decision to surface.
|
||
- **If the R1 invariant test fails, was the fallback path
|
||
taken cleanly?** The fallback path means: (a) ADR-0033 §2
|
||
amended with the chosen mechanism, (b) the smoke-test
|
||
grammar rewritten on (b) or (c), (c) the gate's tests
|
||
pass on the new mechanism, (d) the commit message names
|
||
the mechanism that landed.
|
||
|
||
---
|
||
|
||
## Sub-phase 3b — INSERT grammar + minimal execution
|
||
|
||
### Scope (in)
|
||
|
||
- Author `src/dsl/grammar/sql_insert.rs` exporting
|
||
`pub static SQL_INSERT_SHAPE: Node` (the per ADR-0033 §1
|
||
shape minus row-source SELECT, RETURNING, UPSERT — those
|
||
land in 3c/3g/3h).
|
||
- The shape covers: `INSERT INTO table_name
|
||
[ '(' column_name_list ')' ] VALUES value_tuple ( ',' value_tuple )* [ ';' ]`.
|
||
- The `__rdbms_*` rejection on the table-name slot (per
|
||
ADR-0030 §6 / ADR-0032 §1's `reject_internal_table`
|
||
validator).
|
||
- `Command::SqlInsert { sql, source, target_table,
|
||
listed_columns, returning: false }` — `returning` always
|
||
false in 3b.
|
||
- `Request::RunSqlInsert` + `do_sql_insert` worker handler
|
||
(no shortid auto-fill yet — explicit values required for
|
||
every column).
|
||
- Persistence write-through on the target table after
|
||
execution (ADR-0030 §11).
|
||
- History-log of the literal submitted line (ADR-0030 §11).
|
||
- A standalone development CommandNode (e.g., `sql_insert`)
|
||
in the registry so 3b–3i can exercise the SQL INSERT path
|
||
without depending on 3j's dispatch wiring on shared entry
|
||
words. Removed in 3j.
|
||
|
||
### Scope (out — explicit)
|
||
|
||
- `INSERT … SELECT` — that is 3c.
|
||
- `RETURNING` — that is 3g.
|
||
- `UPSERT` — that is 3h.
|
||
- `shortid` auto-fill — that is 3d.
|
||
- Walker diagnostics specific to INSERT (`insert_arity_mismatch`,
|
||
`auto_column_overridden`, `not_null_missing`) — that is 3i.
|
||
- DSL-vs-SQL dispatch wiring on `data::INSERT` — that is 3j.
|
||
|
||
### Build steps
|
||
|
||
1. Author `sql_insert.rs` with the value-list shape, plus
|
||
reject-internal-table on the target slot.
|
||
2. Add `Command::SqlInsert` with the field list above.
|
||
3. Add `Request::RunSqlInsert` and `do_sql_insert`.
|
||
4. Wire history-log write-before-execute (mirror
|
||
`do_run_select`'s pattern).
|
||
5. Wire persistence write-through after execute.
|
||
6. Build the development CommandNode (`sql_insert`) and
|
||
register it.
|
||
7. Unit + integration tests per the exit gate.
|
||
|
||
### Exit gate
|
||
|
||
**Required green tests:**
|
||
|
||
- **Single-row INSERT runs end-to-end** —
|
||
`INSERT INTO orders (customer_id, total) VALUES (1, 99.50)`
|
||
succeeds; affected-row count surfaces; CSV is re-persisted
|
||
with the new row.
|
||
- **Multi-row INSERT runs end-to-end** —
|
||
`INSERT INTO orders VALUES (1, 'a'), (2, 'b')` succeeds;
|
||
both rows land; CSV reflects both.
|
||
- **Column-list variant** —
|
||
`INSERT INTO t (a, b) VALUES (1, 'x')` succeeds; the
|
||
unmentioned columns get their defaults / NULLs.
|
||
- **No column-list variant** —
|
||
`INSERT INTO t VALUES (…)` accepts the table's full column
|
||
arity in the tuple.
|
||
- **`__rdbms_*` rejection** at the INSERT target-table slot
|
||
(per ADR-0030 §6) — parse-rejects.
|
||
- **History line** is the literal submitted SQL.
|
||
- **Failed INSERT (engine error)** does NOT re-persist the
|
||
CSV — invariant: persistence happens only on engine
|
||
success.
|
||
- All Phase-2 tests (1446 baseline) still green.
|
||
|
||
**Other gates:**
|
||
|
||
- `cargo clippy --all-targets -- -D warnings` clean.
|
||
|
||
### DA gate (written)
|
||
|
||
- **Does an INSERT failing mid-multi-row leave persistence in
|
||
a consistent state?** SQLite's default is per-statement
|
||
transactional — verify the worker doesn't re-persist when
|
||
the engine returns an error.
|
||
- **Does the `target_table` extraction handle case-folding
|
||
the same way the rest of the codebase does?** ADR-0009:
|
||
identifiers are case-preserving but ADR-0012's metadata
|
||
uses canonical casing for lookups. Verify the path.
|
||
- **Are the existing DSL INSERT tests still green?** Any
|
||
DSL-side regression means 3b broke something it shouldn't
|
||
have. Run the DSL INSERT test module explicitly.
|
||
- **Does the history-log line capture pre-engine state?**
|
||
ADR-0030 §11: the literal submitted line is logged before
|
||
execution. Verify the order (log → execute) and that a
|
||
failing INSERT still leaves a log entry (for replay).
|
||
- **Did 3b introduce any structure ADR-0033 §1 doesn't
|
||
sanction?**
|
||
|
||
---
|
||
|
||
## Sub-phase 3c — INSERT … SELECT
|
||
|
||
### Scope (in)
|
||
|
||
- Extend `SQL_INSERT_SHAPE`'s row-source slot to be a Choice
|
||
between `values_clause` and
|
||
`Subgrammar(&sql_select::SQL_SELECT_COMPOUND)`.
|
||
- The Phase-2 SELECT machinery applies for free — CTEs
|
||
(Amendment 2 admits leading `WITH`), JOINs, set ops,
|
||
subqueries, scope-frame harvest.
|
||
- `do_sql_insert` handles the SELECT-driven path; the engine
|
||
handles the insert-from-query flow (no special composite
|
||
preparation needed).
|
||
|
||
### Scope (out — explicit)
|
||
|
||
- Arity diagnostic (`insert_arity_mismatch`) — that is 3i;
|
||
3c's only verification of arity is what the engine reports
|
||
via the friendly layer.
|
||
- Schema-existence on the SELECT's projection — comes for
|
||
free from Phase-2 machinery (ADR-0032 §11); 3c's job is to
|
||
cross-cut-verify it fires on a DML SELECT slot, not to
|
||
re-implement.
|
||
|
||
### Build steps
|
||
|
||
1. Wrap the existing values_clause in a Choice with the
|
||
subgrammar reference.
|
||
2. Verify the development CommandNode admits both row
|
||
sources.
|
||
3. Tests per the exit gate, including the WITH-prefixed
|
||
case (R4 invariant).
|
||
|
||
### Exit gate
|
||
|
||
**Required green tests:**
|
||
|
||
- **Plain INSERT … SELECT** —
|
||
`INSERT INTO archive SELECT * FROM orders WHERE created < '2025-01-01'`
|
||
runs; the rows land in archive; archive's CSV reflects
|
||
them.
|
||
- **Column-list + projection** —
|
||
`INSERT INTO target (a, b) SELECT x, y FROM source`
|
||
runs.
|
||
- **WITH-prefixed SELECT row source (R4)** —
|
||
`INSERT INTO archive WITH t AS (SELECT * FROM orders)
|
||
SELECT * FROM t` runs. (This proves the Amendment-2
|
||
carry-through.)
|
||
- **Schema-existence diagnostic on the SELECT's projection
|
||
(cross-cut)** —
|
||
`INSERT INTO archive SELECT nonexistent_col FROM orders`
|
||
fires `diagnostic.unknown_column` (or the equivalent
|
||
Phase-2 key) on the projection — the Phase-2 pass
|
||
applies here for free; the test pins the claim.
|
||
- All 3a–3b tests still green; baseline 1446 still green.
|
||
|
||
**Other gates:**
|
||
|
||
- `cargo clippy --all-targets -- -D warnings` clean.
|
||
|
||
### DA gate (written)
|
||
|
||
- **Does the WITH-prefixed SELECT row source actually parse
|
||
through `SQL_SELECT_COMPOUND`?** Or does the INSERT prefix
|
||
break the Phase-2 recursion? The R4 test above pins this;
|
||
the DA reads `sql_select::SQL_SELECT_COMPOUND` to confirm
|
||
the entry shape admits `WITH`.
|
||
- **Does the cascade-summary path (3f DELETE) accidentally
|
||
fire for INSERTs whose SELECT touches the target table?**
|
||
It must not — INSERTs have no cascade output. 3c's tests
|
||
shouldn't see a cascade summary even when the source is
|
||
the same as the target.
|
||
- **Does a SELECT-driven INSERT respect the
|
||
`__rdbms_*` rejection on the SELECT's `FROM` slot?**
|
||
Phase-2 already gates this; 3c verifies it doesn't
|
||
silently regress on the DML path.
|
||
|
||
---
|
||
|
||
## Sub-phase 3d — `shortid` auto-fill (worker)
|
||
|
||
### Scope (in)
|
||
|
||
- Worker-side post-fill in `do_sql_insert` (per ADR-0033 §6).
|
||
- Detect omitted `shortid` columns by intersecting the
|
||
user's `listed_columns` against the target table's
|
||
auto-gen column list from `__rdbms_playground_columns`.
|
||
- Synthesise fresh shortids per row (the existing
|
||
`shortid::generate` path).
|
||
- Rewrite the executed SQL to bind the synthesised values
|
||
positionally (or use parameter binding — implementer's
|
||
choice, but the original submitted SQL stays unchanged in
|
||
`history.log`).
|
||
- Cover both row sources: VALUES (single + multi-row) and
|
||
SELECT (each yielded row gets a fresh shortid for the
|
||
omitted auto-gen columns).
|
||
|
||
### Scope (out — explicit)
|
||
|
||
- The `auto_column_overridden` walker WARNING — that is 3i.
|
||
- `serial` auto-fill — handled by SQLite rowid; the worker
|
||
has no work to do for `serial` columns.
|
||
|
||
### Build steps
|
||
|
||
1. Locate the existing DSL `do_insert`'s shortid auto-fill
|
||
logic. Refactor as needed so the SQL path can call the
|
||
same generator (the generator itself, not the surrounding
|
||
typed `Command::Insert` plumbing).
|
||
2. In `do_sql_insert`, after parsing `listed_columns` vs
|
||
table schema, identify omitted auto-gen columns.
|
||
3. For VALUES, synthesise per-row shortids and bind them.
|
||
4. For SELECT, the strategy needs an explicit design call:
|
||
- **Option A:** Rewrite SQL to wrap the SELECT, projecting
|
||
a synthesised shortid alongside (heavier; touches the
|
||
SQL text).
|
||
- **Option B:** Run the SELECT first into a result set,
|
||
synthesise per-row shortids in the worker, then INSERT
|
||
the augmented rows row-by-row (simpler; loses the
|
||
engine's set-based optimisation).
|
||
- **ESCALATE this choice to the user.** Both options are
|
||
valid; ADR-0033 §6 doesn't pick. Plan default if the
|
||
user is unavailable: **Option B** (simpler, matches the
|
||
pedagogical posture; performance is not a goal).
|
||
5. Unit + integration tests per the exit gate.
|
||
|
||
### Exit gate
|
||
|
||
**Required green tests:**
|
||
|
||
- **VALUES single-row auto-fill** —
|
||
`INSERT INTO t (name) VALUES ('x')` where `t` has `id
|
||
shortid pk` auto-fills `id` with a fresh shortid;
|
||
`SELECT * FROM t` shows the synthesised id.
|
||
- **VALUES multi-row distinct shortids** —
|
||
`INSERT INTO t (name) VALUES ('a'), ('b'), ('c')`
|
||
yields three rows with three DISTINCT shortid values.
|
||
- **Explicit value respected (override)** —
|
||
`INSERT INTO t (id, name) VALUES ('hardcoded', 'x')`
|
||
preserves `'hardcoded'`; the warning fires in 3i but
|
||
the value is honoured here.
|
||
- **INSERT … SELECT auto-fills shortids** —
|
||
`INSERT INTO target (name) SELECT name FROM source`
|
||
where `target.id` is shortid produces fresh shortids
|
||
for every yielded row (verified distinct).
|
||
- **Combined `serial` + `shortid` columns** —
|
||
on a table with both, `serial` is engine-filled, `shortid`
|
||
is worker-filled, no collision.
|
||
- All 3a–3c tests still green; baseline 1446 still green.
|
||
|
||
**Other gates:**
|
||
|
||
- `cargo clippy --all-targets -- -D warnings` clean.
|
||
|
||
### DA gate (written)
|
||
|
||
- **Are the synthesised shortids actually unique across a
|
||
multi-row INSERT?** The generator must be called per-row,
|
||
not once and reused. The multi-row distinct test pins
|
||
this.
|
||
- **Does the auto-fill respect the column's case-preserved
|
||
name?** ADR-0009. Verify with a mixed-case shortid column
|
||
name.
|
||
- **What happens if the table has both `serial` and
|
||
`shortid` PK columns?** Spec: serial via SQLite rowid;
|
||
shortid via worker post-fill. Verify no double-fill
|
||
collision and no double-insertion.
|
||
- **Did the SELECT row-source path choose option A or B,
|
||
and was it escalated to the user before landing?** If the
|
||
implementer chose without escalating, that's an autonomous
|
||
decision and 3d's DA verdict is FAIL until the user
|
||
confirms.
|
||
|
||
---
|
||
|
||
## Sub-phase 3e — UPDATE grammar + execution
|
||
|
||
### Scope (in)
|
||
|
||
- Author `src/dsl/grammar/sql_update.rs` exporting
|
||
`pub static SQL_UPDATE_SHAPE: Node`:
|
||
`UPDATE table_name SET assignment_list [ where_clause ] [ ';' ]`.
|
||
- The `__rdbms_*` rejection on the target slot.
|
||
- `assignment_list` reuses `sql_expr::SQL_OR_EXPR` for the
|
||
RHS of each assignment.
|
||
- `Command::SqlUpdate`, `Request::RunSqlUpdate`,
|
||
`do_sql_update`.
|
||
- Persistence write-through on the target table.
|
||
- A development CommandNode `sql_update` until 3j.
|
||
- No `--all-rows` rail (ADR-0030 §12: SQL UPDATE without
|
||
WHERE executes as written).
|
||
|
||
### Scope (out — explicit)
|
||
|
||
- `RETURNING` — that is 3g.
|
||
- `UPDATE … FROM other_table` — OOS-3 in ADR-0033 §13.
|
||
- Walker WARNING on UPDATE-touching-auto-gen-column — that
|
||
warning lands in 3i if the implementer extends
|
||
`auto_column_overridden`'s scope; the spec leaves it as an
|
||
INSERT-only diagnostic per ADR-0033 §8.2. If the
|
||
implementer wants to extend, escalate.
|
||
|
||
### Build steps
|
||
|
||
1. Author the shape.
|
||
2. Add Command/Request/handler.
|
||
3. Wire history-log + persistence.
|
||
4. Register the dev CommandNode.
|
||
5. Tests per the exit gate.
|
||
|
||
### Exit gate
|
||
|
||
**Required green tests:**
|
||
|
||
- **Single-column UPDATE with WHERE** —
|
||
`UPDATE t SET v = 'x' WHERE id = 1` runs; affected-row
|
||
count surfaces; CSV reflects the change.
|
||
- **Multi-column UPDATE** —
|
||
`UPDATE t SET a = 1, b = 2 WHERE id = 1` runs.
|
||
- **UPDATE without WHERE** —
|
||
`UPDATE t SET active = false` runs across all rows (per
|
||
ADR-0030 §12, no `--all-rows` gate); CSV reflects all
|
||
rows changed.
|
||
- **UPDATE with sql_expr in SET** —
|
||
`UPDATE t SET total = price * quantity WHERE …` runs;
|
||
the engine evaluates the expression.
|
||
- **Schema-existence diagnostic fires** on unknown columns
|
||
in SET (cross-cut from Phase-2 machinery).
|
||
- **Predicate warning fires** on a bad WHERE
|
||
(cross-cut from Phase-2 machinery — `= NULL` in the
|
||
WHERE.)
|
||
- All 3a–3d tests still green; baseline 1446 still green.
|
||
|
||
**Other gates:**
|
||
|
||
- `cargo clippy --all-targets -- -D warnings` clean.
|
||
|
||
### DA gate (written)
|
||
|
||
- **Does the SET assignment expression accept the full
|
||
`sql_expr` (function calls, subqueries, CASE)?** Verify
|
||
with `SET v = (SELECT max(other) FROM other_table)` and
|
||
`SET v = CASE WHEN x > 0 THEN x ELSE 0 END`.
|
||
- **Does an UPDATE without WHERE actually run as written
|
||
with no rail?** Per ADR-0030 §12. The test pins the claim.
|
||
- **Does the persistence write-through fire even when
|
||
affected-row count is 0?** Edge case: an UPDATE that
|
||
matches no rows. Worker should still treat the operation
|
||
as successful (engine returned OK); CSV doesn't need to
|
||
rewrite if nothing changed, but the path mustn't crash.
|
||
- **Did 3e regress any DSL UPDATE behaviour?**
|
||
|
||
---
|
||
|
||
## Sub-phase 3f — DELETE grammar + execution + cascade summary
|
||
|
||
### Scope (in)
|
||
|
||
- Author `src/dsl/grammar/sql_delete.rs` exporting
|
||
`pub static SQL_DELETE_SHAPE: Node`:
|
||
`DELETE FROM table_name [ where_clause ] [ ';' ]`.
|
||
- The `__rdbms_*` rejection on the target slot.
|
||
- `Command::SqlDelete`, `Request::RunSqlDelete`,
|
||
`do_sql_delete`.
|
||
- Cascade-summary pre-count per ADR-0033 §7: extract WHERE
|
||
bytes from the matched path, inject into pre-count
|
||
subqueries for each child table FK with `ON DELETE
|
||
CASCADE` / `SET NULL`.
|
||
- Cascade-summary formatter shared with `do_delete` (DSL
|
||
path) — one formatter, two callers (ADR-0033 §7's
|
||
shared-formatter promise).
|
||
- Persistence write-through on target + every cascade-
|
||
affected child table (multi-table persistence is new for
|
||
SQL DML; the DSL path already does this).
|
||
- Development CommandNode `sql_delete` until 3j.
|
||
|
||
### Scope (out — explicit)
|
||
|
||
- `RETURNING` — that is 3g; 3f leaves the cascade summary +
|
||
affected-count as the only output for now.
|
||
- `DELETE … FROM other_table` — not a thing in standard
|
||
SQL; OOS by silence.
|
||
|
||
### Build steps
|
||
|
||
1. Author the shape.
|
||
2. Extract the WHERE-byte-range capture in the walker's
|
||
ast-builder for `SQL_DELETE_SHAPE` — the matched path
|
||
spans the WHERE node; the ast-builder pulls the source
|
||
bytes covered.
|
||
3. Refactor `format_cascade_summary` from `do_delete` so it
|
||
accepts both the DSL typed-Expr path and a string-WHERE
|
||
path. Implementer's call whether to extract via two
|
||
pre-count strategies or one unified pre-count constructor
|
||
that accepts either input form. Plan default: extract the
|
||
pre-count constructor, keep formatting unified.
|
||
4. Wire `do_sql_delete`: pre-count → execute → format
|
||
summary → re-persist target + every child whose row
|
||
count changed.
|
||
5. Tests per the exit gate, including R2's WHERE-with-
|
||
subquery case.
|
||
|
||
### Exit gate
|
||
|
||
**Required green tests:**
|
||
|
||
- **Plain DELETE with WHERE** —
|
||
`DELETE FROM orders WHERE id = 1` runs; affected-row
|
||
count + cascade summary surface; target CSV re-persisted.
|
||
- **DELETE without WHERE** —
|
||
`DELETE FROM orders` runs across all rows; cascade
|
||
summary covers all child rows; target + every cascade-
|
||
affected child CSV re-persisted.
|
||
- **Cascade parity with DSL** — on the same schema and
|
||
data, running `DELETE FROM customers WHERE id = 1` via
|
||
the SQL path produces the same per-relationship summary
|
||
as running the DSL `delete customers where id = 1`.
|
||
- **R2 invariant: WHERE-with-subquery** —
|
||
`DELETE FROM orders WHERE customer_id IN (SELECT id FROM
|
||
customers WHERE country = 'DE')` runs; the cascade pre-
|
||
count uses the same WHERE bytes (the subquery runs N+1
|
||
times — once for the count, once for the DELETE; the
|
||
test pins correctness, not performance).
|
||
- **Cascade pre-count runs BEFORE execute** — verify order
|
||
by exercising a case where the pre-count would yield 0
|
||
if it ran after the DELETE (i.e., the rows being deleted
|
||
are exactly the ones whose children are pre-counted).
|
||
- **Cascade-affected children CSVs re-persisted** — after
|
||
a cascading DELETE, every affected child table's CSV
|
||
reflects the post-state (rows gone for `ON DELETE
|
||
CASCADE`, NULL for `SET NULL`).
|
||
- **`__rdbms_*` rejection** at the DELETE target slot.
|
||
- All 3a–3e tests still green; baseline 1446 still green.
|
||
|
||
**Other gates:**
|
||
|
||
- `cargo clippy --all-targets -- -D warnings` clean.
|
||
|
||
### DA gate (written)
|
||
|
||
- **R2: does the predicate-extraction byte-range injection
|
||
work on a WHERE containing a subquery?** Test
|
||
`DELETE FROM a WHERE id IN (SELECT id FROM b WHERE …)`.
|
||
The pre-count SQL becomes
|
||
`SELECT count(*) FROM child WHERE fk_col IN (SELECT pk_col
|
||
FROM a WHERE id IN (SELECT id FROM b WHERE …))`. The
|
||
subquery runs twice (R2's noted performance concern).
|
||
The DA reads `do_sql_delete`'s pre-count construction to
|
||
confirm the byte range covers the entire WHERE (including
|
||
the inner subquery) and not just the top-level predicate.
|
||
- **Does the cascade pre-count run in a way that doesn't
|
||
see the post-DELETE state?** Order matters: pre-count
|
||
BEFORE execute. Verified by the order test above; DA
|
||
reads the code to confirm.
|
||
- **Is the cascade-summary formatter actually shared?** No
|
||
duplicate copy of the formatter — one function, two
|
||
callers (DSL + SQL).
|
||
- **Are cascade-affected child CSVs identified by querying
|
||
the metadata tables for `ON DELETE CASCADE` / `SET NULL`
|
||
relationships, or by walking the result of the cascade?**
|
||
Implementer's choice; DA confirms whichever path is
|
||
taken is sound and matches DSL `do_delete`.
|
||
|
||
---
|
||
|
||
## Sub-phase 3g — RETURNING
|
||
|
||
### Scope (in)
|
||
|
||
- Add `RETURNING projection_list` as an optional tail on
|
||
all three SQL DML shapes (SQL_INSERT_SHAPE,
|
||
SQL_UPDATE_SHAPE, SQL_DELETE_SHAPE).
|
||
- `projection_list` is the Phase-2 projection list,
|
||
imported unchanged.
|
||
- Walker's `ast_builder` for each shape sets
|
||
`returning: bool` true when the RETURNING tail matched.
|
||
- Worker handlers branch on `returning`:
|
||
- `true` → prepare + step + collect rows into a
|
||
`DataResult`. Reuse the SELECT result-column type
|
||
recovery (`resolve_select_column_types` in `db.rs`,
|
||
ADR-0032 §12 + Amendment 1).
|
||
- `false` → execute, collect affected-row count
|
||
(current 3b/3e/3f behaviour).
|
||
- DELETE … RETURNING preserves the cascade summary
|
||
alongside the result set — both surface to the user.
|
||
|
||
### Scope (out — explicit)
|
||
|
||
- The Command variant shape changes — `returning: bool` is
|
||
already in the spec (ADR-0033 §10); 3b set it false, 3g
|
||
flips it.
|
||
|
||
### Build steps
|
||
|
||
1. Add the RETURNING tail to each shape.
|
||
2. Update each ast_builder to set `returning`.
|
||
3. Update each worker handler to branch.
|
||
4. Reuse `resolve_select_column_types` for the prepared
|
||
statement's column-origin metadata.
|
||
5. For DELETE + RETURNING: ensure cascade pre-count still
|
||
runs and its summary surfaces alongside the rows.
|
||
6. Tests per the exit gate.
|
||
|
||
### Exit gate
|
||
|
||
**Required green tests:**
|
||
|
||
- **INSERT … RETURNING \*** —
|
||
`INSERT INTO t (a) VALUES (1) RETURNING *` returns the
|
||
inserted row as a DataResult; affected-row count is
|
||
implicit (rows.len() = 1).
|
||
- **INSERT multi-row … RETURNING id** —
|
||
`INSERT INTO t (a) VALUES (1), (2), (3) RETURNING id`
|
||
returns three rows; ids distinct.
|
||
- **UPDATE … RETURNING id, new_value** —
|
||
`UPDATE t SET v = 'x' WHERE id = 1 RETURNING id, v`
|
||
returns the modified columns of the affected row.
|
||
- **DELETE … RETURNING \*** —
|
||
`DELETE FROM t WHERE id = 1 RETURNING *` returns the
|
||
pre-DELETE row (BEFORE it's gone).
|
||
- **DELETE … RETURNING + cascade summary** —
|
||
on a parent DELETE with cascade children, the user sees
|
||
both the RETURNING result set AND the cascade
|
||
per-relationship summary.
|
||
- **Result-column type recovery on RETURNING** — for each
|
||
of the ten playground types, a bare-column RETURNING
|
||
reference recovers the playground type via the Phase-2
|
||
column-origin path. (At least one test per type; bundle
|
||
into a single parametric test for compactness.)
|
||
- **Computed expression in RETURNING stays typeless** —
|
||
`RETURNING a + 1` yields `column_types[0] = None`.
|
||
- **R3: RETURNING on multi-row INSERT** — column-origin is
|
||
the same for all yielded rows (per ADR-0032 Amendment 1);
|
||
one test pins it.
|
||
- All 3a–3f tests still green; baseline 1446 still green.
|
||
|
||
**Other gates:**
|
||
|
||
- `cargo clippy --all-targets -- -D warnings` clean.
|
||
|
||
### DA gate (written)
|
||
|
||
- **Does the RETURNING result-set rendering use the same
|
||
DataResult path SELECT uses?** Or did a DML-specific
|
||
copy sneak in? Read the renderer; confirm one path, not
|
||
two.
|
||
- **Does `RETURNING expr AS alias` admit aliases the same
|
||
way the projection list does?** The projection list is
|
||
Phase-2 machinery; 3g imports unchanged. Verify via a
|
||
test.
|
||
- **For DELETE + RETURNING, does the cascade pre-count
|
||
fire correctly?** The cascade summary is per-relationship
|
||
child rows affected — independent of whether the user
|
||
asked for RETURNING. Both must surface.
|
||
- **Result-column type recovery test count: ten types?**
|
||
All ten playground types covered (text, int, real,
|
||
decimal, bool, date, datetime, blob, serial, shortid).
|
||
|
||
---
|
||
|
||
## Sub-phase 3h — UPSERT (`ON CONFLICT … DO NOTHING / DO UPDATE`)
|
||
|
||
### Scope (in)
|
||
|
||
- Add `on_conflict_clause` to `SQL_INSERT_SHAPE`:
|
||
- Optional `(column_name_list)` conflict target.
|
||
- `DO NOTHING` branch.
|
||
- `DO UPDATE SET assignment_list [ where_clause ]` branch.
|
||
- `excluded` admitted as a table alias inside DO UPDATE
|
||
expressions only — its columns are the target table's
|
||
columns (per ADR-0033 §9). Scoped to the DO UPDATE
|
||
branch.
|
||
- Walker captures the UPSERT shape; worker executes the
|
||
validated SQL as-is (no special pre-processing).
|
||
|
||
### Scope (out — explicit)
|
||
|
||
- `INSERT OR REPLACE / IGNORE / ABORT / FAIL / ROLLBACK` —
|
||
OOS-2 in ADR-0033 §13.
|
||
|
||
### Build steps
|
||
|
||
1. Author the on_conflict_clause node tree.
|
||
2. Wire the conflict target column-name list to use the
|
||
same `Columns` ident slot as the column-name list at the
|
||
top of INSERT.
|
||
3. Author the `excluded` pseudo-alias machinery: a special
|
||
`TableBinding` pushed onto the current frame's
|
||
from_scope ONLY while walking the DO UPDATE expressions.
|
||
The binding's columns are sourced from the target
|
||
table's schema cache entry.
|
||
4. Tests per the exit gate.
|
||
|
||
### Exit gate
|
||
|
||
**Required green tests:**
|
||
|
||
- **DO NOTHING on conflict** —
|
||
`INSERT INTO t (id, name) VALUES (1, 'x')
|
||
ON CONFLICT (id) DO NOTHING` on a pre-existing `id = 1`
|
||
runs; affected-row count is 0; CSV unchanged.
|
||
- **DO UPDATE with excluded** —
|
||
`INSERT INTO t (id, name) VALUES (1, 'new')
|
||
ON CONFLICT (id) DO UPDATE SET name = excluded.name`
|
||
on a pre-existing `id = 1` runs; the row's name is
|
||
updated to `'new'`; CSV reflects the change.
|
||
- **DO NOTHING without target spec** —
|
||
`INSERT INTO t … ON CONFLICT DO NOTHING` handles any
|
||
conflict.
|
||
- **`excluded.col` resolves to the target table's
|
||
columns** (cross-cut completion / highlight test:
|
||
Tab at `… DO UPDATE SET name = excluded.|` lists
|
||
the target table's columns).
|
||
- **`excluded` does NOT leak outside DO UPDATE** —
|
||
parsing `INSERT INTO t VALUES (excluded.name, …)` (with
|
||
`excluded` used in the value list, not inside DO UPDATE)
|
||
fires `diagnostic.unknown_qualifier` or equivalent. The
|
||
alias is strictly scoped to the DO UPDATE branch.
|
||
- **Persistence write-through on DO UPDATE** — yes; the
|
||
target row is modified.
|
||
- All 3a–3g tests still green; baseline 1446 still green.
|
||
|
||
**Other gates:**
|
||
|
||
- `cargo clippy --all-targets -- -D warnings` clean.
|
||
|
||
### DA gate (written)
|
||
|
||
- **Does ON CONFLICT … DO UPDATE trigger persistence
|
||
write-through?** Yes; the target row is modified. The
|
||
worker doesn't accidentally bypass re-persistence on the
|
||
upsert path.
|
||
- **Is the `excluded` alias scoped to ONLY the DO UPDATE
|
||
branch, not leaking into the outer INSERT?** The leak
|
||
test above pins this.
|
||
- **Is `excluded` engine-neutral?** No — it's SQLite +
|
||
PostgreSQL spelling (standard SQL uses `MERGE … WHEN
|
||
MATCHED`). ADR-0033 §9 carves this out explicitly; the
|
||
DA confirms the carve-out is justified (it is) and that
|
||
the catalog wording referencing UPSERT doesn't leak
|
||
product names.
|
||
- **Does the conflict target column list reject
|
||
`__rdbms_*` columns?** The columns are owned by the
|
||
target table; if the target is rejected at the outer
|
||
table slot, the inner column list is by construction
|
||
fine. Sanity-verify.
|
||
|
||
---
|
||
|
||
## Sub-phase 3i — Diagnostics
|
||
|
||
### Scope (in)
|
||
|
||
- `diagnostic.insert_arity_mismatch` (ERROR, ADR-0033 §8.1):
|
||
walker pre-flight when `(column_name_list)`'s arity
|
||
disagrees with a value_tuple's arity (per-row for
|
||
multi-row VALUES), or with the SELECT's projection arity
|
||
for INSERT…SELECT.
|
||
- `diagnostic.auto_column_overridden` (WARNING, ADR-0033 §8.2):
|
||
walker emit when INSERT explicitly assigns a value to a
|
||
`serial` / `shortid` column.
|
||
- `diagnostic.not_null_missing` (WARNING, ADR-0033 §8.3):
|
||
walker emit when INSERT's `(column_name_list)` omits a
|
||
NOT-NULL-no-default column.
|
||
- Per-key positive + negative tests (the ADR-0032 §2d
|
||
pattern reused).
|
||
- Catalog entries with engine-neutral wording.
|
||
|
||
### Scope (out — explicit)
|
||
|
||
- Engine-side translations (FK / UNIQUE / NOT-NULL on
|
||
execute) — ADR-0033 §11 reuses existing keys; no new
|
||
ones needed.
|
||
- Predicate warnings on DML's sql_expr slots — inherited
|
||
from Phase-2 machinery; 3i cross-cuts-verifies they
|
||
fire on DML slots but doesn't re-implement.
|
||
|
||
### Build steps
|
||
|
||
1. Author the three walker diagnostic passes (or extend
|
||
existing passes if cleaner — implementer's call).
|
||
2. Author the catalog keys + i18n strings.
|
||
3. Author the positive + negative tests per key.
|
||
4. Verify per-row emit for multi-row arity mismatch.
|
||
5. Cross-cut-verify the inherited Phase-2 passes fire on
|
||
DML sql_expr slots.
|
||
|
||
### Exit gate
|
||
|
||
**Required green tests:**
|
||
|
||
- **`insert_arity_mismatch` positive (single-row)** —
|
||
`INSERT INTO t (a, b) VALUES (1, 2, 3)` fires.
|
||
- **`insert_arity_mismatch` negative (matched arity)** —
|
||
`INSERT INTO t (a, b) VALUES (1, 2)` doesn't fire.
|
||
- **`insert_arity_mismatch` per-row emit (multi-row)** —
|
||
`INSERT INTO t (a, b) VALUES (1, 2), (3, 4, 5), (6), (7, 8)`
|
||
fires on rows 2 and 3, NOT on rows 1 and 4 (two
|
||
diagnostics, two right rows silent).
|
||
- **`insert_arity_mismatch` on INSERT…SELECT** —
|
||
`INSERT INTO t (a) SELECT a, b FROM source` fires;
|
||
anchored on the SELECT's first projection_item span.
|
||
- **`auto_column_overridden` positive** —
|
||
`INSERT INTO t (id, name) VALUES (5, 'x')` (id is
|
||
`serial`) fires WARNING; the statement still runs.
|
||
- **`auto_column_overridden` negative** — omitting `id`
|
||
doesn't fire.
|
||
- **`auto_column_overridden` on shortid** — same as serial
|
||
but `id` is `shortid`.
|
||
- **`not_null_missing` positive** —
|
||
`INSERT INTO t (a) VALUES (1)` (b is NOT NULL no
|
||
default) fires WARNING; the statement still parses (the
|
||
engine rejects on execute).
|
||
- **`not_null_missing` negative** — including `b` doesn't
|
||
fire.
|
||
- **`not_null_missing` does NOT fire for columns with a
|
||
default value** — even if NOT NULL.
|
||
- **Cross-cut: schema-existence diagnostic fires on
|
||
INSERT VALUES** — `INSERT INTO t (nonexistent) VALUES (1)`
|
||
fires the Phase-2 key on the column name.
|
||
- **Cross-cut: schema-existence fires on UPDATE SET** —
|
||
`UPDATE t SET nonexistent = 1 WHERE id = 1` fires.
|
||
- **Cross-cut: schema-existence fires on UPDATE / DELETE
|
||
WHERE** — already verified in 3e/3f exit gates;
|
||
re-reference here.
|
||
- **Cross-cut: predicate warnings fire on DML WHERE / SET
|
||
/ VALUES** — `UPDATE t SET v = 1 WHERE x = NULL` fires
|
||
`eq_null`.
|
||
- All 3a–3h tests still green; baseline 1446 still green.
|
||
|
||
**Other gates:**
|
||
|
||
- `cargo clippy --all-targets -- -D warnings` clean.
|
||
|
||
### DA gate (written)
|
||
|
||
- **Does each new key have BOTH a positive and a negative
|
||
test?** Walk the keys; verify both rows above.
|
||
- **Is `auto_column_overridden` a Warning, not an Error?**
|
||
Read the diagnostic's severity field; confirm.
|
||
- **Is `not_null_missing` a Warning, not an Error?** Same.
|
||
- **Are the catalog wordings engine-neutral?** No "SQLite",
|
||
"PRAGMA", "STRICT", or "rusqlite" leak. Verify by
|
||
reading the i18n strings.
|
||
- **Does multi-row arity-mismatch actually emit per-row?**
|
||
The test pins it; the DA reads the code to confirm the
|
||
emit happens inside the per-row visit, not once for the
|
||
values_clause as a whole.
|
||
|
||
---
|
||
|
||
## Sub-phase 3j — Dispatch wiring on shared entry words
|
||
|
||
### Scope (in)
|
||
|
||
- Convert `data::INSERT`, `data::UPDATE`, `data::DELETE`
|
||
CommandNode shapes from their current DSL-only form to
|
||
`Choice([SQL_<X>_SHAPE, DSL_<X>_SHAPE])` per ADR-0033 §2.
|
||
- Wire `Node::Guard(reject_sql_in_simple_mode)` at the
|
||
head of each SQL_<X>_SHAPE's Seq (the 3a mechanism).
|
||
- Remove the development CommandNodes (`sql_insert` /
|
||
`sql_update` / `sql_delete`) from the registry.
|
||
- Verify the public test surface: existing DSL tests use
|
||
unchanged inputs and see unchanged outputs; new SQL
|
||
tests reach the SQL branch via the shared entry words.
|
||
|
||
### Scope (out — explicit)
|
||
|
||
- Any behaviour change. This is a wiring refactor:
|
||
observable behaviour must be identical before and after
|
||
3j.
|
||
|
||
### Build steps
|
||
|
||
1. Convert each shape. The Choice declares SQL first, DSL
|
||
second (per ADR-0033 §2).
|
||
2. Insert the Guard at the head of each SQL branch's Seq.
|
||
3. Update every test that referenced `sql_insert` /
|
||
`sql_update` / `sql_delete` to use `insert` / `update` /
|
||
`delete` instead.
|
||
4. Remove the development CommandNodes.
|
||
5. Run the full suite.
|
||
|
||
### Exit gate
|
||
|
||
**Required green tests:**
|
||
|
||
- **All existing DSL INSERT/UPDATE/DELETE tests still
|
||
green** — unmodified inputs, unmodified outputs.
|
||
- **All Phase-3 SQL DML tests still green via the new
|
||
dispatch path** — the SQL branch is reached through
|
||
`data::INSERT` etc., not through the development
|
||
CommandNodes. Verifiable: grep for `sql_insert` /
|
||
`sql_update` / `sql_delete` and confirm zero remaining
|
||
references in src/dsl/grammar/.
|
||
- **Structurally-ambiguous input routes SQL-first in
|
||
Advanced** —
|
||
`delete from t where id = 1` in Advanced mode goes
|
||
through the SQL DELETE branch (verifiable: a SQL-only
|
||
side effect fires, e.g., the cascade-summary code path
|
||
shared by both — pin by checking a unique-to-SQL trace
|
||
point, OR by verifying that a SQL-syntax-only construct
|
||
on the same input matches).
|
||
- **DSL-specific input falls through to DSL in Advanced** —
|
||
`delete from t --all-rows` in Advanced matches the DSL
|
||
branch (the `--all-rows` flag is DSL-only, so the SQL
|
||
Seq's NoMatch lets Choice fall through).
|
||
- **Simple mode + SQL input fires the Guard** —
|
||
`delete from t where id = 1 returning *` (DML+RETURNING,
|
||
clearly SQL) in Simple mode produces
|
||
`ValidationFailed` with
|
||
`message_key: "advanced_mode.sql_in_simple"`.
|
||
- **R1 invariant holds in real grammar** — the smoke-test
|
||
from 3a was minimal; 3j is the first time the Guard
|
||
mechanism sees real DML branches with overlapping
|
||
prefixes. A specific test: in Advanced mode, an INSERT
|
||
that the SQL branch parses partway and then rejects
|
||
(e.g., a malformed `RETURNING` tail that's also not
|
||
a valid DSL fragment) — the Choice's behaviour is what
|
||
ADR-0033 §2 dictates: SQL > DSL ordering means the SQL
|
||
branch's Failed propagates as the Choice's outcome (no
|
||
fallback because the DSL branch wouldn't match either).
|
||
The test pins the user-facing error wording.
|
||
- All 3a–3i tests still green; baseline 1446 still green.
|
||
|
||
**Other gates:**
|
||
|
||
- `cargo clippy --all-targets -- -D warnings` clean.
|
||
|
||
### DA gate (written)
|
||
|
||
- **Verify that no test was changed to make 3j pass.** Only
|
||
the CommandNode shape's structure changes; the
|
||
observable behaviour at the public test surface is
|
||
identical for inputs that worked before. Grep the diff
|
||
for tests whose `INPUT` strings changed in 3j — there
|
||
should be none for existing DSL tests; the Phase-3 SQL
|
||
tests' inputs are the same (only their dispatch path
|
||
differs).
|
||
- **Is the test count higher than before 3j?** It should
|
||
be the same or higher (no tests removed, possibly a
|
||
handful added for the structurally-ambiguous + fall-
|
||
through cases above).
|
||
- **R1 hold-up in real grammar?** The 3a smoke-test pinned
|
||
the invariant on synthetic tokens; the DA confirms it
|
||
holds on real DML.
|
||
|
||
---
|
||
|
||
## Sub-phase 3k — Verification sweep
|
||
|
||
### Scope (in)
|
||
|
||
The catch-all sub-phase. Verifies the full end-to-end Phase 3
|
||
surface and every cross-cut "X comes for free" claim. Produces
|
||
the **final phase-exit verification report** (next section).
|
||
|
||
### Build steps
|
||
|
||
1. Author the end-to-end integration tests (Tier 3) for
|
||
real-world-shape DML queries (the exit-gate list below).
|
||
2. Author or extend the cross-cut verification matrix (next
|
||
section) for every "X comes for free" claim from
|
||
ADR-0030/0031/0032/0033.
|
||
3. Run the full test suite. Confirm 1446 baseline + all
|
||
sub-phase 3a–3j additions + the 3k integration tests.
|
||
4. Run `cargo clippy --all-targets -- -D warnings`.
|
||
5. Produce the final verification report.
|
||
|
||
### Exit gate
|
||
|
||
**Required end-to-end integration tests:**
|
||
|
||
- **INSERT … SELECT cross-table** —
|
||
`INSERT INTO archive SELECT * FROM orders WHERE created
|
||
< '2025-01-01'` — runs, archive has the rows, both CSVs
|
||
reflect state, renderer clean.
|
||
- **Multi-row INSERT with all ten playground types** —
|
||
every type lands in a row via a multi-row INSERT;
|
||
values round-trip correctly.
|
||
- **UPDATE with subquery in SET** —
|
||
`UPDATE customers SET last_order_date = (SELECT max(date)
|
||
FROM orders WHERE customer_id = customers.id)` runs;
|
||
values updated; CSV reflects.
|
||
- **DELETE with cascade** — end-to-end including
|
||
per-relationship summary, multi-table CSV re-persistence.
|
||
- **UPSERT round-trip** — DO UPDATE updates an existing
|
||
row; DO NOTHING on a separate conflict no-ops; both
|
||
surface their expected outcomes.
|
||
- **RETURNING on each of INSERT / UPDATE / DELETE** — each
|
||
produces a DataResult; result-column types recovered.
|
||
- **`history.log` replay**: every Phase-3 statement form
|
||
written above replays from the log faithfully (ADR-0030
|
||
§11 unchanged for DML).
|
||
|
||
**Cross-cut verification matrix** (next section): every
|
||
claim must have a passing test, with the test's INPUT
|
||
being SQL syntax for SQL claims (handoff-29 §4.4 pin).
|
||
|
||
**Other gates:**
|
||
|
||
- Total test count: ≥ 1446 + (additions from 3a–3k). The
|
||
exact count depends on per-step authorship; the gate is
|
||
"every required-test row above and in the cross-cut
|
||
matrix has at least one green test", not a magic number.
|
||
- Zero failures, zero skipped (excluding the unchanged
|
||
doctest).
|
||
- `cargo clippy --all-targets -- -D warnings` clean.
|
||
|
||
### DA gate (written)
|
||
|
||
The final DA review, per CLAUDE.md Phase-5 verification and
|
||
handoff-29 §4.1 pin (specific critiques first, verdict
|
||
last; rubber-stamp PASS is a process failure):
|
||
|
||
- **Walk the cross-cut matrix row-by-row.** Every row's
|
||
test must (a) exist, (b) be green, (c) have INPUT that
|
||
matches the row's claim source (SQL claims → SQL
|
||
inputs).
|
||
- **Are there any "free for free" claims from
|
||
ADR-0030/0031/0032/0033 that have no explicit test?**
|
||
Phase-1 gap pattern, Phase-2's four DA findings. Each
|
||
must have one.
|
||
- **Did any sub-phase silently downgrade a requirement to
|
||
"later"?** Phase-2's "out of scope" defer-trap reflex.
|
||
Any "not in scope" call by the implementer that wasn't
|
||
user-confirmed is a verdict FAIL until escalation
|
||
closes (handoff-29 §4.2 pin).
|
||
- **Did any sub-phase make an autonomous decision that's
|
||
not in ADR-0033?** Specific items to check: the 3d
|
||
Option A/B choice (must be user-confirmed if Option A
|
||
was chosen unilaterally); any new catalog keys beyond
|
||
the three in §8.1–§8.3; any new Command variants beyond
|
||
the three in §10.
|
||
- **Are ALL tests green AND zero-skipped?** If not, the
|
||
verdict is FAIL and 3k loops back to whichever sub-phase
|
||
introduced the gap.
|
||
|
||
---
|
||
|
||
## Cross-cut verification matrix
|
||
|
||
Each row is a claim from ADR-0030, ADR-0031, ADR-0032, or
|
||
ADR-0033 that needs an explicit test, with the test's
|
||
location. The matrix is filled in during 3k; the gate is
|
||
"every row green AND every SQL claim's test takes SQL
|
||
input" (handoff-29 §4.4 pin).
|
||
|
||
| Claim source | Claim | Test location | Status |
|
||
|------------------------|--------------------------------------------------------------------------------|---------------|--------|
|
||
| **Statement shapes (§1)** | | | |
|
||
| ADR-0033 §1 | Single-row VALUES INSERT runs end-to-end | `ins::single_row_insert_persists_and_counts` | ✅ |
|
||
| ADR-0033 §1 | Multi-row VALUES INSERT runs end-to-end | `ins::multi_row_insert_persists_both_rows` | ✅ |
|
||
| ADR-0033 §1 | INSERT with `(column_name_list)` runs | `ins::single_row_insert_persists_and_counts` | ✅ |
|
||
| ADR-0033 §1 | INSERT without column list (full table arity) runs | `ins::no_column_list_full_arity_insert_persists` | ✅ |
|
||
| ADR-0033 §4 | `INSERT … SELECT` runs | `ins::insert_select_copies_rows_and_persists`, `e2e::e2e_insert_select_cross_table_copies_rows_and_persists_both` | ✅ |
|
||
| ADR-0033 §4 / R4 | `INSERT … WITH … SELECT` row source runs (Amendment-2 carry-through) | `ins::with_prefixed_insert_select_runs_and_persists` | ✅ |
|
||
| ADR-0033 §1 | UPDATE with WHERE runs | `upd::single_column_update_with_where_persists` | ✅ |
|
||
| ADR-0033 §1 / §12 | UPDATE without WHERE runs (no `--all-rows` rail) | `upd::update_without_where_runs_across_all_rows` | ✅ |
|
||
| ADR-0033 §1 | UPDATE with multi-column SET | `upd::multi_column_update_persists` | ✅ |
|
||
| ADR-0033 §1 | UPDATE with sql_expr in SET (function call, subquery, CASE) | `upd::update_with_sql_expr_in_set`, `e2e::e2e_update_with_subquery_in_set` | ✅ |
|
||
| ADR-0033 §1 | DELETE with WHERE runs | `del::delete_with_where_persists` | ✅ |
|
||
| ADR-0033 §1 / §12 | DELETE without WHERE runs (no `--all-rows` rail) | `del::delete_without_where_runs_across_all_rows` | ✅ |
|
||
| **Dispatch (§2)** | | | |
|
||
| ADR-0033 §2 / 3a | Guard fires BEFORE Seq commits (R1 invariant on synthetic markers) [a] | `walker::advanced_mode_dsl_input_falls_back_to_dsl` | ✅ |
|
||
| ADR-0033 §2 | Simple mode + DSL shared word → DSL branch | `parser::simple_dsl_delete_stays_dsl` | ✅ |
|
||
| ADR-0033 §2 [b] | Simple mode + shared word w/ SQL-only construct → DSL parse error + combined pointer | `parser::simple_shared_word_with_sql_construct_is_a_dsl_parse_error`, `input_render::ambient_hint_combines_dsl_error_with_advanced_sql_pointer`, `app::simple_mode_submit_of_sql_construct_appends_advanced_pointer`; SQL-only *entry* word → `parser::simple_sql_only_entry_word_points_at_advanced_mode` | ✅ |
|
||
| ADR-0033 §2 | Advanced + structurally-ambiguous (`delete from t where id = 1`) → SQL branch | `parser::advanced_ambiguous_delete_routes_to_sql` | ✅ |
|
||
| ADR-0033 §2 | Advanced + DSL-only (`--all-rows`) → DSL branch (falls through) | `parser::advanced_dsl_only_delete_falls_back_to_dsl` | ✅ |
|
||
| ADR-0033 §2 | Same for UPDATE shared entry word (+ `--all-rows` SQL-absorb counter-example) | `parser::advanced_ambiguous_update_routes_to_sql`, `e2e::e2e_update_all_rows_in_advanced_does_not_fall_back_to_dsl` | ✅ |
|
||
| ADR-0033 §2 | Same for INSERT shared entry word | `parser::advanced_ambiguous_insert_routes_to_sql` | ✅ |
|
||
| **RETURNING (§5 / §12)** | | | |
|
||
| ADR-0033 §5 | INSERT … RETURNING * surfaces DataResult | `ins::insert_returning_star_returns_inserted_row`, `e2e::e2e_returning_on_insert_update_delete` | ✅ |
|
||
| ADR-0033 §5 | UPDATE … RETURNING cols surfaces DataResult | `upd::update_returning_yields_modified_columns`, `e2e::e2e_returning_on_insert_update_delete` | ✅ |
|
||
| ADR-0033 §5 | DELETE … RETURNING * surfaces DataResult (pre-DELETE row) | `del::delete_returning_yields_predelete_row`, `e2e::e2e_returning_on_insert_update_delete` | ✅ |
|
||
| ADR-0033 §5 | DELETE … RETURNING preserves cascade summary alongside result set | `del::delete_returning_with_cascade_surfaces_both` | ✅ |
|
||
| ADR-0033 §12 | Result-column type recovery for each of ten playground types via RETURNING | `e2e::e2e_multirow_insert_all_ten_types_roundtrips_and_returning_recovers_each_type` (all 10), `ins::insert_returning_recovers_multiple_bare_column_types` (5) | ✅ |
|
||
| ADR-0033 §12 | Computed expression in RETURNING stays typeless | `ins::insert_returning_computed_expression_is_typeless` | ✅ |
|
||
| ADR-0033 §5 / R3 | Multi-row INSERT … RETURNING: column-origin same for all rows | `ins::multirow_autofill_returning_yields_distinct_generated_ids`, `e2e::e2e_multirow_insert_all_ten_types_…` | ✅ |
|
||
| **shortid auto-fill (§6)** | | | |
|
||
| ADR-0033 §6 | Single-row VALUES auto-fills omitted shortid PK | `ins::values_autofills_omitted_shortid_pk` | ✅ |
|
||
| ADR-0033 §6 | Multi-row VALUES yields DISTINCT shortids per row | `ins::values_multirow_autofills_distinct_shortids` | ✅ |
|
||
| ADR-0033 §6 | INSERT … SELECT auto-fills shortids per yielded row | `ins::insert_select_autofills_distinct_shortids` | ✅ |
|
||
| ADR-0033 §6 | Explicit value for shortid column respected (override; warning in §8.2) | `ins::explicit_shortid_value_is_respected` | ✅ |
|
||
| ADR-0033 §6 | serial + shortid combo: no double-fill collision | `ins::combined_serial_and_shortid_autofill` | ✅ |
|
||
| **Cascade summary (§7)** | | | |
|
||
| ADR-0033 §7 | DSL parity: same schema/data, SQL DELETE produces same per-relationship summary | `del::cascade_parity_with_dsl` | ✅ |
|
||
| ADR-0033 §7 / R2 [c] | WHERE-with-subquery: cascade correct (count-diff, R2 withdrawn) | `del::r2_where_with_subquery` | ✅ |
|
||
| ADR-0033 §7 [c] | Cascade detection by count-diff in a transaction (before/after) | `del::cascade_delete_reports_summary_and_repersists_child` | ✅ |
|
||
| ADR-0033 §7 | Cascade-affected child tables re-persisted | `del::cascade_delete_reports_summary_and_repersists_child`, `e2e::e2e_delete_with_cascade_reports_summary_and_repersists_children` | ✅ |
|
||
| ADR-0033 §7 | Cascade-summary formatter shared (one render path, DSL + SQL) | `del::cascade_parity_with_dsl` | ✅ |
|
||
| **Diagnostics (§8)** | | | |
|
||
| ADR-0033 §8.1 | `insert_arity_mismatch` positive (single-row) | `walker::insert_arity_mismatch_single_row_fires` | ✅ |
|
||
| ADR-0033 §8.1 | `insert_arity_mismatch` negative (matched arity) | `walker::insert_arity_match_is_silent` | ✅ |
|
||
| ADR-0033 §8.1 | `insert_arity_mismatch` per-row (multi-row VALUES) | `walker::insert_arity_mismatch_emits_per_row` | ✅ |
|
||
| ADR-0033 §8.1 | `insert_arity_mismatch` on INSERT…SELECT (projection arity) | `walker::insert_select_arity_mismatch_fires` | ✅ |
|
||
| ADR-0033 §8.2 | `auto_column_overridden` positive (serial) | `walker::auto_column_overridden_fires_on_explicit_serial` | ✅ |
|
||
| ADR-0033 §8.2 | `auto_column_overridden` positive (shortid) | `walker::auto_column_overridden_fires_on_explicit_shortid` | ✅ |
|
||
| ADR-0033 §8.2 | `auto_column_overridden` negative (omitted) | `walker::auto_column_overridden_silent_when_omitted` | ✅ |
|
||
| ADR-0033 §8.3 | `not_null_missing` positive | `walker::not_null_missing_fires_when_required_column_omitted` | ✅ |
|
||
| ADR-0033 §8.3 | `not_null_missing` negative | `walker::not_null_missing_silent_when_included` | ✅ |
|
||
| ADR-0033 §8.3 | `not_null_missing` does NOT fire for NOT-NULL-with-default | `walker::not_null_missing_silent_when_column_has_default` | ✅ |
|
||
| **UPSERT (§9)** | | | |
|
||
| ADR-0033 §9 | ON CONFLICT (col) DO NOTHING runs; no-ops on conflict | `ins::on_conflict_do_nothing_keeps_existing_row`, `e2e::e2e_upsert_round_trip_do_update_then_do_nothing` | ✅ |
|
||
| ADR-0033 §9 | ON CONFLICT (col) DO UPDATE SET … = excluded.… runs; row updated | `ins::on_conflict_do_update_applies_excluded`, `e2e::e2e_upsert_round_trip_…` | ✅ |
|
||
| ADR-0033 §9 | ON CONFLICT DO NOTHING (no target spec) accepts any conflict | `ins::on_conflict_do_nothing_without_target` | ✅ |
|
||
| ADR-0033 §9 | `excluded.col` completes to target table's columns inside DO UPDATE | `completion::excluded_prefix_completes_to_target_columns` | ✅ |
|
||
| ADR-0033 §9 | `excluded` rejected outside DO UPDATE (scoping) | `walker::excluded_outside_do_update_is_unknown_qualifier`, `walker::excluded_in_values_flagged_even_when_do_update_present` | ✅ |
|
||
| **Inherited Phase-2 diagnostics on DML slots (§8.4)** | | | |
|
||
| ADR-0032 §11 | `schema_existence` fires on INSERT VALUES / col-list column | `walker::insert_column_list_unknown_column_is_flagged` | ✅ |
|
||
| ADR-0032 §11 | `schema_existence` fires on UPDATE SET | `walker::sql_update_unknown_set_column_is_error` | ✅ |
|
||
| ADR-0032 §11 | `schema_existence` fires on UPDATE / DELETE WHERE | `walker::sql_update_where_unknown_column_is_error`, `walker::sql_delete_where_unknown_column_is_error` | ✅ |
|
||
| ADR-0032 §11 | `schema_existence` fires on RETURNING projection | `walker::returning_ref_unknown_column_is_flagged`, `walker::returning_ref_in_plain_insert_validated_against_target` | ✅ |
|
||
| ADR-0032 §11.6 | Predicate warning `eq_null` fires on UPDATE WHERE | `walker::sql_update_eq_null_in_where_warns` | ✅ |
|
||
| ADR-0032 §11.6 | Predicate warning `like_numeric` fires on DELETE WHERE | `walker::sql_delete_where_like_numeric_warns` | ✅ |
|
||
| ADR-0032 §11.6 [d] | Predicate warning fires on row-scoped DML sql_expr slots (UPDATE SET CASE, INSERT…SELECT WHERE) — VALUES has no row scope | `walker::sql_update_set_case_predicate_warns`, `walker::insert_select_where_predicate_warns` | ✅ |
|
||
| **Ambient assistance (§§ from ADR-0030 §8)** | | | |
|
||
| ADR-0030 §8 | Syntax highlighting works for SQL DML keywords (INSERT, UPDATE, DELETE, INTO, VALUES, SET, RETURNING, ON, CONFLICT) | `highlight::sql_dml_keywords_classified` | ✅ |
|
||
| ADR-0030 §8 | Tab completion works for SQL DML entry keywords | `completion::empty_input_offers_all_command_entry_keywords` | ✅ |
|
||
| ADR-0030 §8 | Tab completion works for target table after `INSERT INTO ` | `completion::insert_into_offers_table_names_at_target_slot` | ✅ |
|
||
| ADR-0030 §8 | Tab completion works for column names inside `(column_list)` | `completion::insert_into_open_paren_offers_current_table_columns` | ✅ |
|
||
| ADR-0030 §8 | Tab completion works for column names in UPDATE SET / WHERE | `completion::update_set_offers_only_current_table_columns`, `completion::update_where_offers_only_current_table_columns` | ✅ |
|
||
| ADR-0030 §8 [f] | Hint-panel assistance appears at representative DML slots | `input_render::advanced_mode_ambient_offers_dml_slot_candidates` (advanced SQL), `input_render::ambient_hint_at_insert_first_value_shows_int_prose` / `…at_update_set_shows_per_column_prose` / `…at_where_mentions_column_name` (simple DSL prose) | ✅ |
|
||
| ADR-0030 §8 | `[ERR]`/`[WRN]` validity indicator fires for SQL DML diagnostics | `e2e::e2e_validity_indicator_fires_for_sql_dml_diagnostic`, `walker::input_verdict_eq_null_is_warning`, `walking_skeleton::validity_indicator_renders_err_and_wrn_labels` | ✅ |
|
||
| ADR-0030 §8 | Per-command parse-error usage fires for SQL DML shared words | `parse_error_pedagogy::insert_partial_renders_insert_usage_template`, `…update_partial_renders_update_usage_template` | ✅ |
|
||
| **Engine-neutrality + safety** | | | |
|
||
| ADR-0030 §6 / ADR-0033 §1 | `__rdbms_*` rejection at INSERT target slot | `ins::parse_path_rejects_internal_target_table` | ✅ |
|
||
| ADR-0030 §6 / ADR-0033 §1 | `__rdbms_*` rejection at UPDATE target slot | `sql_update.rs::internal_target_table_rejected` | ✅ |
|
||
| ADR-0030 §6 / ADR-0033 §1 | `__rdbms_*` rejection at DELETE target slot | `del::internal_target_table_rejected_at_parse` | ✅ |
|
||
| ADR-0030 §6 / ADR-0033 §1 | `__rdbms_*` rejection in INSERT…SELECT row source's FROM | `sql_insert.rs::select_row_source_rejects_internal_from_table` | ✅ |
|
||
| ADR-0030 §7 / ADR-0033 §11 | Engine UNIQUE constraint failure surfaces via friendly layer (engine-neutral) | `friendly::enrich_unique_insert_resolves_table_column_value_and_pinpoint`, `ins::failed_insert_rolls_back_and_does_not_repersist` | ✅ |
|
||
| ADR-0030 §7 / ADR-0033 §11 | Engine NOT NULL constraint failure surfaces via friendly layer | `friendly::enrich_not_null_resolves_table_and_column` | ✅ |
|
||
| ADR-0030 §7 / ADR-0033 §11 | Engine FK constraint failure on DELETE-without-cascade surfaces | `del::delete_violating_fk_fails_and_persists_nothing` | ✅ |
|
||
| **Persistence + history (§10–§11)** | | | |
|
||
| ADR-0030 §11 | INSERT writes history.log + re-persists target CSV | `ins::insert_appends_literal_line_to_history` | ✅ |
|
||
| ADR-0030 §11 | UPDATE writes history.log + re-persists target CSV | `upd::update_appends_literal_line_to_history` | ✅ |
|
||
| ADR-0030 §11 | DELETE writes history.log + re-persists target + cascade-affected CSVs | `del::delete_appends_literal_line_to_history`, `e2e::e2e_delete_with_cascade_…` | ✅ |
|
||
| ADR-0030 §11 | Every Phase-3 statement form replays from history.log faithfully | `e2e::e2e_replay_phase3_dml_forms_from_a_script` | ✅ |
|
||
| ADR-0033 §10 | INSERT failure does NOT re-persist CSV | `ins::failed_insert_rolls_back_and_does_not_repersist` | ✅ |
|
||
| ADR-0006 / ADR-0033 §10 [e] | Auto-snapshot fires for SQL DML the same way it does for DSL DML | — (ADR-0006 unimplemented for BOTH paths; deferred) | N/A |
|
||
| **OOS rejections (§13)** | | | |
|
||
| ADR-0033 §13 OOS-1 | `INSERT INTO t DEFAULT VALUES` parse-rejects | `e2e::e2e_out_of_scope_dml_forms_parse_reject` | ✅ |
|
||
| ADR-0033 §13 OOS-2 | `INSERT OR REPLACE` / `OR IGNORE` parse-rejects | `e2e::e2e_out_of_scope_dml_forms_parse_reject` | ✅ |
|
||
| ADR-0033 §13 OOS-3 | `UPDATE … FROM other_table` parse-rejects | `e2e::e2e_out_of_scope_dml_forms_parse_reject` | ✅ |
|
||
| ADR-0033 §13 OOS-4 | `WITH x AS (…) UPDATE …` parse-rejects | `e2e::e2e_out_of_scope_dml_forms_parse_reject` | ✅ |
|
||
| ADR-0033 §13 OOS-4 | `WITH x AS (…) DELETE …` parse-rejects | `e2e::e2e_out_of_scope_dml_forms_parse_reject` | ✅ |
|
||
| ADR-0033 §13 OOS-5 | `INDEXED BY` / `NOT INDEXED` table modifiers parse-reject | `e2e::e2e_out_of_scope_dml_forms_parse_reject` | ✅ |
|
||
| ADR-0033 §13 OOS-6 | Multi-statement batch (`…; …`) parse-rejects (single `;`-tail still parses) | `e2e::e2e_out_of_scope_dml_forms_parse_reject`, `e2e::e2e_single_dml_statement_with_trailing_semicolon_parses` | ✅ |
|
||
|
||
**Test-location legend.** `ins::` = `tests/sql_insert.rs`, `upd::` =
|
||
`tests/sql_update.rs`, `del::` = `tests/sql_delete.rs`, `e2e::` =
|
||
`tests/sql_dml_e2e.rs` (new in 3k), `walker::` =
|
||
`src/dsl/walker/mod.rs::tests`, `highlight::` =
|
||
`src/dsl/walker/highlight.rs::tests`, `completion::` =
|
||
`src/completion.rs::tests`, `parser::` = `src/dsl/parser.rs::tests`,
|
||
`input_render::` = `src/input_render.rs::tests`, `app::` =
|
||
`src/app.rs::tests`, `friendly::` = `tests/friendly_enrichment.rs`,
|
||
`parse_error_pedagogy::` = `tests/parse_error_pedagogy.rs`,
|
||
`walking_skeleton::` = `tests/walking_skeleton.rs`,
|
||
`sql_update.rs` / `sql_insert.rs` (bare) = the `#[cfg(test)]` module
|
||
inside `src/dsl/grammar/sql_update.rs` / `sql_insert.rs`.
|
||
|
||
**Footnotes (resolutions surfaced during 3k).**
|
||
|
||
- **[a]** ADR-0033 **Amendment 1** replaced `Node::Guard` with
|
||
category-grouped, mode-aware dispatch in `walker::walk`; the R1
|
||
invariant ("the SQL branch failing must not strand the DSL
|
||
candidate") is now the Advanced-mode fall-through, proven on the
|
||
dispatch smoke registry by `advanced_mode_dsl_input_falls_back_to_dsl`
|
||
and on real DML by `parser::advanced_dsl_only_delete_falls_back_to_dsl`.
|
||
- **[b]** ADR-0033 **Amendment 3** supersedes the original
|
||
"bare `ValidationFailed`" expectation for shared words: a shared
|
||
entry word in Simple mode commits the **DSL candidate** and surfaces
|
||
the real DSL parse error, with the `advanced_mode.also_valid_sql`
|
||
pointer combined at the render layer. The bare
|
||
`advanced_mode.sql_in_simple` hint is reserved for SQL-only *entry
|
||
words* (`select` / `with`).
|
||
- **[c]** ADR-0033 **Amendment 2** withdrew R2: the cascade summary is
|
||
produced by before/after **count-diff in a transaction** (DSL
|
||
parity), not WHERE-byte pre-count injection. The subquery-in-WHERE
|
||
case is correct by construction.
|
||
- **[d]** **Matrix claim corrected (user-confirmed, 3k).** Predicate
|
||
warnings fire on every DML `sql_expr` slot that carries **row scope**
|
||
(UPDATE/DELETE WHERE, UPDATE SET incl. CASE, INSERT…SELECT
|
||
projection & WHERE). Plain `INSERT … VALUES` has no row scope, so a
|
||
bare-column predicate is inapplicable there — consistent with
|
||
ADR-0033 §8.4 ("WHERE and CASE expressions"). The original example
|
||
("inside INSERT VALUES") was an over-statement of the §8.4 claim.
|
||
- **[e]** **N/A — deferred (user-confirmed, 3k).** ADR-0006
|
||
auto-snapshot / undo is unimplemented for **both** the DSL and SQL
|
||
DML paths (the U-series, per `CLAUDE.md` "Things deliberately
|
||
deferred"). The "same way as DSL" parity therefore holds vacuously
|
||
and Phase 3 introduces no regression. ADR-0033 §10's auto-snapshot
|
||
consequence is contingent on the deferred ADR-0006 work; this row is
|
||
N/A, not red, and does not block the phase exit.
|
||
- **[f]** **Attribution corrected (`/runda` round).** The ambient
|
||
hint-panel claim sits under the §8 *advanced* surface, but the
|
||
hint-prose tests run via `ambient_hint` (which defaults to
|
||
`Mode::Simple` — the DSL value-slot prose). 3k's `/runda` round added
|
||
`advanced_mode_ambient_offers_dml_slot_candidates` so the row has a
|
||
genuine **advanced-mode** DML hint-panel attribution (column
|
||
candidates at an `UPDATE … SET` LHS and inside an `INSERT … (` column
|
||
list); the simple-mode DSL prose tests remain as the DSL-surface
|
||
complement.
|
||
|
||
The implementer fills in `Test location` and `Status`
|
||
(✅ / ❌) as 3k proceeds. A row marked red blocks the
|
||
phase exit.
|
||
|
||
**Attribution rule (handoff-29 §4.4 pin):** for any row
|
||
whose Claim source is a SQL claim, the `Test location` must
|
||
point at a test whose INPUT is SQL syntax. Cross-checked
|
||
at attribution time, not at DA-review time.
|
||
|
||
---
|
||
|
||
## Final phase-exit verification report
|
||
|
||
Produced at the end of 3k. A markdown document at
|
||
`docs/handoff/<date>-phase-3-verification.md` (matching the
|
||
Phase-2 naming pattern). The report contains:
|
||
|
||
1. **Test suite totals** — passed / failed / skipped /
|
||
ignored, exact numbers from `cargo test` output.
|
||
Compared against the Phase-2 baseline (1446 / 0 / 0 / 1).
|
||
2. **The full cross-cut verification matrix** with every
|
||
row green.
|
||
3. **A requirements-to-test mapping** — every numbered
|
||
decision in ADR-0033 §§1–13 with the test(s) that prove
|
||
it.
|
||
4. **A list of any autonomous decisions** made during
|
||
implementation, with rationale and explicit user-
|
||
confirmation pointer for each. (Per CLAUDE.md: an
|
||
autonomous decision without explicit user confirmation
|
||
is a verdict FAIL.)
|
||
5. **The DA's written final review** — with specific
|
||
critiques listed first and the PASS / FAIL verdict
|
||
last. No "conditional" verdicts (handoff-28 §4.1 pin).
|
||
|
||
Only after the report is produced and signed off does the
|
||
user-facing "Phase 3 complete" message issue.
|
||
|
||
## Open questions to escalate before code starts
|
||
|
||
These are matters the plan deliberately leaves to the
|
||
implementer to escalate when reached, *not* to decide
|
||
silently:
|
||
|
||
1. **shortid auto-fill on INSERT … SELECT (3d).** Option A
|
||
(SQL rewrite) vs Option B (worker materialisation).
|
||
ADR-0033 §6 doesn't pick. Plan default if the user is
|
||
unavailable: Option B (simpler, matches pedagogical
|
||
posture). Implementer escalates BEFORE 3d code lands;
|
||
if user unavailable, choice is recorded in the
|
||
verification report under "autonomous decisions" for
|
||
retroactive confirmation.
|
||
2. **R1 mechanism fallback (3a).** If option (a) (Guard
|
||
validator) doesn't work, options (b) and (c) per
|
||
ADR-0033 §2's enumeration. The plan does NOT pre-pick;
|
||
if (a) fails, the implementer evaluates (b) and (c)
|
||
and brings a recommendation to the user.
|
||
3. **Cascade-summary pre-count construction (3f).** Two
|
||
pre-count strategies (typed-Expr from DSL, byte-range
|
||
string from SQL) vs one unified constructor. Plan
|
||
default: unified constructor that accepts either input
|
||
form, with the formatter alone shared. Implementer's
|
||
call; not an escalation unless a third option surfaces.
|
||
4. **UPSERT engine-neutral catalog wording (3h).** The
|
||
`excluded` keyword is engine-neutral-carved-out per
|
||
ADR-0033 §9; the surrounding catalog wording (e.g.,
|
||
the help-text for an UPSERT statement, the error
|
||
message if `excluded` is used outside DO UPDATE) must
|
||
not leak SQLite product names. If the implementer
|
||
isn't sure, escalate before writing the i18n string.
|
||
|
||
## What this plan does NOT contain
|
||
|
||
- Time estimates. Phase 3 is medium-sized but introduces
|
||
R1's structural risk; calibration is poor. Milestones,
|
||
not hours.
|
||
- Sub-phase commit granularity beyond "ship green tests
|
||
at each gate". The implementer commits at natural break
|
||
points within a sub-phase; the gate is what gets
|
||
verified.
|
||
- Re-statements of ADR-0033's decisions. The plan refers
|
||
to ADR-0033 by section; the implementer reads both.
|
||
- Process pins from the prior session that don't change
|
||
anything mechanical. They land in the implementer's
|
||
context via handoff-29 §4, not via duplication here.
|