docs: ADR-0036 revised to surgical "validate-and-retain"; +X4/X5 open questions
Narrow ADR-0036 from "bind literals via the DSL path" to "validate literal values (shared validators) + retain them; execute verbatim, keep auto-fill and command identity mode-specific" — after a concrete auto-fill difference (non-PK serial) confirmed the modes aren't identical even for single-row literals. Augments (no longer supersedes) ADR-0030 §4 / ADR-0033 §10; Amendment 3 stands. README + forward-notes on 0030/0033 updated. Records requirements.md X4 (serial auto-fill — possible bug) and X5 (framework cohesion / share-mechanics-not-commands).
This commit is contained in:
@@ -157,17 +157,18 @@ that `docs/simple-mode-limitations.md` records as the inverse
|
|||||||
of the simple-mode subset. The DSL `Expr` is the *DSL's*
|
of the simple-mode subset. The DSL `Expr` is the *DSL's*
|
||||||
representation; the SQL surface does not round-trip through it.
|
representation; the SQL surface does not round-trip through it.
|
||||||
|
|
||||||
> **Forward note (2026-05-26).** The **Proposed ADR-0036** narrows the
|
> **Forward note (2026-05-26, ADR-0036 Accepted).** ADR-0036 **augments**
|
||||||
> "DML → validated SQL text" half of this section: **literal data
|
> the "DML → validated SQL text" half of this section — it does **not**
|
||||||
> values** in `INSERT`/`UPDATE` should be parsed to typed `Value`s,
|
> change the execution model. Advanced-mode DML still executes verbatim;
|
||||||
> validated, and bound as parameters (as the DSL already does), not
|
> what's added is that "validated" now also means *value*-validated: the
|
||||||
> handed to the engine as text — so advanced mode gets the same
|
> **literal data values** in `INSERT`/`UPDATE` are checked against the
|
||||||
> value-validation, hinting, highlighting, and error context as simple
|
> playground type system (and retained for error reporting) **before** the
|
||||||
> mode. The `SELECT` half and the full-expression-surface rationale above
|
> verbatim statement runs, sharing the DSL's per-type validators. No
|
||||||
> **stand**: expressions and queries remain verbatim text (ADR-0026's
|
> binding, no reconstruction, no command-identity change (ADR-0033 Am3
|
||||||
> limited `Expr` is *not* imposed on the SQL surface). The dividing line
|
> stands). The `SELECT` half and the full-expression-surface rationale
|
||||||
> is "bindable literal vs engine-evaluated expression-or-query." Pending
|
> above **stand**: expressions and queries remain verbatim text (ADR-0026's
|
||||||
> acceptance.
|
> limited `Expr` is *not* imposed on SQL). Dividing line: a static literal
|
||||||
|
> value (validate it) vs an engine-evaluated expression-or-query.
|
||||||
|
|
||||||
### 5. Type vocabulary — the playground's, not the engine's
|
### 5. Type vocabulary — the playground's, not the engine's
|
||||||
|
|
||||||
|
|||||||
@@ -10,15 +10,19 @@ for the phase-exit report and the filled cross-cut matrix in
|
|||||||
`docs/plans/20260520-adr-0033-phase-3.md`. Amendments 1–3 below
|
`docs/plans/20260520-adr-0033-phase-3.md`. Amendments 1–3 below
|
||||||
are part of this acceptance.
|
are part of this acceptance.
|
||||||
|
|
||||||
> **Forward note (2026-05-26).** The **Proposed ADR-0036** narrows the
|
> **Forward note (2026-05-26, ADR-0036 Accepted).** ADR-0036 **augments**
|
||||||
> §10 verbatim-execution model for **literal data values**: rather than
|
> the §10 model — it does **not** change verbatim execution and does
|
||||||
> handing the user's literal text to the engine, `INSERT`/`UPDATE` should
|
> **not** collapse the two-command identity of **Amendment 3** (which
|
||||||
> parse literal values to typed `Value`s, validate them, retain them on
|
> stands). It adds a *value-validation* step: `INSERT`/`UPDATE` literal
|
||||||
> the command (for binding *and* error enrichment), and bind them as
|
> values are parsed to typed `Value`s, **validated** against the column
|
||||||
> parameters — as the simple-mode DSL already does. Expressions, `WHERE`
|
> type (sharing the DSL's validators), and **retained** on the command
|
||||||
> predicates, `INSERT … SELECT`, `RETURNING`, UPSERT, and `SELECT` stay
|
> (so a constraint error can show the offending value) — then the
|
||||||
> verbatim text. The `Sql*` command variants and worker handlers remain;
|
> statement still executes **verbatim**. `Command::SqlInsert` gains a
|
||||||
> only the literal-value execution path changes. Pending acceptance.
|
> captured-literals payload; its execution and `plan_shortid_autofill`
|
||||||
|
> are unchanged. Expressions, `WHERE`, `INSERT … SELECT`, `RETURNING`,
|
||||||
|
> UPSERT, and `SELECT` are untouched. (The serial/shortid auto-fill
|
||||||
|
> difference vs simple mode is tracked separately as `requirements.md`
|
||||||
|
> X4.)
|
||||||
|
|
||||||
## Context
|
## Context
|
||||||
|
|
||||||
|
|||||||
@@ -1,31 +1,48 @@
|
|||||||
# ADR-0036: DML execution — bind literal values, reserve verbatim text for expressions and queries
|
# ADR-0036: Value validation for advanced-mode DML — validate literals, keep execution and identity mode-specific
|
||||||
|
|
||||||
## Status
|
## Status
|
||||||
|
|
||||||
**Proposed** (design agreed with the user in conversation, 2026-05-26;
|
**Accepted** (design agreed with the user in conversation, 2026-05-26;
|
||||||
`/runda` verification pass completed 2026-05-26 — three factual/precision
|
`/runda` verification pass completed 2026-05-26; the mechanism was then
|
||||||
corrections applied to the evidence + matrix, and three design forks
|
**deliberately narrowed** during the same conversation — see below — from
|
||||||
confirmed with the user: per-position classification with
|
"bind literal values through the DSL's path" to the surgical
|
||||||
`RETURNING`/`ON CONFLICT` as clauses over bindable `VALUES`; the
|
**"validate-and-retain, execute verbatim"** after the user pushed back on
|
||||||
bindable-literal set including signed numerics, `NULL`, and boolean/string
|
consolidating the two modes and a concrete auto-fill difference confirmed
|
||||||
literals; and `WHERE` left as text deliberately. Pending the user's
|
that even the single-row literal case is **not** identical across modes).
|
||||||
go-ahead to implement).
|
**Phase 1 implemented 2026-05-26** (`INSERT … VALUES` literal validation +
|
||||||
|
offending-value retention; capture-at-parse, no grammar change, execution
|
||||||
|
unchanged). Phases 2 (`UPDATE … SET` literals) and 3 (completion
|
||||||
|
hinting/highlighting) pending.
|
||||||
|
|
||||||
**Supersedes**, in part, **ADR-0030 §4** ("DML and `SELECT` → executed
|
**Augments** **ADR-0030 §4** and **ADR-0033 §10** — it does **not**
|
||||||
as the validated SQL itself … modelling them as a typed `Command` buys
|
supersede them and does **not** change the execution model. Advanced-mode
|
||||||
nothing") and the **execution model of ADR-0033 §10** (the three
|
DML still executes the validated SQL **verbatim**; ADR-0033 Amendment 3's
|
||||||
`Sql{Insert,Update,Delete}` variants executing the user's text
|
two-command identity (`Command::Insert` vs `Command::SqlInsert`) **stands
|
||||||
verbatim). It **narrows** those decisions rather than reversing them:
|
unchanged**. What this ADR adds is a **value-validation step**: the word
|
||||||
the `SELECT` half of ADR-0030 §4 stands unchanged, and verbatim text
|
"validated" in "executed as the validated SQL itself" (ADR-0030 §4) is
|
||||||
execution remains correct — and necessary — for everything that is an
|
extended to mean *value*-validated, not merely *syntactically* validated —
|
||||||
**expression or a query**. What changes is that **literal data values**
|
the literal data values in an advanced-mode `INSERT`/`UPDATE` are checked
|
||||||
stop being handed to the engine as text and instead flow through the
|
against the playground type system (and retained for error reporting)
|
||||||
same typed, validated, parameter-bound path the simple-mode DSL has used
|
before the statement runs.
|
||||||
all along.
|
|
||||||
|
|
||||||
Builds on the ADR-0035 precedent (DDL executes *structurally*, not
|
Builds on the ADR-0035 precedent (DDL executes *structurally*, not
|
||||||
verbatim) and completes the thought: structure was the first place
|
verbatim): there, structure was the first place "grammar as text" was too
|
||||||
"grammar as text" broke; literal data values are the second.
|
broad. This ADR makes a **narrower** correction for DML — not to *how* it
|
||||||
|
executes, but to *what gets checked* before it does.
|
||||||
|
|
||||||
|
**Conversation note (the principle this records).** The first instinct was
|
||||||
|
to *consolidate* — bind literals via the DSL path, even emit
|
||||||
|
`Command::Insert` from the advanced surface. That was rejected, for a
|
||||||
|
reason worth preserving: simple- and advanced-mode commands are kept
|
||||||
|
distinct **because they can legitimately differ**, and they do — e.g.
|
||||||
|
auto-fill: simple-mode `do_insert` fills an omitted non-PK `serial` with
|
||||||
|
`MAX(col)+1`, advanced-mode does not (`requirements.md` **X4**, flagged as
|
||||||
|
a possible bug to investigate separately). Collapsing the commands would
|
||||||
|
silently drag in such differences. The durable principle (also
|
||||||
|
`requirements.md` **X5**): **keep a distinct command per distinct case;
|
||||||
|
share execution *mechanics* as library helpers, never by fusing command
|
||||||
|
identity.** This ADR shares exactly one mechanic — the per-type value
|
||||||
|
validators — and nothing else.
|
||||||
|
|
||||||
## Context
|
## Context
|
||||||
|
|
||||||
@@ -129,198 +146,176 @@ type recovery — entirely from **walking the validated parse**, with the
|
|||||||
engine executing the text. Queries have no data values to validate
|
engine executing the text. Queries have no data values to validate
|
||||||
against columns; owning them buys nothing and costs enormously.
|
against columns; owning them buys nothing and costs enormously.
|
||||||
|
|
||||||
So the dividing line is **not** "DDL vs DML." It is **bindable literal
|
So the dividing line is **not** "DDL vs DML." It is **a static literal
|
||||||
vs engine-evaluated expression-or-query.**
|
value (which we can validate) vs an engine-evaluated
|
||||||
|
expression-or-query (which we cannot).**
|
||||||
|
|
||||||
## Decision
|
## Decision
|
||||||
|
|
||||||
### 1. The principle
|
### 1. The principle
|
||||||
|
|
||||||
> **Own structure and literal values; reserve verbatim text for
|
> **In advanced-mode `INSERT`/`UPDATE`, validate each literal data value
|
||||||
> expressions and queries.**
|
> against its target column's type before executing, and retain the
|
||||||
>
|
> literal so a constraint error can name it. Execute the statement
|
||||||
> Classification is **per value position**, not per statement. Wherever a
|
> verbatim, exactly as today. Do not bind, do not reconstruct, do not
|
||||||
> value position holds a **bare literal**, parse it to a typed `Value`,
|
> touch auto-fill, do not collapse command identity.**
|
||||||
> validate it against the playground type system, surface the column type
|
|
||||||
> in completion, highlight a mismatch, and execute it as a **bound
|
|
||||||
> parameter** — exactly as the simple-mode DSL does. Wherever the
|
|
||||||
> position holds a **computed expression** (`qty*2`, a function call, a
|
|
||||||
> scalar subquery), or the value *source* is itself a query
|
|
||||||
> (`INSERT … SELECT`), hand the validated text to the engine — because
|
|
||||||
> that is where text-to-engine is necessary, not merely convenient.
|
|
||||||
|
|
||||||
The test for which side a position falls on is operational: **can this
|
Only the **value validation** is shared between simple and advanced mode —
|
||||||
value be bound as a parameter?** A literal can; a computed expression or
|
via the existing per-type validators (`Value::bind_for_column` /
|
||||||
a subquery cannot — the engine must evaluate it.
|
`validate_date` / `shortid::validate`). Everything else stays
|
||||||
|
mode-specific: execution is still verbatim text-to-engine,
|
||||||
|
`plan_shortid_autofill` is untouched, and `Command::SqlInsert` /
|
||||||
|
`Command::SqlUpdate` remain distinct from their DSL counterparts.
|
||||||
|
|
||||||
**What counts as a literal** (the bindable set — matching the DSL lexer's
|
**What counts as a literal** (the set we validate — matching the
|
||||||
`consume_number_literal` / `consume_string_literal` and the
|
`null`/`true`/`false` words plus number/string literals as the walker
|
||||||
`null`/`true`/`false` words): `NULL`, a boolean literal, a string
|
tokenises them): `NULL`, a boolean literal, a string literal, and a
|
||||||
literal, and a **signed** numeric literal (`-5`, `3.14`). A signed
|
**signed** numeric literal (`-5`, `3.14`). A signed numeric counts as a
|
||||||
numeric must count as a literal *even if* the grammar would otherwise
|
literal even though `sql_expr` tokenises the sign separately (`Punct('-')`
|
||||||
read `-5` as unary-minus-of-`5`, or negative numbers would silently skip
|
then `NumberLit`) — a leading sign at the start of a value position is
|
||||||
validation. Anything else in a value position — arithmetic, function
|
part of the literal, not an operator. Anything else in a value position —
|
||||||
calls, `CASE`, subqueries, column references — is an expression and stays
|
arithmetic, function calls, `CASE`, subqueries, column references — is an
|
||||||
text.
|
**expression**: there is no static value to validate, so it is left to the
|
||||||
|
engine (unchanged).
|
||||||
|
|
||||||
**Clauses are not value sources.** `RETURNING` and `ON CONFLICT …` are
|
**Why not bind / converge.** Binding was the *first* instinct and is
|
||||||
clauses *on* a statement, not value positions. An
|
**rejected**. The two proven gaps (a malformed literal slipping through;
|
||||||
`INSERT INTO T VALUES (1, 'x') RETURNING id` or `… ON CONFLICT DO UPDATE
|
the offending value missing from errors) are closed by **validation +
|
||||||
…` still has **bindable literal `VALUES`**; the clause rides on the
|
retention alone** — binding adds nothing to either. Meanwhile, executing
|
||||||
generated statement (and its own expressions — e.g. `excluded.x` in a
|
the user's *own* text verbatim is already safe (their quoting stands; no
|
||||||
`DO UPDATE SET` — stay text). Only when the value *source* is a query
|
re-quoting risk because we do not reconstruct), and binding/convergence
|
||||||
(`INSERT … SELECT`) is there nothing to bind.
|
would risk dragging in genuinely mode-specific behaviour (auto-fill — X4;
|
||||||
|
natural-order column mapping) that must stay separate. So we share the
|
||||||
|
validators and nothing else. This keeps the modes cleanly apart
|
||||||
|
(`requirements.md` X5) while fixing the bug that they should *not* differ
|
||||||
|
on: whether a learner's malformed value is caught.
|
||||||
|
|
||||||
### 2. What this means per statement
|
### 2. What this means per statement
|
||||||
|
|
||||||
- **`INSERT … VALUES`** — each value position is a literal *or* an
|
Execution is **unchanged** for every statement below; the only addition is
|
||||||
expression. Literal positions are typed/validated/bound; expression
|
a pre-execution validation of literal value positions.
|
||||||
positions stay text. A row that is all literals executes exactly like
|
|
||||||
the DSL `do_insert` (`VALUES (?1, ?2, …)`); a row mixing the two
|
|
||||||
generates `VALUES (?1, a+1, ?2)`. Multi-row `VALUES` repeats this per
|
|
||||||
row. A trailing **`RETURNING`** or **`ON CONFLICT …`** clause is
|
|
||||||
carried on the generated statement — the bound `VALUES` are unaffected;
|
|
||||||
the clause's own expressions stay text.
|
|
||||||
- **`UPDATE … SET`** — `SET col = <literal>` is typed/validated/bound;
|
|
||||||
`SET col = <expr>` stays text.
|
|
||||||
- **`WHERE` (UPDATE/DELETE)** — **stays text, deliberately**, even for a
|
|
||||||
simple `WHERE id = 5`. `WHERE` is an expression in general; keeping it
|
|
||||||
text preserves the full SQL surface (ADR-0026's `Expr` is the *DSL's*
|
|
||||||
representation and is not imposed on SQL), and the value-feedback
|
|
||||||
motivation is already met by `VALUES`/`SET` — a constraint error names
|
|
||||||
a *written* value, not a predicate operand. This is a scope choice, not
|
|
||||||
an oversight; per-position binding is **not** applied to `WHERE`.
|
|
||||||
- **`DELETE`** — no data values to bind; `WHERE` per the rule above.
|
|
||||||
- **`INSERT … SELECT`** — entirely text: the value *source* is a query,
|
|
||||||
so there are no literals to bind.
|
|
||||||
- **`SELECT`** — unchanged. Text-to-engine, feedback-from-walk
|
|
||||||
(ADR-0032). Explicitly **not** lowered to a typed AST.
|
|
||||||
|
|
||||||
### 3. What it fixes — one change, four faces
|
- **`INSERT … VALUES`** — every literal position (single- or multi-row) is
|
||||||
|
validated against its column type before the verbatim insert runs; a
|
||||||
|
malformed literal is refused with the same friendly wording the DSL uses
|
||||||
|
(shared `bind_for_column`). Expression positions are skipped (nothing to
|
||||||
|
validate). `RETURNING` / `ON CONFLICT` / `INSERT … SELECT` need no
|
||||||
|
special handling — validation simply applies to whatever literal
|
||||||
|
`VALUES` are present, and the statement still executes verbatim.
|
||||||
|
- **`UPDATE … SET`** — `SET col = <literal>` is validated; `SET col =
|
||||||
|
<expr>` is skipped. (Phase 2 — see §5.)
|
||||||
|
- **`WHERE` (UPDATE/DELETE)** — **not** validated. `WHERE` is an
|
||||||
|
expression in general; the value-feedback motivation is met by
|
||||||
|
`VALUES`/`SET` (a constraint error names a *written* value). Deliberate
|
||||||
|
scope choice, not an oversight.
|
||||||
|
- **`SELECT`** — entirely unchanged. No data values to validate.
|
||||||
|
|
||||||
Retaining the typed literal (a) lets validation run on it (the proven
|
### 3. What it fixes
|
||||||
gap), (b) lets completion hint the column type and highlighting flag a
|
|
||||||
mismatch, (c) lets the worker **bind** it (safe, type-correct), and (d)
|
|
||||||
leaves it **on the command** so `enrich_*` can show the real value in a
|
|
||||||
constraint error. The neutral "that value" safety net (ADR-0035
|
|
||||||
Amendment 1) remains correct for the genuinely-computed cases — there is
|
|
||||||
no input literal to show, and that is the honest answer.
|
|
||||||
|
|
||||||
### 4. Explicit requirement — carry the typed literals
|
Validating the literal closes the **validation gap** (the malformed `date`
|
||||||
|
`2025/01/15` is now refused in advanced mode, as proven by the
|
||||||
|
characterization test). Retaining the literal on the command closes the
|
||||||
|
**error-value gap** (`enrich_*` reads it, so a constraint error shows the
|
||||||
|
real value instead of the neutral "that value"). Completion **hinting /
|
||||||
|
highlighting** is **not** delivered here — it needs a grammar-level change
|
||||||
|
(§5, Phase 2). The neutral "that value" safety net (ADR-0035 Amendment 1)
|
||||||
|
remains correct for genuinely-computed expression values — there is no
|
||||||
|
input literal to show.
|
||||||
|
|
||||||
`SqlInsert`/`SqlUpdate` (or their successors) **must retain the parsed
|
### 4. Explicit requirement — retain the literals, change nothing else
|
||||||
typed literal values** (per column position), not only the raw text, so
|
|
||||||
that both the executor (binding) and the error enricher (display) can
|
|
||||||
use them. This is the concrete hinge on which faces (a), (c), and (d)
|
|
||||||
turn; it must not be left implicit.
|
|
||||||
|
|
||||||
The command **also** keeps the user's **original source text**: that is
|
`Command::SqlInsert` (and later `SqlUpdate`) **gains a captured-literals
|
||||||
what `history.log` records and what `replay` re-runs (ADR-0034), so the
|
payload** (per row, per position; `None` for an expression position) in
|
||||||
journal stays faithful to what the user typed even though execution now
|
addition to the existing raw text. The executor validates from it and the
|
||||||
binds the literals from a generated statement. Two representations, one
|
error enricher reads it. The original source text is **unchanged** and is
|
||||||
command: original text for the journal, parsed literals for execution +
|
still what `history.log` records and `replay` re-runs (ADR-0034). The
|
||||||
feedback + errors.
|
command variant, its execution, and `plan_shortid_autofill` are **not**
|
||||||
|
modified. Validation reuses the existing value-binding helper
|
||||||
|
(`impl_value_for` / `Value::bind_for_column`) for wording parity with the
|
||||||
|
DSL — the resulting bound value is **discarded** (we do not bind for
|
||||||
|
execution), only its `Result` is used.
|
||||||
|
|
||||||
### 5. Mechanism (recommended, with the alternative recorded)
|
### 5. Mechanism + phasing
|
||||||
|
|
||||||
- **Recommended — typed-slot hybrid grammar.** At each DML value
|
- **Phase 1 (this ADR's immediate work) — capture + validate + retain.**
|
||||||
position, the SQL grammar offers `Choice(typed-literal-slot,
|
At parse, `build_sql_insert` classifies each `VALUES` position from the
|
||||||
sql_expr)`: the typed slot (reusing the DSL's live `column_value_list`
|
matched path (a single literal token, or a signed number → a typed
|
||||||
/ per-type `TypedValueSlot`s — `data.rs:141`/`189`/`269`, `shared.rs`)
|
`Value`; anything else → an expression marker) and stores the per-row
|
||||||
captures + validates + retains the literal and drives
|
result on the command — **no grammar change, no reparse**. The executor
|
||||||
completion/highlighting; `sql_expr` is the fallback for a genuine
|
validates the captured literals against the resolved column types before
|
||||||
expression. This is the only
|
the verbatim insert; the enricher reads them. Covers single- and
|
||||||
option that delivers **live completion hints** for SQL DML values,
|
multi-row, with or without `RETURNING`/`ON CONFLICT`, because execution
|
||||||
because the hint comes from the grammar walk.
|
is untouched.
|
||||||
- **Alternative — post-parse semantic pass.** Keep the grammar as
|
- **Phase 2 — `UPDATE … SET` literal validation** (same technique on the
|
||||||
`sql_expr` and add a pass that, for positions that are bare literals,
|
SET assignment list).
|
||||||
validates + retains them. Simpler; delivers validation, retention, and
|
- **Phase 3 — completion hinting / highlighting.** This is the *only*
|
||||||
the error-value fix, but **not** live completion hinting. Recorded as
|
part that needs a grammar change: a `Choice(typed-literal-slot,
|
||||||
the fallback if the hybrid grammar proves too costly in the walker.
|
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).
|
||||||
|
|
||||||
### 6. Phased implementation
|
### 6. Non-goals
|
||||||
|
|
||||||
1. **Literal `INSERT … VALUES`** — including the `RETURNING` and
|
- **Binding / statement reconstruction.** Explicitly out. Execution stays
|
||||||
`ON CONFLICT …` variants (they exist today and the shortid-autofill
|
verbatim. (This was the rejected first instinct.)
|
||||||
path already carries those trailing clauses, so excluding them would
|
- **Collapsing command identity.** `Command::Insert` and
|
||||||
be the artificial split). The common case; converges execution on the
|
`Command::SqlInsert` stay distinct; **ADR-0033 Amendment 3 stands**.
|
||||||
DSL bound path; closes the proven validation gap **and** the
|
- **Changing auto-fill.** The simple-vs-advanced `serial`/`shortid`
|
||||||
error-value display in one move. Highest value, smallest blast radius.
|
auto-fill difference (`requirements.md` X4) is **untouched** here and
|
||||||
2. **`UPDATE … SET` literals** (bind literal assignments; expression
|
tracked separately as a possible bug.
|
||||||
assignments stay text).
|
- **A structural `SELECT`** and **a full typed SQL-expression AST** —
|
||||||
3. **Completion/highlighting** for the typed slots on the SQL surface
|
both out (queries and expressions stay text; ADR-0031's "no `Expr` AST"
|
||||||
(if not already delivered by the mechanism chosen in §5).
|
and ADR-0030 §4's full-surface guarantee stand).
|
||||||
|
|
||||||
`SELECT` and `INSERT … SELECT` (and every *expression* value position)
|
|
||||||
remain on the text side throughout — **by design and documented**, not by
|
|
||||||
omission. `RETURNING` / `ON CONFLICT` are not on this list: they are
|
|
||||||
clauses over a statement whose literal `VALUES` still bind (§1/§2), and
|
|
||||||
Phase 1 carries them.
|
|
||||||
|
|
||||||
### 7. Non-goals
|
|
||||||
|
|
||||||
- **A structural `SELECT`.** Not now, not as a consequence of this ADR.
|
|
||||||
Queries stay text. (If a future need ever forces `SELECT` toward an
|
|
||||||
owned AST, that is its own ADR with its own justification — flagged as
|
|
||||||
a watch-item, not a plan.)
|
|
||||||
- **A full typed SQL-expression AST.** Expressions stay validated text;
|
|
||||||
ADR-0031's "no `Expr` AST" stands. We do not impose the DSL's limited
|
|
||||||
`Expr` on the SQL surface (that would regress ADR-0030 §4's full-surface
|
|
||||||
guarantee).
|
|
||||||
- **Removing the verbatim path.** It remains the executor for every
|
|
||||||
expression and query position; this ADR narrows *where* it applies, it
|
|
||||||
does not delete it.
|
|
||||||
|
|
||||||
## Consequences
|
## Consequences
|
||||||
|
|
||||||
- **Advanced mode stops being a feedback-free zone for data values.** A
|
- **Advanced mode stops being a feedback-free zone for data values.** A
|
||||||
learner typing a malformed `date`/`shortid`/`int` literal in a SQL
|
learner typing a malformed `date`/`shortid`/`int` literal in a SQL
|
||||||
`INSERT` gets the same catch-and-explain they get in simple mode.
|
`INSERT` gets the same catch-and-explain they get in simple mode —
|
||||||
- **One model, fewer paths.** Literal DML converges on the DSL's proven
|
via the *shared validator*, not a shared command.
|
||||||
parameter-binding execution; the two surfaces stop diverging on the
|
- **The modes stay cleanly separate.** Execution, auto-fill, and command
|
||||||
case they share.
|
identity are all unchanged; the only thing now shared is the value
|
||||||
- **The full SQL expression + query surface is preserved**, because
|
validators. This is the `requirements.md` X5 principle in practice
|
||||||
expressions and `SELECT` are explicitly left on the text side. No
|
(share a mechanic, not a command) and avoids the consolidation traps
|
||||||
regression to the advanced surface ADR-0030 §4 protects.
|
(X4 auto-fill) that the bind/converge approach would have hit.
|
||||||
- **Rework, not rewrite.** ADR-0033's `Sql*` command variants and the
|
- **Small, low-risk, no execution reconstruction.** Because we do not
|
||||||
worker handlers stay; they gain typed-literal payloads and the literal
|
rebuild the statement, there is no "mixed `VALUES (?1, expr, ?2)`"
|
||||||
execution path changes. Phased so each step is independently shippable
|
splicing problem, no multi-row execution change, and no `RETURNING`/
|
||||||
and testable, with the existing ADR-0033 tests as the regression net.
|
`ON CONFLICT`/`INSERT … SELECT` special-casing — they keep working as
|
||||||
- **A new seam to keep honest:** the literal-vs-expression boundary in
|
the existing ADR-0033 tests assert.
|
||||||
the grammar. It must be explicit and tested (a position that is a bare
|
- **One new seam to keep honest:** the literal-vs-expression
|
||||||
literal binds; a position that is any expression stays text; a signed
|
classification at parse. It must be tested (single literal / signed
|
||||||
numeric is a literal; `NULL`/`true`/`false` are literals), or it will
|
literal / `NULL`/`true`/`false` → validated; arithmetic / function /
|
||||||
drift.
|
subquery → skipped), or it will drift.
|
||||||
- **Execution reconstruction is the load-bearing implementation
|
- **A normalization difference is *avoided*, not introduced.** We
|
||||||
challenge.** A row mixing literals and expressions must be re-emitted
|
validate the literal but do not rewrite it; the engine stores the
|
||||||
as `VALUES (?1, expr-text, ?2)` — bound placeholders spliced with the
|
user's text as written. (Had we bound/normalized, advanced inserts
|
||||||
original expression spans, with any `RETURNING`/`ON CONFLICT` tail
|
might store a canonicalised value — a behaviour change we sidestep.)
|
||||||
re-appended. This is the strongest argument for the **typed-slot
|
- **Phase 3 will revisit literal *detection*** (swapping the parse-time
|
||||||
grammar** mechanism (§5): it yields per-position structure (slot →
|
classification for typed slots that also drive hints). The
|
||||||
bind; `sql_expr` → text span) directly, whereas the post-parse pass
|
validation/enrichment built on it is permanent; only the detection is
|
||||||
must recover those spans itself. The existing shortid-autofill rewrite
|
provisional — a deliberate, documented small throwaway.
|
||||||
(`plan_shortid_autofill`) already does statement reconstruction with
|
|
||||||
bound params + a preserved trailing clause, so the pattern is not
|
|
||||||
new — but it must be got right or a literal silently becomes text.
|
|
||||||
- **Cost if we are wrong about `SELECT`.** If a future requirement shows
|
|
||||||
`SELECT` genuinely needs an owned AST, this ADR does not help with it —
|
|
||||||
but nor does it obstruct it. The boundary chosen here (queries = text)
|
|
||||||
is the current best line, taken with eyes open.
|
|
||||||
|
|
||||||
## See also
|
## See also
|
||||||
|
|
||||||
- ADR-0030 §4 — the execute-path split this ADR narrows for DML (the
|
- ADR-0030 §4 / ADR-0033 §10 — the execute-path this ADR **augments**
|
||||||
`SELECT` half stands).
|
(adds value validation); the verbatim execution model and the
|
||||||
- ADR-0033 — the SQL DML grammar + the verbatim execution model this ADR
|
`SELECT`/expression text path both stand.
|
||||||
amends for literal values.
|
- ADR-0033 Amendment 3 — the two-command identity, **preserved** (this ADR
|
||||||
- ADR-0035 — the DDL precedent: structural, not verbatim ("verbatim was
|
does *not* collapse `Insert`/`SqlInsert`).
|
||||||
a DML convenience, not a rule").
|
- ADR-0035 — the DDL precedent (structural, not verbatim); this ADR is the
|
||||||
|
narrower DML analogue (validate, don't restructure).
|
||||||
- ADR-0026 — the DSL's deliberately-limited `Expr`; *not* imposed on the
|
- ADR-0026 — the DSL's deliberately-limited `Expr`; *not* imposed on the
|
||||||
SQL surface here.
|
SQL surface. ADR-0031 — `sql_expr` is validate-only; unchanged.
|
||||||
- ADR-0031 — `sql_expr` is validate-only (no `Expr` AST); unchanged.
|
- ADR-0032 — `SELECT` feedback-from-walk; the proof that text-to-engine is
|
||||||
- ADR-0032 — `SELECT` feedback-from-walk; the proof that text-to-engine
|
right for queries.
|
||||||
is right for queries.
|
- ADR-0029 — the column type/constraint model the shared validators
|
||||||
- ADR-0029 — the column type/constraint model the literal validators
|
|
||||||
enforce.
|
enforce.
|
||||||
- ADR-0035 Amendment 1 (F2 follow-up) — the neutral "that value" safety
|
- ADR-0035 Amendment 1 (F2 follow-up) — the neutral "that value" safety
|
||||||
net that remains correct for computed values.
|
net, correct for computed values.
|
||||||
|
- `requirements.md` **X4** (auto-fill difference — possible bug, untouched
|
||||||
|
here) and **X5** (framework cohesion / share-mechanics-not-commands —
|
||||||
|
the principle this ADR follows).
|
||||||
|
|||||||
+1
-1
File diff suppressed because one or more lines are too long
@@ -576,6 +576,38 @@ handoff-14 cleanup; 449 after B2/C2.)
|
|||||||
- [~] **X3** Accessibility: TUI screen-reader support is
|
- [~] **X3** Accessibility: TUI screen-reader support is
|
||||||
best-effort and not a v1 commitment; revisit if user need
|
best-effort and not a v1 commitment; revisit if user need
|
||||||
emerges.
|
emerges.
|
||||||
|
- [~] **X4** Auto-fill semantics differ between simple and advanced
|
||||||
|
mode — **possible bug, to investigate** (raised 2026-05-26). The
|
||||||
|
simple-mode insert (`do_insert`, `db.rs`) auto-fills an omitted
|
||||||
|
**non-PK `serial`** column with `MAX(col)+1` *and* generates
|
||||||
|
`shortid`s; the advanced-mode SQL insert (`plan_shortid_autofill`)
|
||||||
|
auto-fills **only `shortid`** and does **not** do non-PK `serial`
|
||||||
|
MAX+1. The `shortid` distinction is intentional; the non-PK-`serial`
|
||||||
|
gap looks unintended (no obvious reason advanced mode should skip
|
||||||
|
filling a `serial`). Decide whether advanced mode should match
|
||||||
|
simple mode's serial auto-fill (likely yes) or whether the
|
||||||
|
difference is deliberate, and align ADR-0018 accordingly. Left
|
||||||
|
untouched by the ADR-0036 value-validation work (which is surgical
|
||||||
|
and does not change auto-fill).
|
||||||
|
- [~] **X5** Framework cohesion / restructuring — **strategic,
|
||||||
|
revisit later** (raised 2026-05-26). The grammar/execution
|
||||||
|
framework (lexer → walker `Node`s → unified grammar tree → typed
|
||||||
|
`Command`s → executors; ADR-0023/0024 and everything layered on
|
||||||
|
since) grew organically from the original grammar outline and now
|
||||||
|
reuses + mixes elements across many levels without a *cohesive
|
||||||
|
written specification* of what the framework comprises, which
|
||||||
|
elements are meant to be reusable, and where the boundaries sit for
|
||||||
|
the recurring "reuse vs create new" decision. A concrete symptom:
|
||||||
|
commands are coupled tightly enough to their execution that reusing
|
||||||
|
an *execution behaviour* tempts reusing the whole *command* (see the
|
||||||
|
ADR-0036 discussion — the temptation to emit `Command::Insert` from
|
||||||
|
the advanced path just to reuse `do_insert`). Desired end state
|
||||||
|
(user-stated): **unique commands for every unique case, with a clean,
|
||||||
|
documented structure for reusable mechanics** (so execution helpers
|
||||||
|
are shared as library functions, not by collapsing command
|
||||||
|
identity). Consider a dedicated specification + restructuring run
|
||||||
|
(its own ADR) to map the framework and set the reuse-boundary rules,
|
||||||
|
easing future maintenance and extension. Not scheduled.
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user