Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 20 additions & 3 deletions docs/ENGINE_REFACTORING.md
Original file line number Diff line number Diff line change
Expand Up @@ -472,7 +472,7 @@ feature work**, and so we can tell the difference between "this is awkward" and

### R13 — Two expression evaluators, every boolean operator implemented twice
- **Status:** 🟡 IN PROGRESS — filed 2026-09-13 out of [P46](SQL_PARITY.md#p46);
**slice 1 done 2026-09-13**, slice 2 next.
**slices 1 and 2 done 2026-09-13**, slice 3 next.
**The active workstream:** parity fixes that touch expression evaluation land
as slices of this entry, not as patches.
- **Where:** `src/data/arithmetic_evaluator.rs` (`ArithmeticEvaluator`, value →
Expand Down Expand Up @@ -549,6 +549,22 @@ feature work**, and so we can tell the difference between "this is awkward" and
[P52](SQL_PARITY.md#p52) NULL join keys on both join paths.
**Slice 2's acceptance test is now concrete:** every `KNOWN` entry for the
value evaluator disappears, and the corpus moves by exactly the P48/P50 cases.
- **Slice 2 result (2026-09-13).** Every boolean-producing arm of
`ArithmeticEvaluator` now answers in three-valued logic, NULL for UNKNOWN:
a predicate-layer `compare_trilean` for comparisons (the shared comparator keeps
`NULL = NULL` equal for ORDER BY), `Trilean`'s tables for `AND`/`OR`/`NOT`
(`to_bool` deleted), OR's table for `IN`/`NOT IN`, AND's for `BETWEEN`, NULL
from `LIKE`, and simple `CASE x WHEN NULL` no longer matching (found and pinned
at the start of the slice). `Trilean::from_value`/`to_value` became the one
crossing between `DataValue` and the truth tables. **The acceptance slice 1
set held exactly:** all 32 value-evaluator `KNOWN` entries went FIXED in one
change with the WHERE column unmoved, and parity moved by exactly the six
P48/P50 cases (168 → 174 AGREE over 204). No FORMAL example churn. Scoping
first paid off here too: the only external consumers of predicate output were
the WHERE evaluator (already NULL → UNKNOWN), HAVING's `is_truthy` and `IIF`
(both NULL → not-true, already correct), so nothing downstream had to change.
**Two evaluators now agree on all 36 predicates, but there are still two** —
slices 3–6 are what make that agreement structural rather than coincidental.
- **Slices, in the R10 pattern — no-op slices kept apart from the one that
changes answers:**
1. ✅ **Pin the divergences, no engine change.** *(Done 2026-09-13.)* A per-operator matrix run through
Expand All @@ -557,7 +573,7 @@ feature work**, and so we can tell the difference between "this is awkward" and
agree, with today's disagreements marked. Plus corpus cases for the
clause-level shapes above (HAVING, SELECT, JOIN ON), filed as P-findings.
Acceptance: parity buckets move only by the new cases.
2. **Make `ArithmeticEvaluator` three-valued.** The one semantic slice:
2. ✅ **Make `ArithmeticEvaluator` three-valued.** *(Done 2026-09-13 — see result below.)* The one semantic slice:
comparisons, `IN`/`NOT IN`, `BETWEEN`, `LIKE` yield NULL for a NULL operand;
`AND`/`OR`/`NOT` follow the truth tables; `to_bool` stops mapping NULL to
false for predicates. Closes [P48](SQL_PARITY.md#p48) and the HAVING / SELECT
Expand Down Expand Up @@ -614,7 +630,7 @@ R8 legacy WHERE ──── independent; stage 2 is self-contained, do it in a
R10 Trilean ──────── DONE; closed P18/P19 (parity 125 → 129)
R11 ORDER BY resolver ─ independent; small, but a behaviour change — wants its own parity run
R12 aggregate registries ─ independent; step 1 is a provable no-op, do it before the next aggregate fix
R13 one evaluator ─── ACTIVE from 2026-09-13; slice 1 (pin) → 2 (3VL, = P48) → 3 (construction) → 4–6 (retire WHERE arms)
R13 one evaluator ─── ACTIVE from 2026-09-13; slices 1 (pin) and 2 (3VL) DONE → 3 (construction) → 4–6 (retire WHERE arms)
```

**A note on ordering, from the P18/P19 work being next.** The WHERE evaluator
Expand Down Expand Up @@ -653,3 +669,4 @@ AGREE count — which makes it safe to land well before the semantics change.
| 2026-09-13 | P46 fixed: all WHERE operands resolve through one path, one `ArithmeticEvaluator` per evaluation. Parity 157 → **168 AGREE**; P49 filed | #80 |
| 2026-09-13 | R13 filed and made the active workstream: two evaluators with divergent NULL semantics, three truth collapses. Probing found four live clause-dependent divergences (HAVING, SELECT `IN`, JOIN NULL keys, P48) | — |
| 2026-09-13 | R13 slice 1: evaluator matrix (36 predicates × both evaluators × 5 NULL-bearing rows, DuckDB-derived expectations). WHERE 36/36, value evaluator 4/36. P48 widened; P50–P52 filed; six corpus cases, AGREE unchanged | — |
| 2026-09-13 | R13 slice 2: value evaluator three-valued; `Trilean::from_value`/`to_value`; `to_bool` removed. Matrix value column 4/36 → 36/36 in one change. Closes P48, P50 — 168 → **174 AGREE** | — |
38 changes: 28 additions & 10 deletions docs/SQL_PARITY.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,9 @@ session apiece to fix properly, so **discovery is paused and the effort moves to
picking them off**. Widen the corpus again when the open list is short, or
opportunistically when a fix needs a case that doesn't exist yet.

Corpus coverage today: tiers 01–10, **203 cases** (168 AGREE / 20 DIFFER /
12 GAP / 1 OURS_ONLY / 2 BOTH_ERR as of 2026-09-13, after R13 slice 1 pinned
six — [P48](#p48) widened, [P50](#p50)–[P52](#p52) filed). The largest single movement so far remains the 2026-09-05
Corpus coverage today: tiers 01–10, **204 cases** (174 AGREE / 15 DIFFER /
12 GAP / 1 OURS_ONLY / 2 BOTH_ERR as of 2026-09-13, after R13 slice 2 closed
[P48](#p48) and [P50](#p50); [P51](#p51), [P52](#p52) open). The largest single movement so far remains the 2026-09-05
NULL-ordering slice, which closed [P13](#p13) stage 2 and [P17](#p17) together —
eleven cases in one change. **Tier 10 (aggregate & NULL edges) is still
deliberately partial** — it holds the P14, P18–P20 and P41 cases and their
Expand All @@ -92,7 +92,7 @@ Suggested fix order, by silent blast radius:
| ~~9d~~ | ~~[P37](#p37) window in `WHERE` returns 0 rows~~ | ✅ **Fixed 2026-09-05** — corpus count unchanged, and that is the finding: the case is `OURS_ONLY` before *and* after, so the harness cannot see this fix or a future regression of it (first entry of that kind — the regression test is a Rust module). The filed root cause was wrong: `ExpressionLifter` *does* lift from `WHERE`. The real defect was one arm in the WHERE evaluator answering FALSE for any bare value used as a predicate — `WHERE true` returned zero rows too. It did **not** close [P15](#p15), which needs the opposite change |
| ~~9e~~ | ~~[P41](#p41) `MODE` tie-break is random per run~~ | ✅ **Fixed 2026-09-06** — 152 → **156 AGREE** (four new cases). Small, as predicted, but not where it was filed: the named `ModeState` was a *shadowed* implementation and fixing it moved nothing. Reference does specify a rule and it is **first-occurrence**, not the "smallest value wins" this row proposed. Unblocked both example files, now FORMAL. Spun off [P42](#p42), [P43](#p43), [R12](ENGINE_REFACTORING.md#r12) |
| ~~9g~~ | ~~[P46](#p46) column-vs-column `WHERE` returns 0 rows~~ | ✅ **Fixed 2026-09-13** — 157 → **168 AGREE**. Wider than filed: *every* non-literal right-hand operand read as NULL (comparisons, `IN` items, `BETWEEN` bounds), and a literal on the left errored (`WHERE 1=0`). One resolver for all operands closed it. Did **not** need [P48](#p48) alongside, as this row predicted — the fix kept the comparison in the WHERE evaluator. Spun off [P49](#p49) (correlated outer refs bind to the inner table), accepted knowingly |
| **NEXT** | [R13](ENGINE_REFACTORING.md#r13) one expression evaluator — **the active workstream from 2026-09-13** | Not a finding but the reason several are cheap to fix only once. WHERE and the value evaluator implement every boolean operator separately with different NULL rules, so the same predicate answers differently in WHERE, HAVING, SELECT and JOIN ON. **Working rule:** a parity fix that touches expression evaluation lands as an R13 slice, not a patch. [P48](#p48) *is* R13 slice 2 |
| **NEXT** | [R13](ENGINE_REFACTORING.md#r13) one expression evaluator — **the active workstream from 2026-09-13** | **Slices 1–2 done 2026-09-13** (value evaluator three-valued, 168 → 174 AGREE, closed P48/P50); slice 3 (one construction path) next. Not a finding but the reason several are cheap to fix only once. WHERE and the value evaluator implement every boolean operator separately with different NULL rules, so the same predicate answers differently in WHERE, HAVING, SELECT and JOIN ON. **Working rule:** a parity fix that touches expression evaluation lands as an R13 slice, not a patch. [P52](#p52) (NULL join keys) is slice 6 |
| then | [P47](#p47) `MIN`/`MAX` ranked by type | Silent and cheap, and outside the evaluator (aggregate registries — confirm the live one first, [R12](ENGINE_REFACTORING.md#r12)), so it can land alongside R13 without breaking the working rule |
| then | [P14](#p14), [P20](#p20), [P23](#p23) | Smaller, self-contained, decisions already taken. Was row 9b, then NEXT until the 2026-09-12 findings displaced it. **Check each against the R13 working rule first** — P20 (`\|\|` with NULL) is an operator in the value evaluator |
| 9f | [P42](#p42) `MODE` is numeric-only | Companion to [R12](ENGINE_REFACTORING.md#r12), and cheap if taken with it: the shadowed implementation already handles non-numerics and preserves type, so the fix is largely to stop the live path throwing away what it knows. Also buys the corpus its clearest tie-break case |
Expand Down Expand Up @@ -2246,11 +2246,13 @@ out of date.
---

### P48 — The value evaluator is two-valued: a NULL operand projects `false` (or `true`) instead of NULL
- **Status:** 🔴 OPEN — silent, and it disagrees with our own `WHERE` evaluator.
**This is [R13](ENGINE_REFACTORING.md#r13) slice 2.**
- **Status:** 🟢 FIXED 2026-09-13 as [R13](ENGINE_REFACTORING.md#r13) slice 2 —
168 → 174 AGREE with [P50](#p50). All six faults below closed in one change.
- **Corpus:** `01_select.toml :: select_col_vs_col_null_operand`,
`select_eq_null_literal`, `select_in_list_with_null`, `select_and_with_null`
(all `expect = "DIFFER"`). Per operator: `tests/evaluator_matrix_tests.rs`.
`select_eq_null_literal`, `select_in_list_with_null`, `select_and_with_null`,
`simple_case_when_null` (all were DIFFER, now AGREE). Per operator:
`tests/evaluator_matrix_tests.rs`, whose value-evaluator column went from 4/36
to 36/36.
- **Widened 2026-09-13 by the R13 matrix — filed as one comparison, it is the
whole evaluator.** 36 predicates, each run through both evaluators over a
five-row table with NULL in every operand position, expected answers taken
Expand Down Expand Up @@ -2318,6 +2320,19 @@ out of date.
empty string — the same coercion instinct in the string operators),
[P46](#p46).

- **Fix (2026-09-13).** `ArithmeticEvaluator` now evaluates predicates in
three-valued logic with NULL standing for UNKNOWN. Comparisons go through a
predicate-layer `compare_trilean` (NULL on either side → NULL) — the shared
`compare_values` still reports `NULL = NULL` as equal, because ORDER BY needs
it, exactly as R10 left it. `AND`/`OR`/`NOT` use `Trilean`'s truth tables;
`to_bool` is gone. `IN`/`NOT IN` take OR's table, `BETWEEN` AND's, `LIKE`
yields NULL. `Trilean::from_value`/`to_value` are the one crossing between the
two representations. Found at the start of the slice and pinned first: simple
`CASE x WHEN NULL` matched a NULL `x` through a separate `values_equal`, now
also fixed.
- **What did not move, deliberately:** join matching ([P52](#p52), slice 6) and
case-insensitive comparison in this evaluator (slice 3). FORMAL examples had
no expectation churn — none captured a projected `false` for NULL.
---

### P49 — An unknown table prefix silently falls back to the unqualified column
Expand Down Expand Up @@ -2369,8 +2384,11 @@ out of date.
---

### P50 — `HAVING` admits a group whose predicate is UNKNOWN under `NOT`
- **Status:** 🔴 OPEN — silent, too many rows. [R13](ENGINE_REFACTORING.md#r13) slice 2.
- **Corpus:** `07_grouping.toml :: having_not_over_null` (`expect = "DIFFER"`).
- **Status:** 🟢 FIXED 2026-09-13 by [R13](ENGINE_REFACTORING.md#r13) slice 2, with
[P48](#p48). The comparison now reaches `NOT` as NULL, so it stays NULL and
`is_truthy` drops the group. `is_truthy` itself remains, to be replaced by the
single collapse in slice 6.
- **Corpus:** `07_grouping.toml :: having_not_over_null` (was DIFFER, now AGREE).
- **Observed:** `GROUP BY team HAVING NOT (MIN(score) > 40)` on `null_edges.csv`
returns **4** groups; DuckDB returns **3**. The `delta` group's scores are all
NULL, so `MIN(score) > 40` is UNKNOWN and `NOT` of it stays UNKNOWN — the group
Expand Down
Loading
Loading