do_rebuild_from_text re-emitted table-level CHECKs into the recreated
DDL (so they stayed enforced) but never repopulated __rdbms_playground_
table_checks. A fresh rebuild (missing .db, reconstructed from
project.yaml) therefore left the CHECK metadata empty: DROP CONSTRAINT,
describe, and a later save would lose it — including a named CHECK's
name. In-place rebuilds only worked because the wipe never touched the
table. (Latent since 4a.3 for unnamed checks; exposed by 4g's named
round-trip claim.)
Rebuild now wipes and repopulates CHECK_TABLE from the yaml snapshot
(name + seq + expr), like META/REL, and adds the 4g `name` column if a
pre-4g table predates it (the rebuild-only migration). Regression test:
a named CHECK's metadata survives a fresh rebuild (DROP CONSTRAINT by
name resolves).
ALTER TABLE <T> ADD [CONSTRAINT <name>] (CHECK | UNIQUE | FOREIGN KEY)
and DROP CONSTRAINT <name>. ADD = table-CHECK + composite UNIQUE + FK
(ADD PRIMARY KEY and a named UNIQUE refused — composite UNIQUE is
anonymous in our model). Each ADD reuses a low-level path with a dry-run
guard (table-CHECK/UNIQUE rebuild; FK -> add_relationship, bare
REFERENCES -> parent single PK). DROP CONSTRAINT resolves the name to a
named table-CHECK then a child-side FK, else refuses. One undo step each.
Named table-CHECKs round-trip: a nullable `name` column on
__rdbms_playground_table_checks (rebuild-only arrival; a named add on a
pre-4g project is refused with a "rebuild first" hint) plus a project.yaml
check_constraints {expr, name} extension (bare-string form still reads).
The internal-__rdbms_* guard was folded into do_add_constraint /
do_add_relationship, completing that guard class.
Grammar: the action Choice keeps one branch per verb (add/drop/rename/
alter) with an inner Choice fanning out on the distinct second keyword,
since the walker's Choice does not backtrack between same-led branches.
Tests: 7 Tier-1 parse + 2 yaml round-trip + 1 internal-guard + 9 Tier-3
e2e. Help/usage refreshed; ADR-0035 §13 4g + README + requirements.md in
lockstep.
Fourth AlterTableAction (AlterColumnType), runtime-decomposed to the
existing change_column_type executor with ForceConversion — which IS the
§7 advanced policy: lossy converts with a note (no force flag),
incompatible + the ADR-0017 static refusals (↔blob, same-type,
date↔datetime, non-int→serial) still refuse, while int→serial is allowed
(auto-fills nulls + UNIQUE, ADR-0018 §8). No new mode/note/persistence;
undo is the advanced safety net.
Grammar adds a fourth action branch leading on `alter`, discriminated in
the builder by the `type` keyword (unique — ADD COLUMN's type is an
ident); the type slot reuses SQL_TYPE. The internal-__rdbms_* guard was
folded into do_change_column_type (user-confirmed), closing the simple
`change column` exposure.
Tests: 7 Tier-3 e2e via run_replay + 4 Tier-1 parse (incl. a column-named-
`type` discriminator probe) + the simple-surface guard. Help/usage
refreshed; ADR-0035 §13 4f + README + requirements.md in lockstep.
Advanced-only `alter` entry word; ALTER TABLE <T> ADD COLUMN <col> <type>
[constraints] | DROP COLUMN <col> | RENAME COLUMN <old> TO <new> ->
SqlAlterTable, runtime-decomposed to the existing column executors
(do_add_column / do_drop_column / do_rename_column) — one undo step each,
no new worker layer. The COLUMN keyword is required (reserves bare
RENAME TO for 4h, ADD CONSTRAINT for 4g).
- ADD COLUMN takes NOT NULL / UNIQUE / DEFAULT / CHECK (no PK / inline
REFERENCES). do_add_column extended to consume the SQL raw-text
default_sql / check_sql (sql_expr is validate-only, the 4a.2
mechanism), reaching parity with CREATE TABLE's column constraints.
- Drop/rename column refuse a column any CHECK references — table-level
AND column-level (incl. a column's own self-check on rename) — the
4a.3 deferral, detected up-front by tokenizing the raw CHECK text
(skipping string literals). In the shared executors, so it guards both
the simple and SQL surfaces and fixes a latent rename-drift bug that
desynced the stored CHECK text and broke rebuild.
- SQL DROP COLUMN refuses an index-covered column (no --cascade SQL
spelling — matches SQLite + the simple default).
- The column executors and do_add_index gained an internal-__rdbms_*
guard (refuse as "no such table"), closing a pre-existing exposure on
both surfaces. (do_change_column_type / do_add_constraint /
do_add_relationship are a tracked follow-up.)
- `alter` is advanced-only; AlterTableAction::AddColumn is boxed
(clippy::large_enum_variant).
Docs: ADR-0035 status + §13 4e; ADR README; requirements.md Q1. Plan:
docs/plans/20260525-adr-0035-sql-ddl-4e.md.
Tests: 1854 passing / 0 failing / 0 skipped / 1 ignored; clippy clean.
Advanced-mode SQL CREATE [UNIQUE] INDEX [IF NOT EXISTS] [<name>] ON
<T> (cols) -> SqlCreateIndex and DROP INDEX [IF EXISTS] <name> ->
SqlDropIndex, both reusing the ADR-0025 executors (do_add_index /
do_drop_index), like 4c reused do_drop_table.
- CREATE UNIQUE INDEX admitted in advanced mode (ADR-0025 Amendment 1):
ADR-0025 deferred UNIQUE indexes for the simple-mode DSL, but advanced
mode trusts the user like SQL does. Adds an additive IndexSchema.unique
flag (project.yaml, serde-default, version stays 1); rebuild re-emits
CREATE UNIQUE INDEX; the redundant-set guard keys on (columns, unique).
Simple-mode `add unique index` stays deferred.
- IF [NOT] EXISTS on both forms reuses the 4c no-op-with-note skip
(journalled, not snapshotted) via CreateIndexOutcome / DropIndexOutcome.
- Unnamed CREATE INDEX auto-named (ADR-0025 convention); the [UNIQUE]
prefix is a concrete-keyword Choice and the optional name an on-led-first
selector (the drop-index selector precedent) — trap-safe.
- create/drop each gain a second advanced node; the existing all-candidates
dispatch handles it (locked by parse tests).
- Unique indexes marked [unique] in the structure view and items panel.
- do_add_index refuses internal __rdbms_* tables as "no such table",
closing a latent exposure on both the simple `add index` and the new
SQL CREATE INDEX surfaces (ADR-0025 Amendment 1).
Docs: ADR-0035 status + §13 4d + 4i; ADR-0025 Amendment 1; ADR README;
requirements.md Q1/C3. Plan: docs/plans/20260525-adr-0035-sql-ddl-4d.md.
Tests: 1834 passing / 0 failing / 0 skipped / 1 ignored; clippy clean.
Add advanced-mode SQL `DROP TABLE [IF EXISTS] <name>` -> SqlDropTable,
executing through the existing do_drop_table (cascade / inbound-
relationship refusal / metadata cleanup) — full parity with the simple
`drop table`. The only new behaviour is `IF EXISTS` as a
no-op-with-note: a new DropOutcome::Skipped mirroring
CreateOutcome::Skipped (journalled, no snapshot), rendered via a new
ddl.drop_skipped_absent note + DslDropSkipped event.
- Grammar: SQL_DROP_TABLE node (entry `drop`, shape `table [if exists]
<name> [;]`), registered Advanced. SQL-first dispatch: `drop table T`
-> SqlDropTable in advanced; `drop column`/`relationship`/`index`/
`constraint` fall back to the simple `drop` node (and still execute).
- Worker: Request::SqlDropTable + db.sql_drop_table; the if-exists-and-
absent arm journals + replies Skipped without a snapshot, else
snapshot_then(do_drop_table) -> Dropped.
- Completion: advanced `drop ` now surfaces the SQL `table` (the
shared-entry-word behaviour from `create`); test split into simple
(full DSL list) + advanced (SQL surface).
Known shared-entry-word completion unevenness (advanced `drop ` offers
only `table`; partial `drop rel` returns an empty list) deferred to 4i
(merge candidate sets for shared entry words) along with a flagged user
request to visually distinguish simple- vs advanced-mode completions in
the hint UI — tracked in ADR §13 4i (d)/(e), the 4c plan, and the
completion test. The DSL drops still parse + execute via fallback.
10 new tests (parse/builder + Tier-3: drop existing + one-undo-step +
restore, IF EXISTS skip + journal, plain-absent error, inbound refusal).
Docs: ADR-0035 Status/§13, README, requirements.md Q1.
Tests: 1805 passing, 0 failing, 1 ignored. Clippy clean.
Add foreign keys to advanced-mode SQL CREATE TABLE — the SQL spelling of
an ADR-0013 named relationship, created in the same transaction as the
table (one undo step).
- Grammar: inline `<col> … REFERENCES <parent>[(<col>)] [ON DELETE/UPDATE
…]` (a new column constraint) and table-level `[CONSTRAINT <name>]
FOREIGN KEY (<col>) REFERENCES …` (two new element branches — both
start on a concrete keyword, never a leading Optional, which would
abort the element Choice). Referential clauses reuse
shared::REFERENTIAL_CLAUSES.
- Builder: greedy FK-clause consumption (parens consumed internally so
they don't perturb the 4a.3 element-boundary depth tracker); inline FK
auto-named, table FK takes an optional CONSTRAINT name.
- Worker: do_create_table resolves + validates each FK before building
the DDL (self-ref validates against the in-statement columns/PK; bare
REFERENCES resolves to the parent's single-column PK, composite ->
error; PK-target + Type::fk_target_type compatibility), emits the
FOREIGN KEY clause identically to schema_to_ddl, and writes the
relationship metadata in the create transaction.
- Reuse: name/uniqueness/metadata-insert/type-compat factored into shared
helpers; do_add_relationship refactored to use them.
- FKs round-trip via the existing relationship plumbing (no new
persistence structures); describe surfaces the relationship.
Self-references and bare `REFERENCES <parent>` supported (user-confirmed).
Self-ref pre-submit indicator wrinkle deferred to 4i (tracked in ADR §13,
a code comment, and the plan).
DA/runda round added cross-cutting probes (FK survives the add-column
rebuild + a later rebuild_from_text; referential actions survive rebuild;
drop-child clears the relationship; drop-parent refused; bare self-ref
resolves to own PK) — all green, no fixes needed.
27 new tests (grammar/builder + Tier-3). Docs: ADR-0035 Status/§13,
README, requirements.md Q1.
Tests: 1795 passing, 0 failing, 1 ignored. Clippy clean.
Add table-level CHECK (e.g. `CREATE TABLE t (a int, b int, CHECK (a < b))`)
to advanced-mode SQL CREATE TABLE. Since SQLite exposes no PRAGMA for CHECK
constraints, a table-level CHECK cannot be read back from the engine and
becomes the source of truth in a new internal metadata table
`__rdbms_playground_table_checks (table_name, seq, check_expr)`.
- Grammar: new TABLE_CHECK element in ELEMENT_CHOICES.
- Builder: distinguishes a table-level CHECK from a column-level one by
element position (no column-def open in the element), using depth-aware
boundary tracking so a length-arg comma (`numeric(10,2)`) or a
table-PRIMARY KEY's inner comma is not mistaken for an element separator.
- Worker: do_create_table emits the CHECK clauses and writes the metadata
rows in its transaction; schema_to_ddl emits them identically on rebuild;
read_schema / read_schema_snapshot read them from the metadata table;
do_drop_table clears them.
- Persistence: TableSchema.check_constraints round-trips through project.yaml
(#[serde(default)], optional on read), mirroring unique_constraints.
- Composite UNIQUE deliberately stays PRAGMA-detected (engine-reportable,
unlike CHECK) — user-confirmed.
DA/runda round added cross-cutting tests and a forward-looking doc fix:
- table CHECK survives a rebuild triggered by `add column`, and a later
rebuild_from_text (the ADR-0013 rebuild primitive uses a raw DROP, so the
metadata rows keyed on the final name are preserved);
- dropping a column a table CHECK references fails cleanly (rollback, table
intact); detection is 4e, friendly wording is H1;
- dropping a table clears its CHECK metadata (no orphan rows on re-create);
- amended ADR §6 so 4h's RENAME also updates the new metadata table.
20 Tier-3 + 9 grammar/builder + 2 YAML tests. Docs: ADR-0035 Status/§13/§6,
README index, requirements.md Q1. Help/usage skeleton + describe display of
table-level constraints deferred to 4i (symmetric with 4a.2).
Tests: 1769 passing, 0 failing, 1 ignored. Clippy clean.
Advanced-mode SQL CREATE TABLE gains the constraints that need no new
internal table (the 4a.2 slice):
- Grammar (sql_create_table.rs): column-level DEFAULT/CHECK and
table-level UNIQUE(cols). DEFAULT is a literal or a *parenthesised*
expression (standard SQL) — a bare sql_expr greedily eats a following
NOT (NOT IN/LIKE/BETWEEN), breaking `DEFAULT 0 NOT NULL`; the parens
bound it. CHECK is paren-bounded already.
- Builder (ddl.rs): captures CHECK/DEFAULT raw SQL text by byte span
(sql_expr builds no AST) via capture_parenthesised_span /
capture_expr_span; routes single-column table UNIQUE into the
column's flag and composite UNIQUE into unique_constraints.
- Command/worker: ColumnSpec gains check_sql/default_sql (raw, preferred
over the typed Expr/Value); Command::SqlCreateTable + Request +
do_create_table gain unique_constraints; do_create_table emits raw
CHECK/DEFAULT and composite UNIQUE clauses.
- Round-trip (part D): ReadSchema/TableSchema gain unique_constraints;
read_schema detects composite UNIQUE via PRAGMA index_list origin 'u'
(single-column still folds to the column flag); schema_to_ddl emits
them; YAML RawTable/write_table round-trips (optional-on-read).
CHECK round-trips via __rdbms_playground_columns.check_expr, DEFAULT
via PRAGMA table_info — no new metadata table.
Table-level/multi-column CHECK remains 4a.3 (rejected "not yet
supported"); FK is 4b.
Tests: +7 builder (raw-text capture incl. the DEFAULT 0 NOT NULL
boundary the fix was found by; single/composite UNIQUE routing) and +4
Tier-3 (CHECK enforced, DEFAULT applied, composite UNIQUE enforced, and
all three survive a rebuild — the part-D round-trip). 1752 pass / 0 fail
/ 1 ignored; clippy clean. Plan + requirements.md updated.
Command + builder + worker for advanced-mode SQL CREATE TABLE
(sub-phase 4a), executed structurally through do_create_table:
- Command::SqlCreateTable + build_sql_create_table (ddl.rs): aliases via
from_sql_name (incl. double precision), column- and table-level
PRIMARY KEY, redundant-flag de-dup off a sole PK, IF NOT EXISTS.
Advanced REGISTRY entry on the shared `create` word (SQL-first, DSL
fallback); no-PK tables allowed (user-confirmed).
- Worker (db.rs): Request::SqlCreateTable + CreateOutcome + snapshot_then
(one undo step); IF NOT EXISTS no-op (no snapshot, but journalled, like
read-only commands). do_create_table inline-PK rule aligned with the
rebuild generator schema_to_ddl — no round-trip DDL drift; serial
autoincrement is independent of inline-PK (verified by round-trip
tests).
- Runtime/App: dispatch + CommandOutcome::SchemaSkipped +
AppEvent::DslCreateSkipped (structure + "already exists — skipped"
note). Friendly catalog keys added (engine-neutral).
DEFAULT/CHECK/table-level UNIQUE are absent from the 4a grammar (parse
error with usage skeleton; friendly message + support land in the 4a.2
constraint slice) — user-confirmed.
Tests: type resolver, grammar shape, builder (incl. the PK
detection bug they caught), and tests/sql_create_table.rs (worker
round-trip, serial autoincrement first/non-first across rebuild, IF NOT
EXISTS no-op + journalling, no-PK table, one undo step) + a replay-as-
write test. 1739 pass / 0 fail / 1 ignored; clippy clean.
Exit gate: ADR-0035 Proposed -> Accepted (validated end-to-end by 4a);
README + requirements.md Q1 updated.
Advanced-mode SQL type slot accepts the ten playground keywords plus the
standard-SQL aliases (integer/varchar/timestamp/numeric/float/double
precision/binary/..., case-insensitive). Simple-mode FromStr is unchanged
(rejects aliases). Unknown names -> None for the friendly diagnostic.
/runda found silent data loss: with the non-fatal snapshot-failure
policy, a committed mutation whose snapshot couldn't be staged left
the redo stack stale (redo-clear was only a side effect of finalize),
so a later redo silently discarded the new work. Same gap in batches.
- SnapshotStore::clear_redo() drops the redo stack + payloads
- snapshot_then / end_batch call it when committed user work has no
staged snapshot; for disk-full it succeeds where a full backup
couldn't (tiny index write + payload deletes)
- unit test + integration regression (forced staging failure)
- ADR-0006 implementation note records the fix + residual edge
1698 passed / 0 failed / 1 ignored; clippy clean.
The undo ring is local working state, handled at all three
project-file seams (R13):
- .gitignore template ignores /.snapshots/
- export excludes .snapshots/ (like playground.db / history.log)
- safely_delete_temp_project allowlists .snapshots/ so a temp that
was modified then undone back to empty stays auto-deletable
- undo::SNAPSHOTS_DIR is now a pub const referenced by all three
- tests: gitignore content, export exclusion, cleanup allowlist
1693 passed / 0 failed / 1 ignored; clippy clean.
- snapshot_then() brackets all 19 mutating dispatch arms: stage a
pre-op snapshot, finalise on success / discard on rollback; gated
on a user command source (internal ops like open-time rebuild are
not snapshotted) and on undo being enabled
- BatchState + BeginBatch/EndBatch requests: a batch takes one
boundary snapshot, suppresses per-command snapshots, and finalises
iff a mutation committed (one undo step per replay/batch)
- Undo/Redo/PeekUndo/PeekRedo requests handled in worker_loop with
&mut conn for the restore; cleanup() sweeps crash leftovers on open
- Database::{undo,redo,peek_undo,peek_redo,begin_batch,end_batch} +
open_with_persistence_and_undo(); snapshot failures are non-fatal
(logged), restore failures surface
- 6 Tier-3 integration tests through the real worker
1680 passed / 0 failed / 1 ignored; clippy clean.
Replay (§3): run_replay parses <ts>|<status>|<source> journal records — runs ok, skips non-ok — while still accepting bare .commands scripts (prefix-detected so a | inside a bare command isn't misread). Fixes replay history.log, which died on line 1.
Journal failures (§1/§2): failed commands are recorded err via a new Action::JournalFailure, emitted by the pure-sync App for both parse failures and worker-execution failures (runtime appends best-effort, never fatal). Hydration reads all records so typo'd/rejected commands are recallable across sessions.
Amendment 1 — replay filters app-lifecycle commands: a working replay history.log exposed that the journal also records save as/load/new/export/import/rebuild/mode (which would panic the worker dispatch or abort replay). Replay now re-applies only schema/data writes and skips every app-lifecycle command + nested replay, classified by entry word so modal/incomplete forms (save as, bare mode) and quit skip uniformly rather than aborting. All skips continue (reversing the nested-replay refusal); import and nested replay warn. replay.error_nested removed; replay.skipped_import/_replay added; ReplayCompleted carries warnings. requirements.md U3/U4 updated; app-command runtime-failure journalling tracked as a follow-up.
1659 passing / 0 failing / 0 skipped / 1 ignored. Clippy clean.
Wire `insert`/`update`/`delete` as shared DSL/SQL entry words through the
category-grouped dispatcher (ADR-0033 Amendment 1): the Advanced SQL nodes
move off the dev words (`sqlinsert`/`sql_update`/`sql_delete`) to the real
keywords, registered alongside the Simple DSL nodes. Remove the dev-word
scaffold; collapse build_sql_{insert,update,delete} to source.trim();
de-duplicate the two REGISTRY entry-word listing sites.
Dispatch model (ADR-0033 Amendment 3, written this round):
- A command is the mode-rooted grammar-path outcome; identity is intrinsic.
Advanced mode tries SQL first, falling back to the Simple DSL command when
no SQL branch matches a token (`delete … --all-rows` falls back;
`update … --all-rows` does not — the SET expression absorbs it, harmless
since the engine treats `--all-rows` as a comment).
- Simple mode commits the DSL candidate for a shared word, surfacing the real
DSL error; bare "this is SQL" is reserved for SQL-only entry words
(`select`/`with`). A content rejection on the SQL candidate (internal
table) is committed, never masked by the DSL fallback.
Combined DSL-error + advanced-SQL pointer (ADR-0033 Amendment 3): a Simple-mode
definite DSL error that would run as SQL in advanced mode gains the
`advanced_mode.also_valid_sql` suffix — in the live hint (ambient_hint_in_mode)
and on submit (dispatch_dsl), via the shared advanced_alternative_note — so the
actionable DSL fix and the mode pointer coexist (submit covers constructs that
surface only on submit, e.g. `delete … returning`).
Internal-table rejection symmetrised (/runda finding B, ADR-0030 §6): the DSL
data-command target slots (insert/update/delete/show data/show table) gained
reject_internal_table, so `__rdbms_*` tables are refused in Simple mode too —
previously only the advanced SQL grammar rejected them.
Mode-awareness: classify_input_with_schema_in_mode and
invalid_ident_at_cursor_in_mode stop leaking the advanced SQL view into
simple-mode hints for shared words.
Tests: dev-word inputs migrated to the real words (advanced); DSL grammar /
completion / phase-D / db tests parse in Simple mode (the DSL surface); replay
keeps its advanced-mode model (one stale assertion fixed); dispatcher routing,
combined-pointer, and internal-table tests added. Suite 1626 pass / 0 fail /
1 ignored; clippy --all-targets -D warnings clean.
Defer M4 (execution-time mode side-channel; tracked in requirements.md) to its
own ADR.
A focused adversarial round (/runda) found a single root cause with
six manifestations, all pre-existing latent false-positives: the
INSERT target is recorded under the `insert_target_table` role, not
as a diagnostic `bindings` entry, so refs that should resolve to the
*target* row were instead checked against the statement's bindings —
which for an `INSERT … SELECT` are the SELECT's *source* tables (the
wrong scope), producing false unknown_column / unknown_qualifier
diagnostics on valid input.
New helper bare_ref_insert_target re-scopes a ref onto the INSERT
target when it sits in a target-referencing region: the UPSERT
DO UPDATE action (byte range) or an INSERT's RETURNING list. Applied
across every ref form:
1. INSERT column list (insert_column) — validated vs the target,
skipped in the bare-column branch (was checked vs SELECT source).
2. ON CONFLICT (col) target (conflict_target_column) — same.
3. DO UPDATE SET RHS / WHERE bare refs — validated vs the target
(also closes the #12 residual for VALUES upserts).
4. RETURNING bare refs — validated vs the target.
5. target-qualified refs `t.col` in DO UPDATE / RETURNING — the
unified `excluded` / target-qualifier resolution in the
qualified-ref None branch.
6. target-qualified star `t.*` in RETURNING — same re-scoping in
the qualified-star handler.
Each fix has a positive (resolves cleanly) and negative (genuinely
unknown column / unrelated qualifier still flagged) test; the
`excluded` leak guard and all prior diagnostics remain green.
1613 pass / 0 fail / 1 ignored. Clippy clean.
DA pass on 3i. Fix: build_schema_cache set not_null = c.notnull ||
c.primary_key, which would false-flag an omitted `int` PK as a
not_null_missing WARNING — but an int PK is an INTEGER PRIMARY KEY
rowid alias that auto-fills (and SQLite's PK-NULL quirk means a PK
isn't implicitly NOT NULL anyway). Use c.notnull alone (ADR-0033
§8.3 "declared NOT NULL"): faithful and false-positive-free.
Arity-walk hardening (same class as the ON CONFLICT regression the
existing tests caught mid-3i): RETURNING after VALUES is a depth-0
keyword that ends the tuple list (only the real tuple is flagged),
and a comma nested in a function-call value (depth ≥ 2) does not
inflate the tuple's value count.
Tests (+2). 1598 pass / 0 fail / 1 ignored. Clippy clean.
New dml_target_column_diagnostics pass: an ERROR for an unknown column
in the INSERT column list or the UPSERT DO UPDATE SET (validated
directly against the insert_target_table). The INSERT target isn't a
flat-scope `bindings` entry, so the existing schema-existence pass
didn't cover these; a targeted pass avoids the false INSERT…SELECT
ambiguity a global binding would cause.
Closes the 3i cross-cut "schema-existence fires on INSERT VALUES"
gate item, and closes the DA finding #12 (UPSERT DO UPDATE SET column
now flagged like a top-level UPDATE's SET column). Residual: bare
sql_expr_ident refs in the DO UPDATE SET RHS / WHERE remain
unvalidated for upserts (the documented flat-scope limitation).
Tests (+5): unknown INSERT column flagged + known silent; unknown
DO UPDATE SET column flagged + known/excluded silent; predicate
warning (= NULL) fires on a SQL UPDATE WHERE (cross-cut). 1596 pass /
0 fail / 1 ignored. Clippy clean.
Extend SchemaCache TableColumn with not_null + has_default (with a
TableColumn::new constructor for the common no-constraint case),
populated in build_schema_cache from ColumnDescription (a PK column
counts as not-null). New dml_not_null_missing_diagnostics pass: a
WARNING when a SQL INSERT's explicit column list omits a column that
is NOT NULL with no DEFAULT — advisory (the engine enforces it).
serial/shortid (auto-filled) and defaulted columns are excluded.
Anchored on the target-table ident (no token for the omitted column).
Catalog key diagnostic.not_null_missing (engine-neutral). Tests (+4):
fires on omitted required column; silent when included, when
defaulted, and for auto-gen serial/shortid. ~24 TableColumn literal
sites updated for the two new fields (build clean). 1591 pass / 0
fail / 1 ignored. Clippy clean.
All three ADR-0033 §8 DML diagnostics now implemented. Remaining 3i:
cross-cut verification + #12 UPSERT DO UPDATE validation.
New dml_insert_arity_diagnostics pass (ERROR): when an explicit
(column_name_list) arity disagrees with a row's arity. VALUES tuples
are checked per-row (each offending tuple emits its own diagnostic on
its span; matched rows stay silent). INSERT … SELECT compares the
first SELECT leg's projection arity, anchored on the first projection
item; a WITH-prefixed row source is skipped (engine still reports it —
a false positive would be worse). No-column-list form deferred
(needs schema; outside the 3i gate).
The VALUES walk stops at the first depth-0 keyword so an ON CONFLICT
(col) conflict target / RETURNING tail is not mis-counted as a value
tuple (caught by the existing upsert_excluded tests during dev).
Catalog key diagnostic.insert_arity_mismatch (engine-neutral).
Tests (+7): single-row + matched + per-row multi-row; INSERT…SELECT
mismatch + matched; ON CONFLICT interaction (only the real tuple
flagged, clean case silent). 1587 pass / 0 fail / 1 ignored. Clippy
clean. Remaining 3i: not_null_missing (needs TableColumn
not_null+default), cross-cut verification, #12 UPSERT DO UPDATE
validation.
New dml_auto_column_diagnostics pass: a WARNING when a SQL INSERT's
explicit column list names a serial/shortid (auto-generated) column —
the explicit value bypasses the auto-counter/generator and may collide
with later auto-generated values. Advisory only (ADR-0027 §1); the
statement still runs. Conflict-target columns (distinct
conflict_target_column role) are not mistaken for inserted columns.
Catalog key diagnostic.auto_column_overridden (engine-neutral).
Tests (+4): serial + shortid fire; omitted is silent; ON CONFLICT
target not falsely flagged. 1580 pass / 0 fail / 1 ignored. Clippy
clean. Remaining 3i: insert_arity_mismatch, not_null_missing (needs
TableColumn not_null+default), cross-cut verification, #12 UPSERT
DO UPDATE validation.
on_conflict_clause on SQL_INSERT_SHAPE: optional (col,…) conflict
target (distinct conflict_target_column role so it never enters
listed_columns), DO NOTHING / DO UPDATE SET … [WHERE …]. `do` is
factored out of the action Choice so nothing/update disambiguate
without tripping the walk_seq/walk_choice shared-prefix trap
(ADR-0033 Amendment 1). Worker runs the UPSERT verbatim (SQLite
native); no new execution path.
build_sql_insert: row_source now stops before the FIRST trailing
clause — ON CONFLICT (3h) or RETURNING (3g) — and do_sql_insert's
shortid auto-fill rewrite re-appends the whole trailing tail, so an
auto-filled INSERT keeps its ON CONFLICT / RETURNING.
excluded pseudo-table (§9): resolves to the target's columns inside
the DO UPDATE action and completes at `excluded.|`, but stays flagged
as unknown_qualifier in VALUES / RETURNING / non-upsert statements.
Diagnostic pass scopes it by the DO UPDATE byte-range (update token →
RETURNING/end); completion resolves it against the INSERT target's
current_table_columns. NOTE: scoping uses byte-range rather than the
plan's prescribed from_scope TableBinding push — same behaviour, no
walker scope-frame change.
Tests (+13): grammar accept/reject; DO NOTHING / DO UPDATE-excluded /
no-target execution + persistence; auto-fill × ON CONFLICT with a
REAL unique conflict (proves the clause survives the rewrite, not a
no-op); excluded resolves in DO UPDATE SET + WHERE, flagged in VALUES
(incl. same statement), unknown column under excluded; excluded.|
completion; conflict-target not in listed_columns. 1576 pass / 0 fail
/ 1 ignored. Clippy clean. Dev sql_insert entry word still removed in
3j.
Known follow-up (tracked for 3i): UPSERT DO UPDATE bare column refs
(SET LHS / WHERE) are not schema-validated, unlike regular UPDATE —
the INSERT target isn't a diagnostic binding. Fits 3i's cross-cut
SET/WHERE validation scope.
Shared RETURNING_CLAUSE (reuses Phase-2 PROJECTION_LIST, now
pub(crate)) as an optional tail on all three SQL DML shapes.
`returning: bool` on the Command variants, set by the ast-builders
and threaded to the worker. run_returning collects the returned rows
as a DataResult (RETURNING mutates + yields in one pass), reusing
resolve_select_column_types for bare-column type recovery; computed
projections stay typeless. DeleteResult gains a `data` field rendered
alongside the cascade summary.
Follow-set fix: `returning` is added to the table-source and
projection bare-alias follow-sets so an INSERT … SELECT row source
stops before RETURNING instead of reading it as a table alias.
Auto-fill × RETURNING: build_sql_insert stops row_source before the
RETURNING token (keeping it preparable for shortid materialisation),
and plan_shortid_autofill re-appends the RETURNING tail so generated
shortids surface in RETURNING *.
Tests (+17): grammar accept on all three; INSERT/UPDATE/DELETE
RETURNING incl. *, aliases, multi-row, type recovery + computed-
typeless; auto-fill × RETURNING (single + multi-row distinct ids);
INSERT…SELECT…RETURNING execution; UPDATE…RETURNING zero-match;
DELETE…RETURNING cascade+rows; app-level render of both. Dev
sql_insert/sql_update/sql_delete entry words still removed in 3j.
1562 pass / 0 fail / 1 ignored. Clippy clean.
A self-referential ON DELETE CASCADE FK (e.g. T.ParentId -> T.id) is
returned by read_relationships_inbound as a child whose table IS the
delete target. The before/after row-count diff then includes the
directly-deleted rows (already in rows_affected), so deleting a chain
root reported 3 cascaded rows when only 2 were removed via the
self-reference.
Fix in both do_delete (DSL) and do_sql_delete (SQL): when the child
table equals the target, subtract rows_affected from the diff and
guard on the corrected count (a leaf delete no longer reports a
phantom 0-row self-cascade); the target's CSV is already queued, so a
self-ref child is not re-added to rewritten_tables. Pre-existing in
do_delete; surfaced by the 3f DA pass, fixed in both paths to keep
DSL/SQL parity. Behaviour: report only the rows removed via the
self-reference (user-confirmed).
Also adds an app-level render test for the SQL DELETE path
(handle_dsl_delete_success via CommandOutcome::Delete) — the shared
renderer's ok-summary + per-relationship cascade line were exercised
only through the DSL path before.
Test-first: self_referential_cascade_counts_only_cascaded_rows added
for both paths (asserted 2, failed at 3 before the fix). 1545 pass /
0 fail / 1 ignored. Clippy clean.
New src/dsl/grammar/sql_delete.rs (FROM <table> [WHERE] [;]),
Command::SqlDelete, Request::RunSqlDelete, do_sql_delete worker.
do_sql_delete mirrors the DSL do_delete: detect FK cascade by
before/after child row-count diffing, re-persist target + every
cascade-affected child, history-on-success inside the tx. Reuses
CommandOutcome::Delete -> handle_dsl_delete_success, so the
per-relationship cascade summary formatter is shared, not duplicated.
ADR-0033 Amendment 2: supersedes §7's WHERE-injected pre-count. Its
premise (DSL handler builds pre-counts from the typed Expr) was wrong
— do_delete uses count-diff. The pre-count would also have broken the
§2 parity promise by reporting SET NULL the DSL path doesn't. Count-
diff gives exact parity, no WHERE-byte extraction, and withdraws R2.
SET NULL reporting deferred for both paths (user-confirmed).
Tests: +6 grammar unit, +12 integration (cascade parity with DSL,
both R2 subquery cases, before-execute order, no-WHERE, FK-rejection
rollback, childless-parent, two-child cascade). 1542 pass / 0 fail /
1 ignored. Clippy clean. Dev sql_delete entry word removed in 3j.
New src/dsl/grammar/sql_update.rs: SQL_UPDATE_SHAPE =
<table> SET col = sql_expr (',' …)* [WHERE sql_expr] [';'], the
__rdbms_* target rejection, and the shared sql_expr on both the
assignment RHS and the predicate. No --all-rows rail — a SQL
UPDATE without WHERE runs as written (ADR-0030 §12). Reuses
sql_select::WHERE_CLAUSE (now pub(crate)) so the predicate
diagnostics are identical. The target uses the shared `table_name`
ident role (not a bespoke one) so the Phase-2 schema-existence and
predicate-warning passes collect it as a scope binding and check
the SET / WHERE columns for free — a bespoke role left them
unchecked (the cross-cut tests caught this).
Command::SqlUpdate { sql, target_table }; Request::RunSqlUpdate +
do_sql_update (execute validated SQL via execute_with_fk_enrichment,
re-persist the target CSV, append history.log). 3e surfaces the
affected-row count only; precise row output is RETURNING (3g), so
the update-success render skips a column-less data set rather than
showing a misleading "(no rows)" band. Behind the dev `sql_update`
entry word until 3j.
Tests: grammar accept/reject; integration (single/multi-col,
no-WHERE all-rows, sql_expr in SET, scalar subquery in SET,
zero-match success, history); walker cross-cut (unknown SET column
→ unknown_column, `= NULL` in WHERE → eq_null warning); app-level
render-guard both ways (column-less → count only; with columns →
table renders). 1524 green, clippy clean.
plan_shortid_autofill read exactly listed_columns.len() cells from
the materialised row source. When the row source produced a
different column count than the user's list, the extra columns were
silently dropped (wider → wrong data, insert succeeded) or read
out of range (narrower). Guard: if the materialised statement's
column_count differs from the listed-column count, skip auto-fill
and execute the verbatim statement so the engine reports the
mismatch — matching the non-auto-fill path. A friendly pre-flight
diagnostic remains sub-phase 3i.
Tests: VALUES with too many values; INSERT…SELECT with a wider and
a narrower projection — each rejected with nothing persisted.
When an INSERT's column list omits one or more shortid columns,
the worker now fills them. Command::SqlInsert gains listed_columns
and row_source, captured in build_sql_insert from the matched path
(the row source is located by the first values/select/with Word
token, so a string literal like 'select' can't be mistaken for the
keyword). do_sql_insert calls plan_shortid_autofill, which — per
the user-confirmed Option B — materialises the row source by
running it as a query, generates a distinct shortid per row via the
existing generate_shortid_batch (deduped against stored values),
and reconstructs a parameterised multi-row INSERT over the listed
columns plus the omitted shortid columns. Uniform for VALUES and
INSERT…SELECT, and handles multiple omitted shortids in one row
(each gets its own batch). No explicit list, no omitted shortid, or
a zero-row source → execute verbatim (the 3b path). serial stays
engine-filled via rowid. history.log keeps the original line, never
the rewrite (§11).
Tests: VALUES single/multi-row distinct; explicit override
honoured; INSERT…SELECT distinct fills; combined serial(engine) +
shortid(worker); two shortids (PK + non-PK) both fill; one provided
+ one omitted; compound-PK shortid member; mixed-case column name
(ADR-0009 DA gate); original-source-in-history on the rewrite path.
Still behind the dev `sqlinsert` entry word (3j). 1503 green,
clippy clean.
Make the INSERT row source a Choice between the VALUES clause and
Subgrammar(&sql_select::SQL_SELECT_COMPOUND). SQL_SELECT_COMPOUND
is itself a Choice that admits a leading WITH, so a WITH-prefixed
SELECT row source (R4) parses through it for free; the two
branches start on disjoint keywords (values vs select/with) so the
Choice never ambiguously commits. No worker change — do_sql_insert
already executes the validated SQL and re-persists, and the engine
handles insert-from-query.
Tests: grammar accept (plain / column-list+projection / WITH-
prefixed / trailing-semi) and reject (__rdbms_* on the SELECT's
FROM slot, incomplete select); integration parse-path lowering +
worker round-trip (rows land, CSV re-persisted) incl. R4 WITH end-
to-end; walker cross-cut that the Phase-2 unknown_column diagnostic
fires on the INSERT…SELECT projection; DA-gate test that a self-
sourced INSERT…SELECT runs as a plain insert (no cascade summary —
that is DELETE-only). Still behind the dev `sqlinsert` entry word
(shared `insert` is 3j). 1493 tests green, clippy clean.
walk_repeated discarded the last matched item's trailing-optional
expectations at a clean item boundary, so a comma-separated list
offered no continuation after a complete item: `order by Name `
gave no asc/desc, `select Name ` no `as`, `create table …
Code(text) ` no not/unique/default/check. Capture the last item's
skipped set and surface it when the list ends at an item boundary
(the separator `,` itself is deliberately not surfaced).
That fix made expression-position candidate lists long, which
exposed a visibility problem: the hint panel's candidate line is
single-row and window-scrolls on overflow, centring on item 0 when
nothing is selected — so with keywords-first, schema identifiers
scrolled off behind the `>` marker. Reverse the ordering: schema
identifiers (table/column/relationship names) now sort before
keywords, since a name the user would have to look up is the
highest-value completion and must stay visible (keywords are
learned over time; the tok_identifier/tok_keyword colour split
marks the boundary). This reverses the handoff-14 keywords-first
call, now recorded in ADR-0022 Amendment 2.
Tests: walker expected-set + completion-layer regressions for the
trailing-optionals and the ordering; candidate_ordering.rs header
invariant inverted; ~20 typing-surface snapshots re-baselined; a
two-line hint box recorded as a deferred follow-up.
walk_seq's Incomplete arm unconditionally merged the accumulated
skipped-Optional expectations (pending_skipped) into the child's
expected set. When a child committed terminals before going
Incomplete (e.g. `order by` consumed, now awaiting a sort item),
this leaked ~13 clause keywords from clauses positioned *before*
the committed child — WHERE/GROUP BY/HAVING, the FROM's JOIN
options, set-ops — into the ORDER BY completion list, shoving the
actual columns off-screen.
Merge pending_skipped only when the Incomplete-producing child
consumed nothing (path length unchanged): the cursor still sits at
the optional boundary, so those optionals are genuine alternatives.
A committed child means the cursor is past them.
Tests: walker expected-set guard (+ over-correction guard) and a
full-stack completion-layer regression test.
F1: the hint panel is the completion UI, so a premature "no such table/
column" ERROR on the token the user is still typing must not shadow its
completion. ambient_hint now suppresses an under-cursor error diagnostic
when a completion exists for the (non-empty) partial it overlaps, and
falls through to the candidates. Genuinely-unknown names (no prefix match)
still show the error; WARNINGs are unaffected. Both modes.
F2: projection-before-FROM ("select <cursor> from T" after deleting *)
offered the global column list instead of T's columns, because the §10.6
look-ahead's full-input walk can't reach FROM through an empty projection.
When the look-ahead finds no scope, retry with a neutral placeholder
inserted at the cursor so the trailing FROM/CTE scope is recovered for
narrowing. Only the repaired walk's from_scope/cte_bindings are used.
Test-first: 3 F1 tests (mid-typed completes, unknown still errors, simple-
mode DSL) + 1 F2 multi-table narrowing test. 1469 baseline green.
Advanced-mode hinting + completion-preview were dead: render_hint_panel
returned None for advanced mode (stale ADR-0022 §12 gate, predating the
SQL grammar) and the hint resolver/ambient_hint never threaded Mode, so a
SQL statement was gated as "this is SQL". The unified walker (ADR-0030/
0031/0032) speaks SQL, so this lifts the gate.
- ambient_hint_in_mode + hint_resolution_at_input_in_mode +
expected_for_hint_snapshot(mode); candidate/diagnostic/parse sub-calls
run in the active mode.
- render_hint_panel calls ambient for all modes; one-shot `:` sigil
stripped (strip_one_shot_prefix) so `: sel` hints `select`.
- ADR-0022 Amendment 1 + README index.
Found by manual advanced-mode testing; Phase 2 marked SQL hint/completion
green at the engine layer but never exercised the UI. App-level render
test (advanced_mode_hint_panel_surfaces_sql_candidates) + ambient-layer
regression locks. 1466 baseline green.
SQL_INSERT_SHAPE (INTO <table> [(cols)] VALUES tuple(s)) with __rdbms_*
target rejection; Command::SqlInsert{sql,target_table}; Request::RunSqlInsert
+ do_sql_insert worker (tx-guarded: execute, then finalize_persistence for
CSV + history before commit, so failures roll back and don't re-persist).
Auto-show is best-effort via last_insert_rowid range.
Isolated behind a dev `sqlinsert` entry word (Advanced) so the SQL path is
testable without making `insert` a shared word yet (that's 3j, after 3d
auto-fill parity). Command::SqlInsert carries only sql+target_table; the
plan's listed_columns/returning land in 3d/3g where they're read.
6 grammar accept/reject tests + 8 integration tests (single/multi-row,
column-list, full-arity, history, rollback-on-failure, multi-row atomicity,
parse-path reconstruction, internal-table rejection). 1452 baseline green.
Replaces ADR-0033 §2's original Node::Guard + Choice(SQL,DSL) mechanism,
which was found during 3a to be unworkable: any guard-in-Choice approach
forces a walk_choice change (walk_choice falls through only on NoMatch, so
simple-mode valid-DSL would wrongly surface "this is SQL"), and walk_seq
treats a NoMatch past idx 0 as a hard Failed, breaking advanced-mode DSL
fall-through.
Mechanism (Amendment 1): each REGISTRY entry is tagged
CommandCategory::{Simple, Advanced}, generalising the whole-command
is_advanced_only gate. walk() becomes a thin dispatcher over decide()
(mode-aware candidate selection: simple commits the DSL node or emits the
"this is SQL" hint; advanced tries SQL first, DSL as a full-line fallback)
and an extracted walk_one_command(); speculative match-testing runs on a
scratch WalkContext so the caller's context is only touched by the
committed walk. No Node::Guard, no walk_choice/walk_seq change.
6 dispatch smoke tests on a shared-entry-word smoke registry; 1446 baseline
green; clippy clean.
Three DA critiques from the Phase-2 verification flagged real gaps;
this commit closes them.
1. Type recovery row-independence (critique #1). The all-10-types
test left col_blob NULL because the DSL Value enum has no Blob
variant. The DA flagged this as a potential row-dependence gap.
Added `database_run_select_type_recovery_works_on_empty_table`
that proves column-origin metadata works on Text AND Blob
columns with zero rows, pinning the invariant. The all-types
test now carries an explicit comment referencing it.
2. Engine.* pattern matching against real SQLite output (critique
#2). The pre-rework tests fed `translate_generic` hand-coded
strings; never verified that the pinned SQLite version actually
produces those wordings. Added three engine-routing tests in
`tests/sql_select.rs` that produce real engine errors via
`run_select` and assert catalog routing. Aggregate-in-WHERE
confirms end-to-end. GROUP-BY-required and scalar-subquery
are SQLite-permissive (no real error on the natural triggers),
so those tests verify the matcher doesn't false-positive on
benign queries + that synthetic messages route correctly.
3. Manual TUI verification (critique #3) surfaced an additional
gap: `App::input_validity_verdict()` was hard-coded silent in
Advanced mode, so SQL predicate warnings emitted but never
reached the [WRN] indicator. Wired the verdict through to the
active effective mode; updated two pre-existing tests that
pinned the now-superseded "silent in Advanced" behavior; added
one new test confirming a SQL `LIKE`-on-numeric warning fires
the indicator. Launched the TUI, typed a representative
warning-triggering SELECT, confirmed SELECT/FROM/WHERE/LIKE
highlight as keyword colour AND the [WRN] indicator appears.
Test totals: 1441 → 1446 passing (+5). Clippy clean.
Cross-cut verification matrix for ADR-0032 Phase 2 is now fully
populated with concrete test references — every row green. Filling
the matrix surfaced three real gaps that this commit closes.
1. Advanced-mode syntax highlighting (ADR-0030 §8 matrix row).
The `ui.rs` Advanced branch routed through `plain_input_spans`,
bypassing the highlight walker entirely. In production SQL
keywords past the entry word rendered as plain identifiers.
Fix: mode-aware variants of `highlight_runs`,
`render_input_runs`, `lex_to_runs`, and `input_diagnostics`;
the Advanced render path now uses the highlighted form with
`Mode::Advanced`. `plain_input_spans` removed (unused).
2. Engine.* key wiring (ADR-0032 §11.4 / §13 matrix rows + handoff
§3.3 follow-up). The four Phase-2 engine.* catalog entries
were authored in 2d but never reached: `translate_generic`
discarded the engine message and returned a vague catalog
entry. Fix: pattern-match the engine message text for the four
Phase-2 categories (aggregate misuse, group-by required,
compound arity mismatch fallback, scalar-subquery cardinality)
inside `translate_generic`, routing each to its engine-neutral
catalog entry.
3. Matrix-coverage tests. Thirteen new tests covering the rows
that had no explicit coverage:
- 3 SQL keyword/operator/CASE highlight tests
- 4 engine.* engine-message tests
- 3 sql_expr column-completion tests (WHERE, HAVING)
- 3 predicate-warning slot tests (CASE, ORDER BY, projection)
- 1 all-10-playground-types recovery test (tests/sql_select.rs)
Plan document (docs/plans/20260520-adr-0032-phase-2.md) updated:
every (TBD) row in the cross-cut matrix replaced with a concrete
test file::function reference and a green status marker.
Test totals: 1428 → 1441 passing (+13 new). Clippy clean.
Amendment 2 records the §10.6 fixup-pass mechanism choice. §10.6
prescribes "rewriting the highlight class" on projection-list
idents at end-of-walk; the actual implementation uses a different
mechanism that achieves the identical user-visible behavior:
1. 2d's two-pass schema-existence diagnostic collects every FROM
binding from the matched path first, then resolves projection
idents against the complete scope. The post-walk re-resolve
§10.6 calls for, just embedded in the diagnostic emitter.
2. input_render.rs's diagnostic-overlay path colors each
diagnostic span Error/Warning, achieving the visual change
§10.6 describes without needing a new HighlightClass variant.
The completion-mid-typing piece is improved by the §10.5
look-ahead probe (sub-phase 2e earlier).
Four new regression tests in `projection_before_from_tests` pin
the behavior so a future refactor can't silently regress it:
correct ident resolves silently, unknown ident flags via
diagnostic on its span, multi-projection only flags unknowns,
projection-without-FROM is silent.
ADR index entry updated to reference Amendment 2.
Test totals: 1424 → 1428 passing (+4). Clippy clean.
ADR-0032 §10.5 — at the cursor, an `<ident>.` prefix narrows
column candidates to that qualifier's binding columns. Resolves
through from_scope aliases first, then table names, then
cte_bindings (for `cte_alias.|`). Falls back to the schema cache
for DSL paths (`from <Table>.<col>`). Unresolved qualifier →
empty column list; the structural error path surfaces the
unresolved-prefix message.
Look-ahead probe — the "edit an existing query" workflow. When
the cursor is mid-projection but FROM exists after the cursor, a
second walk on the full input populates from_scope and the
column candidates narrow accordingly. Gated on the leading walk
producing no scope so cursor-past-FROM positions pay no cost.
The full input must parse for this to work; an unparseable
mid-edit state falls back to the §10.6 global posture.
CompletionProbe now exposes `from_scope` (top-frame table
bindings) and `cte_bindings` (union of in-scope CTE bindings,
innermost-first dedupe). The walker drains these at the cursor
position; the completion engine reads them for qualifier
resolution and unqualified narrowing.
Test totals: 1415 → 1424 passing (+9: 5 qualified-prefix +
4 look-ahead). Clippy clean.
ADR-0032 §10.3 says cte_bindings lives on the scope frame, with
inner subqueries free to declare their own CTEs that shadow outer
ones. The grammar didn't actually admit nested WITH inside
SQL_SELECT_COMPOUND — a real ADR-vs-implementation gap.
Closes the gap by making SQL_SELECT_COMPOUND a Choice between a
WITH-prefixed form and a plain form. The naive Optional-prefix
approach silently broke the paren-vs-subquery dispatch in
sql_expr.rs's PAREN_GROUP: Optional matches 0 bytes, committing
the Seq, so SELECT_CORE's NoMatch on `(a + b)` became Failed and
the Choice couldn't fall through to or_expr. The Choice-fronted
form keeps the fast NoMatch on non-WITH non-SELECT first tokens.
Side effect: scalar subquery / IN / EXISTS / derived-table
bodies now admit a leading WITH too, which matches standard SQL.
Updated two tests that were guarding the old `(WITH …)` rejection
behavior. Added one new harvest test exercising nested-WITH inside
a CTE body — the harvest's `expand_binding` mechanism already
handled the data correctly; the grammar gap was the sole blocker.
Test totals: 1414 → 1415 passing (+1 nested-with-in-cte test).
Clippy clean.
Implements the six ADR-0032 §10.3 output-column derivation rules
at CTE body-frame exit, populating the placeholder CteBinding's
columns. Unblocks `diagnostic.cte_arity_mismatch` (which compares
declared col-list arity vs derived projection arity) and the
upcoming qualified-prefix completion in 2e proper.
- `WalkContext::pending_cte_harvest`: bookkeeping for an in-progress
CTE harvest, armed by writes_cte_name + extended by cte_column
idents, consumed by the next walk_scoped_subgrammar invocation
(CTE syntax has no intervening ScopedSubgrammar, so timing is
deterministic). Cleared on every walk_scoped_subgrammar entry
to prevent stale state surviving a speculative walk rollback.
- `run_cte_harvest`: post-walk path-scan classifier that
reconstructs the body's first leg's projection-list and applies
the six derivation rules. Compound bodies take columns from the
first leg per spec; recursive CTE bodies take the non-recursive
(first) leg. Optional (col-list) renames positionally with
preserved types.
- `expand_binding`: bridges a TableBinding to a CteColumn list,
resolving CTE-source bindings (empty columns + table-name
matches an in-scope CteBinding) through to the CTE's harvested
columns. Enables sibling CTEs to project correctly: in
\`WITH a AS (...), b AS (SELECT * FROM a) ...\`, b's harvest sees
a's derived columns through the body's from_scope binding.
- `WalkContext::pending_diagnostics`: accumulator for diagnostics
emitted DURING the walk by node handlers with context the
post-walk passes can't reconstruct. Drained by the top-level
walk function on both match and non-match paths so a re-used
context can't leak entries between walks.
Test totals: 1399 → 1414 passing (+15: 10 derivation rules + 1
sibling CTE + 4 arity match/mismatch tests). Clippy clean.
Closes the two diagnostics deferred by sub-phase 2d that were not
attached to a user-approved deferral. `cte_arity_mismatch` stays
deferred — it depends on the §10.3 stage-2 CTE harvest, which IS a
user-approved deferral.
- `diagnostic.projection_alias_misplaced` (ADR-0032 §11.2): emitted
when a projection alias is referenced from `WHERE` / `HAVING` /
`GROUP BY`. `ORDER BY` references are allowed and silent. The
pass is integrated into `schema_existence_diagnostics`: when a
bare-column ref doesn't resolve to any binding's column but DOES
match a projection alias in the current SELECT leg, the new
diagnostic pre-empts the misleading `unknown_column` that would
otherwise fire on the same span. Real-column-shadowed-by-alias
cases (engine resolves to the table column) stay silent. Subquery
scopes (paren depth > 0) keep their own implicit alias bag —
outer aliases don't leak into inner WHERE.
- `diagnostic.compound_arity_mismatch` (ADR-0032 §11.2 / §11.7): a
new MatchedPath-walking pass that counts projection items per
SELECT leg by tallying top-level commas at the leg's own paren-
depth, then compares adjacent legs across `UNION` / `UNION ALL` /
`INTERSECT` / `EXCEPT` operators. The diagnostic anchors on the
operator span. Per-depth book-keeping lets chained compound
queries inside CTE bodies / subqueries report independently.
Function-call argument commas (deeper depth) are correctly
ignored.
Test totals: 1385 → 1399 passing (+14), 0 failed, 1 ignored.
Clippy clean.
`Cargo.toml`: add `column_metadata` to rusqlite's feature list.
This pulls in the SQLite `SQLITE_ENABLE_COLUMN_METADATA`
compile flag and surfaces `sqlite3_column_table_name` /
`sqlite3_column_origin_name` on prepared statements via
rusqlite's `Statement::columns_with_metadata()`.
`do_run_select` in db.rs now calls a new
`resolve_select_column_types(conn, stmt)` helper after
`prepare`. The helper walks each result-column's origin
metadata; when both `table_name` and `origin_name` come back
populated (the result column traces back to a base-table
column), it looks up the playground type in
`__rdbms_playground_columns`. The per-column types thread
through to `format_cell(value, ty)` so the data-table
renderer (ADR-0016) gets the same per-type rendering it
applies to `show data` results.
Effect: ADR-0030 Phase-1 §4.5 (bool SELECT results render as
`0` / `1`) is lifted for any bare-column reference whose
origin the engine carries through — per ADR-0032 Amendment 1
(2026-05-20 empirical probe), that means all non-recursive
CTE bodies, scalar subqueries (aliased or not), derived
tables, set ops, and JOINs. Computed projections and
recursive-CTE result columns remain typeless (the engine
populates no origin), which the renderer handles via neutral
alignment.
The lookup is engine-driven verbatim — no grammar-side
structural classification (ADR-0032 Amendment 1 replaces
§12's original "structurally a single column reference" rule
with "trust column_table_name / column_origin_name").
Tests (3 new in `tests/sql_select.rs`, all green):
- `database_run_select_recovers_bool_column_type` — the
Phase-1 §4.5 case: `SELECT Active FROM Products` returns
`column_types = [Some(Bool)]` and rows render as `true` /
`false`.
- `database_run_select_recovers_text_type_through_alias` —
`SELECT Name AS n FROM Users` remaps the result column
name to `n` but the origin metadata still resolves the
playground type to `Some(Text)`.
- `database_run_select_computed_expression_stays_typeless`
— `SELECT Score + 1 FROM T` keeps `column_types[0] =
None`, the documented Amendment-1 exception.
The CTE pass-through, scalar subquery, set-op, and JOIN
cases all work for free given the empirical findings;
their behaviour is asserted by the Amendment-1 probe
results recorded in the ADR, so no per-case integration
tests are duplicated here.
Test totals: 1382 → 1385 passing (+3), 0 failed, 1 ignored.
Clippy clean.
Implements the bulk of ADR-0032 §11 diagnostics. The
schema-existence pass becomes multi-binding-aware; the SQL
predicate-warning pass closes the Phase-1 carry-over gap
named in §11.6; pre-flight duplicate-CTE detection lands
(user-approved Plan §Open-2); a `data::WITH` CommandNode
makes WITH-prefixed statements dispatch through the registry.
Catalog (`src/friendly/strings/en-US.yaml`, `src/friendly/keys.rs`):
- Six new `diagnostic.*` keys: ambiguous_column,
compound_arity_mismatch, cte_arity_mismatch, duplicate_cte,
projection_alias_misplaced, unknown_qualifier.
- Eight new `engine.*` translation keys (ADR-0032 §11.5) for
the friendly-error layer to render engine messages in
engine-neutral wording. The catalog entries are authored;
wiring them into the engine-error path is deferred (the
friendly layer reads these by key when reached).
Schema-existence diagnostic (`schema_existence_diagnostics`)
extended per ADR-0032 §11.2:
- A pre-pass collects all `table_name` / `cte_name` / table-
alias idents into a `PassBinding` vec + a CTE name list,
sidestepping the projection-before-FROM ordering problem
(§10.6). The main pass then resolves identifiers against the
complete scope.
- Bare column references resolve against any binding's
columns. Zero matches → `diagnostic.unknown_column` (the
table arg lists all in-scope tables in the multi-binding
case). Two-or-more matches → `diagnostic.ambiguous_column`.
- Qualified `t.c` refs detect their qualifier via a look-ahead
on the matched path (Punct '.' + Ident{role:
sql_expr_qualified_ref} after the leading Ident). Unknown
qualifier → `diagnostic.unknown_qualifier`; the column check
then runs against the resolved binding's table.
- The `t.*` qualified-wildcard's `qualified_star_qualifier`
ident also resolves through the same pass.
- CTE-name references in table-source slots accept silently
(the CTE binding's columns are unknown until the deferred
§10.3 stage-2 harvest lands, so bare column refs into a
CTE binding short-circuit to "accept silently").
- Duplicate CTE names in the same `WITH` block emit
`diagnostic.duplicate_cte` on the second occurrence
(Plan §Open-2).
Phase-1 gap closure (`sql_predicate_warnings`, ADR-0032 §11.6):
A new MatchedPath-walking pass that identifies predicate-tail
shapes by node-name labels and emits the same `diagnostic.*`
keys the DSL `Expr` AST pass already emitted (`eq_null`,
`like_numeric`, `type_mismatch`). Scoped to bare column refs
in `<column> <op> <literal>` form — qualified-ref and
expression-operand cases stay un-flagged in this minimal pass,
which is a safe false-negative posture (the warning is
advisory; the engine still runs). Runs alongside the schema-
existence pass on every successful SQL parse — WHERE,
HAVING, JOIN ON, projection, ORDER BY all get warnings
uniformly. Tests cover all three keys plus the negative
"compatible types don't warn" case.
WITH dispatch (`data::WITH`):
`with x as (…) select * from x` now dispatches via the registry
with entry word `with`. Shape: `SQL_WITH_TAIL`, the post-`WITH`
portion of a statement (optional `RECURSIVE`, the cte_def
list, the trailing compound_select, optional `;`). Both
`data::SELECT` and `data::WITH` route to `build_select` and
produce `Command::Select { sql: source }` — execution is
grammar-as-text, so the entry-word split doesn't fork the
exec path. `is_advanced_only` extended to include `with`.
Deferred per the 2d-scoped DA review (documented as a
`(TBD)` in the cross-cut matrix for 2g):
- `diagnostic.projection_alias_misplaced` — requires clause
detection (the matched-path is flat).
- `diagnostic.compound_arity_mismatch` — needs per-leg
projection counting.
- `diagnostic.cte_arity_mismatch` — depends on §10.3 stage-2
harvest, which 2b deferred.
- `engine.*` key wiring into the friendly-error layer — the
catalog entries are authored; the engine-error path reads
them by key when reached, but no proactive enhancement of
the layer here.
Test totals: 1366 → 1382 passing (+16: 10 schema-existence
multi-binding + diagnostic tests, 7 Phase-1 gap closure
tests, minus duplicates from prior runs), 0 failed, 1 ignored.
Clippy clean.
The Phase-1 SQL `SELECT` grammar nodes that used to live in
`src/dsl/grammar/data.rs` retire — 22 statics / consts and the
`reject_internal_table` validator copy are removed, ~150 lines
of grammar machinery gone. `data::SELECT.shape` now references
the post-`SELECT` portion of the ADR-0032 fragment via a thin
`Node::Subgrammar(&sql_select::SQL_SELECT_TAIL)`.
`SQL_SELECT_TAIL` is a new export from `sql_select.rs`,
parallel to `SQL_SELECT_STATEMENT`. It represents what a
top-level `SELECT` statement looks like AFTER the registry's
entry-word dispatch has already consumed the leading `SELECT`
keyword: the DISTINCT/ALL prefix, projection list, optional
FROM / WHERE / GROUP BY / HAVING, the compound set-op chain
(each subsequent leg's `SELECT` is part of `SET_OP_TAIL`),
outer ORDER BY / LIMIT, and a tolerated trailing `;`.
WITH-prefixed statements (`WITH x AS (…) SELECT * FROM x`)
are NOT in 2c's scope — they need a separate `data::WITH`
`CommandNode` so the entry-word dispatch routes correctly.
For now, top-level WITH continues to fall through to the
chumsky parser route (the same as in Phase 1). The
`SQL_SELECT_STATEMENT` static (which includes the optional
WITH prefix) stays available for use by that future
CommandNode or by any other consumer that needs the full
statement shape.
All seven Phase-1 SQL `SELECT` integration tests
(`tests/sql_select.rs`) pass without modification, satisfying
the 2c exit gate's "behaviour preserved" requirement. The
70 fragment unit tests and the 26 driver-level scope tests
also pass — the migration is a refactor, no new tests
required.
Behaviour change explicitly sanctioned by ADR-0032 §8:
Phase-1's `LIMIT_VALIDATOR` (positive-int-only, parse-time)
is superseded by the full `sql_expr` admission. `LIMIT max(10,
x)` and similar now parse; the engine constrains the value at
execution time per the ADR's "grammar admits, engine
rejects" posture.
Plan §2b status note: the 2026-05-20 deferral of §10.3 stage 2
(CTE output-column harvest derivation) is recorded in
`docs/plans/20260520-adr-0032-phase-2.md` per the
user-approved deferral.
Test totals: 1366 passing (unchanged), 0 failed, 1 ignored.
Clippy clean. data.rs loses ~150 lines of dead grammar; the
single source of truth for the SQL `SELECT` shape is now
`sql_select.rs`.
claude@clouddev1