chore: handoff 17 — ADR-0026 complex WHERE expressions implemented
This commit is contained in:
claude@clouddev1
@@ -0,0 +1,245 @@
|
|||||||
|
# Session handoff — 2026-05-18 (17)
|
||||||
|
|
||||||
|
Seventeenth handover. This session was an **implementation
|
||||||
|
run**: it built **ADR-0026 (complex WHERE expressions)** — the
|
||||||
|
keystone the handoff-16 design run set up. Steps 1–4 and 6 of
|
||||||
|
the ADR's build order are done, committed, tested, and clippy-
|
||||||
|
clean. **Step 5 (the §7 diagnostic *flagging*) is deferred to
|
||||||
|
ADR-0027** — a deliberate sequencing call; see §3, which is
|
||||||
|
the one thing in this handoff that may want your input.
|
||||||
|
|
||||||
|
**Headline: complex WHERE expressions work end to end.**
|
||||||
|
`update` / `delete` / `show data` take a full boolean
|
||||||
|
expression — `AND` / `OR` / `NOT`, the six comparisons,
|
||||||
|
`LIKE` / `IN` / `BETWEEN` / `IS [NOT] NULL`, parentheses; it
|
||||||
|
compiles to parameterised SQL; `show data` gained `where` and
|
||||||
|
`limit`. The functional `C5a` requirement is satisfied.
|
||||||
|
|
||||||
|
## State at handoff
|
||||||
|
|
||||||
|
**Branch:** `main`. Working tree clean. Four commits since
|
||||||
|
handoff-16 (`ac41938`), all local — push asynchronously, do
|
||||||
|
not treat as blocking:
|
||||||
|
|
||||||
|
```
|
||||||
|
a50c6cd WHERE expressions: matrix cells + predicate_tail grammar fix (ADR-0026 step 6)
|
||||||
|
f75f71b WHERE expressions: wire into update/delete/show data + SQL gen (ADR-0026 steps 3-4)
|
||||||
|
59e6a54 grammar: WHERE-expression fragment + Expr AST + build_expr (ADR-0026 step 2)
|
||||||
|
f0b2043 walker: add Subgrammar node + recursion-depth cap (ADR-0026 step 1)
|
||||||
|
```
|
||||||
|
|
||||||
|
**Tests:** **1079 passing, 0 failing, 1 ignored** (`cargo
|
||||||
|
test` — up from 1039). The ignored test is the long-standing
|
||||||
|
`` ```ignore `` doc-test in `src/friendly/mod.rs`. The
|
||||||
|
typing-surface matrix is **161 cells** (was 152 — `+9` in the
|
||||||
|
new `tests/typing_surface/where_expression.rs`).
|
||||||
|
|
||||||
|
**Clippy:** clean (`cargo clippy --all-targets -- -D
|
||||||
|
warnings`, nursery group).
|
||||||
|
|
||||||
|
## §1. What shipped — ADR-0026 steps 1–4 + 6
|
||||||
|
|
||||||
|
### Step 1 — `Subgrammar` node + depth cap (`f0b2043`)
|
||||||
|
|
||||||
|
New `Node::Subgrammar(&'static Node)` variant: a named
|
||||||
|
`static` grammar fragment recurses through a reference (a
|
||||||
|
`Seq` / `Choice` embeds children by value and cannot close a
|
||||||
|
cycle). `WalkContext::subgrammar_depth` counts active frames;
|
||||||
|
past `MAX_SUBGRAMMAR_DEPTH` (64) the walk fails friendly
|
||||||
|
(`parse.custom.expression_too_deep`) instead of overflowing
|
||||||
|
the stack. Depth is saved/restored per frame so a
|
||||||
|
rolled-back `Choice` branch leaves no residue.
|
||||||
|
|
||||||
|
### Step 2 — expression grammar + `Expr` AST + `build_expr` (`59e6a54`)
|
||||||
|
|
||||||
|
`src/dsl/grammar/expr.rs` (new): the **stratified**
|
||||||
|
WHERE-expression grammar — `or` / `and` / `not` /
|
||||||
|
`bool_primary` / `predicate` tiers as named `static` `Node`
|
||||||
|
fragments, recursing through `Subgrammar`. New recursive
|
||||||
|
`Expr` / `Predicate` / `Operand` / `CompareOp` AST in
|
||||||
|
`dsl::command`. `build_expr` folds the flat matched-terminal
|
||||||
|
slice into an `Expr`.
|
||||||
|
|
||||||
|
### Steps 3–4 — wiring + SQL generation (`f75f71b`)
|
||||||
|
|
||||||
|
- **Grammar** (`grammar/data.rs`): `update` / `delete`
|
||||||
|
`where` is now `Subgrammar(&expr::OR_EXPR)`; `show data`
|
||||||
|
gained optional `where <expr>` and `limit <n>`.
|
||||||
|
- **AST**: `RowFilter::Where{column,value}` →
|
||||||
|
`RowFilter::Where(Expr)`; `ShowData` gained
|
||||||
|
`filter: Option<Expr>` + `limit: Option<u64>`. A
|
||||||
|
`RowFilter::eq` convenience constructor keeps
|
||||||
|
simple-equality call sites readable.
|
||||||
|
- **SQL** (`db.rs`): `compile_expr` lowers `Expr` to a
|
||||||
|
parameterised WHERE (every literal a `?` placeholder, `<>`
|
||||||
|
for inequality). `show data … limit n` emits `LIMIT ?`
|
||||||
|
with an implicit primary-key `ORDER BY`.
|
||||||
|
- **§8 hints**: the expression's right-hand operands resolve
|
||||||
|
through a schema-aware `DynamicSubgrammar`
|
||||||
|
(`where_rhs_operand`) so the hint panel narrows to the
|
||||||
|
compared column's type — but the operand grammar carries
|
||||||
|
**no validators**, so a type-mismatched literal parses
|
||||||
|
(§7 permissive).
|
||||||
|
- 11 db integration tests cover AND/OR/NOT, BETWEEN, IN,
|
||||||
|
LIKE, filtered `show data`, limit ordering.
|
||||||
|
|
||||||
|
### Step 6 — matrix cells + a grammar fix (`a50c6cd`)
|
||||||
|
|
||||||
|
`tests/typing_surface/where_expression.rs` (9 cells). Writing
|
||||||
|
them caught a real bug: `predicate_tail`'s `[NOT] negatable`
|
||||||
|
branch started with `Optional(not)`, and an `Optional`-first
|
||||||
|
`Seq` always "commits" — so the walker's `Choice` returned
|
||||||
|
that branch's `Incomplete` early and **discarded the sibling
|
||||||
|
branches' expected sets**, dropping `is` and the comparison
|
||||||
|
operators from completion after a column. Fixed by splitting
|
||||||
|
into explicit `NOT negatable` + bare `negatable` branches;
|
||||||
|
the matched terminal sequence is unchanged so `build_expr`
|
||||||
|
was untouched.
|
||||||
|
|
||||||
|
## §2. The §3 builder realization (settled with the user)
|
||||||
|
|
||||||
|
ADR-0026 §3 admitted two realizations of "the fragment-builder
|
||||||
|
the walker invokes". The project owner chose **option 1
|
||||||
|
("reconstruct in builder")** before implementation:
|
||||||
|
|
||||||
|
- The stratified grammar is walked normally; terminals flow
|
||||||
|
into the flat `MatchedPath` unchanged (driving highlight /
|
||||||
|
completion / the expected-set). `build_expr` then folds
|
||||||
|
that flat slice into the `Expr` — a deterministic recursive
|
||||||
|
descent, run only at submit-time dispatch.
|
||||||
|
- Two honest deviations from §3's wording, recorded in
|
||||||
|
ADR-0026's new **"As-built notes"** section: there is **no
|
||||||
|
`MatchedKind::Expr` variant** (`MatchedPath` stays purely
|
||||||
|
terminals; the `Expr` is built in the command
|
||||||
|
`ast_builder`s), and there **is** a second structural pass
|
||||||
|
(scoped to dispatch — "no second parse" read as "no
|
||||||
|
separate parser framework").
|
||||||
|
|
||||||
|
Do not re-litigate; build to the as-built notes.
|
||||||
|
|
||||||
|
## §3. ⚠️ Step 5 is deferred — this is the one open call
|
||||||
|
|
||||||
|
ADR-0026 step 5 is "schema-aware type-mismatch and `= NULL`
|
||||||
|
flagging in the walker". It splits in two:
|
||||||
|
|
||||||
|
- **The §7 *behaviour* relaxation — DONE.** A type-mismatched
|
||||||
|
WHERE literal (`where Age = 3.14`, `Age` an int) used to be
|
||||||
|
*rejected* at bind time. It no longer is: `bind_where_literal`
|
||||||
|
binds it by its syntactic shape and the command runs. Test:
|
||||||
|
`walker::tests::phase_d_delete_where_permits_decimal_at_int_column`.
|
||||||
|
- **The §7 *diagnostic flagging* — DEFERRED to ADR-0027.**
|
||||||
|
Surfacing a type-mismatched comparison or `= NULL` as an
|
||||||
|
*error-class highlight + hint* is the seam with ADR-0027,
|
||||||
|
which designs the **walker diagnostics-severity model** the
|
||||||
|
flagging belongs in. ADR-0027 itself says its WARNING
|
||||||
|
severity "has **no triggers** until ADR-0026 is
|
||||||
|
implemented" — i.e. ADR-0026's type-mismatch / `= NULL`
|
||||||
|
conditions *are* those triggers.
|
||||||
|
|
||||||
|
**Why deferred, not done badly now:** building the flagging
|
||||||
|
as a standalone ad-hoc mechanism (a bespoke `HighlightClass::
|
||||||
|
Error` pass + a bespoke hint) before ADR-0027's severity
|
||||||
|
model exists would almost certainly be reworked when that
|
||||||
|
model lands. The clean sequencing is: implement ADR-0027's
|
||||||
|
diagnostics-severity model, and make ADR-0026 §7's
|
||||||
|
type-mismatch + `= NULL` detection the first triggers feeding
|
||||||
|
it.
|
||||||
|
|
||||||
|
**This is a sequencing judgement made while you were away.**
|
||||||
|
If you'd rather have a quick standalone §7 flag now, say so;
|
||||||
|
otherwise the recommendation is to fold it into ADR-0027.
|
||||||
|
It's recorded in ADR-0026's "As-built notes", `requirements.md`
|
||||||
|
`C5a`, and `CLAUDE.md`'s deferred list.
|
||||||
|
|
||||||
|
## §4. What's next
|
||||||
|
|
||||||
|
1. **ADR-0027 — input-field validity indicator.** Now
|
||||||
|
genuinely unblocked: ADR-0026 supplies the WARNING-severity
|
||||||
|
triggers (type mismatch, `= NULL`). Implementing ADR-0027
|
||||||
|
should also land ADR-0026 step 5's flagging as its first
|
||||||
|
triggers (see §3).
|
||||||
|
2. **ADR-0028 — query plans (`explain`).** Also unblocked —
|
||||||
|
`show data … where` (now real) is the filtered query whose
|
||||||
|
plan flips between a full scan and an index search.
|
||||||
|
3. Either order after ADR-0027, per handoff-16.
|
||||||
|
|
||||||
|
Other open clusters are unchanged from handoff-16 §3
|
||||||
|
(snapshot/undo `U1`/`U2`; constraints `C3` — `CHECK` can now
|
||||||
|
reuse ADR-0026's expression grammar; `C4` m:n; `C3a`; `C1`
|
||||||
|
table rename; `H1`; `SD1`; `TT5` CI; `V4`; `I1`; `TU1`).
|
||||||
|
Prioritisation is a user product decision — ask.
|
||||||
|
|
||||||
|
## §5. Architectural delta (vs. handoff-16)
|
||||||
|
|
||||||
|
### Grammar / walker
|
||||||
|
|
||||||
|
- `Node::Subgrammar(&'static Node)` — new variant; static
|
||||||
|
counterpart of `DynamicSubgrammar`.
|
||||||
|
- `WalkContext::subgrammar_depth: usize`;
|
||||||
|
`walker::driver::MAX_SUBGRAMMAR_DEPTH = 64`.
|
||||||
|
- `CompletionProbe` gained `pending_hint_mode` — completion
|
||||||
|
now suppresses keyword candidates at a grammar-declared
|
||||||
|
`ProseOnly` slot (the WHERE operand, which also accepts a
|
||||||
|
column ident, would otherwise surface the misleading
|
||||||
|
`null`/`true`/`false` trio).
|
||||||
|
- `src/dsl/grammar/expr.rs` — the whole expression fragment +
|
||||||
|
`build_expr`. Entry point: `pub static expr::OR_EXPR`.
|
||||||
|
|
||||||
|
### Command AST
|
||||||
|
|
||||||
|
- `Expr` / `Predicate` / `Operand` / `CompareOp` — new,
|
||||||
|
recursive; in `dsl::command`, re-exported from `dsl`.
|
||||||
|
- `RowFilter::Where(Expr)` (was `Where{column,value}`);
|
||||||
|
`RowFilter::eq(col, val)` convenience constructor.
|
||||||
|
- `Command::ShowData` gained `filter: Option<Expr>`,
|
||||||
|
`limit: Option<u64>`.
|
||||||
|
|
||||||
|
### db
|
||||||
|
|
||||||
|
- `Request::QueryData` / `Database::query_data` gained
|
||||||
|
`filter` + `limit`; signature is now
|
||||||
|
`query_data(table, filter, limit, source)`.
|
||||||
|
- `compile_expr` / `compile_predicate` / `compile_operand` /
|
||||||
|
`bind_where_literal` / `syntactic_bound` — `Expr` → SQL.
|
||||||
|
- `do_query_data` builds `WHERE` / `ORDER BY` / `LIMIT`.
|
||||||
|
|
||||||
|
### Catalog
|
||||||
|
|
||||||
|
- New key `parse.custom.expression_too_deep` (depth cap).
|
||||||
|
|
||||||
|
### A latent bug fixed in passing
|
||||||
|
|
||||||
|
`completion::invalid_ident_at_cursor` treated a digit-led
|
||||||
|
token (`1`) as an identifier — now that the WHERE operand
|
||||||
|
slot accepts a column ref *and* a literal, it mis-flagged
|
||||||
|
`where id=1`'s `1` as an unknown column. It now skips
|
||||||
|
digit-led tokens. Regression test:
|
||||||
|
`completion::tests::numeric_literal_in_where_is_not_flagged_as_invalid_column`.
|
||||||
|
|
||||||
|
## §6. How to take over
|
||||||
|
|
||||||
|
1. **Read this file, then handoff-16** for the design-run
|
||||||
|
context that set ADR-0026 up.
|
||||||
|
2. **Read `CLAUDE.md`** — working-style rules.
|
||||||
|
3. **Read `docs/adr/0026-complex-where-expressions.md`** —
|
||||||
|
especially the new **"As-built notes"** section (§3
|
||||||
|
builder realization, the deviations, the step-5 deferral).
|
||||||
|
4. **Read `docs/adr/0027-input-validity-indicator.md`** — the
|
||||||
|
recommended next ADR; §3 of this handoff explains how
|
||||||
|
ADR-0026 step 5 folds into it.
|
||||||
|
5. **Run `cargo test`** — 1079 passing, 0 failing, 1 ignored.
|
||||||
|
6. **Run `cargo clippy --all-targets -- -D warnings`** — clean.
|
||||||
|
7. **Decide the §3 question** (standalone §7 flag now vs.
|
||||||
|
fold into ADR-0027), then start ADR-0027.
|
||||||
|
|
||||||
|
### Note on the typing-surface matrix
|
||||||
|
|
||||||
|
`tests/typing_surface/` is now **161 cells**. After any
|
||||||
|
grammar/walker change: a failing cell with *correct* new
|
||||||
|
behaviour → update its snapshot
|
||||||
|
(`INSTA_UPDATE=always cargo test --test typing_surface_matrix
|
||||||
|
<family>`); a failing cell with *wrong* behaviour → the cell
|
||||||
|
earned its keep. This session: 5 `delete_with_where` /
|
||||||
|
`update_with_where` snapshots were correctly updated (the
|
||||||
|
WHERE operand surface widened), and the new
|
||||||
|
`where_expression` family added 9 cells.
|
||||||
Reference in New Issue
Block a user