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
39 changes: 33 additions & 6 deletions docs/ENGINE_REFACTORING.md
Original file line number Diff line number Diff line change
Expand Up @@ -471,7 +471,8 @@ feature work**, and so we can tell the difference between "this is awkward" and
type handling from the dead implementation to the live one is most of that fix.

### R13 — Two expression evaluators, every boolean operator implemented twice
- **Status:** 🔴 OPEN — filed 2026-09-13 out of [P46](SQL_PARITY.md#p46).
- **Status:** 🟡 IN PROGRESS — filed 2026-09-13 out of [P46](SQL_PARITY.md#p46);
**slice 1 done 2026-09-13**, slice 2 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 All @@ -495,14 +496,14 @@ feature work**, and so we can tell the difference between "this is awkward" and
`group_by_expressions::is_truthy` (HAVING). JOIN compares with bare
`compare_with_op`, which answers `NULL = NULL` as equal.
- **Impact: one predicate means different things depending on the clause it is
written in.** Probed 2026-09-13 on `null_edges.csv`, none of it filed yet:
written in.** Probed 2026-09-13 on `null_edges.csv`; filed by slice 1:

| Shape | Where | Ours | SQL / DuckDB |
|---|---|---|---|
| `NOT (score > bonus)`, bonus all NULL | WHERE | no rows ✅ | no rows |
| `NOT (MAX(score) > MIN(bonus))` | HAVING | **all 5 groups** 🚫 | none |
| `score IN (50, NULL)` for a NULL score | SELECT | **true** 🚫 | NULL |
| `a.label = b.label` self-join, 5 NULL labels | JOIN ON | **32 rows** 🚫 (NULLs pair up, 7 + 5×5) | 7 |
| `NOT (MIN(score) > 40)`, one all-NULL group | HAVING | **group kept** 🚫 | dropped — [P50](SQL_PARITY.md#p50) |
| `score IN (50, NULL)` for a NULL score | SELECT | **true** 🚫 | NULL — [P48](SQL_PARITY.md#p48) |
| `a.label = b.label` self-join, 5 NULL labels | JOIN ON | **32 rows** 🚫 (NULLs pair up, 7 + 5×5) | 7 — [P52](SQL_PARITY.md#p52) |
| `id < partner_id` for a NULL partner | SELECT | **false** 🚫 | NULL — [P48](SQL_PARITY.md#p48) |

[P46](SQL_PARITY.md#p46) was the same split from the other side: WHERE lagging
Expand All @@ -523,9 +524,34 @@ feature work**, and so we can tell the difference between "this is awkward" and
`RecursiveWhereEvaluator` shrinks to that adapter plus the LIKE regex cache,
then goes. This is the evaluation counterpart of [R2](#r2): R2 gave traversal
one shape; R13 gives *meaning* one place.
- **Which evaluator is the base — decided with slice 1.** Neither, wholesale.
`ArithmeticEvaluator` has the right *shape*: it maps any expression to a value,
already serves every clause, and routes method calls through the function
registry. `RecursiveWhereEvaluator` has the right *rules*: three-valued logic
(R10) and case-insensitivity. So the value evaluator is the path everything
threads through, and the WHERE evaluator's rules move into it (slice 2) before
WHERE starts delegating to it (slice 4). The matrix below is what makes that
move checkable.
- **Slice 1 result (2026-09-13).** `tests/evaluator_matrix_tests.rs` runs 36
predicates through both evaluators over a five-row table with NULL in every
operand position; the expected per-row answers were generated by DuckDB, not
derived. **WHERE evaluator: 36 of 36. Value evaluator: 4 of 36** — the 32
divergences are recorded in the file's `KNOWN` list, grouped by fault, with the
corpus contract: a recorded divergence that changes, *including being fixed*,
fails the test. The faults are wider than the entry assumed at filing — beyond
"NULL gives false", the value evaluator compares **NULL to NULL as equal**, so
`SELECT a = NULL` is TRUE for NULL rows ([P18](SQL_PARITY.md#p18) itself,
live outside WHERE). Six clause-level corpus cases pin the user-visible forms;
parity AGREE unchanged at 168, 197 → 203 cases. Filed:
[P48](SQL_PARITY.md#p48) widened to the whole evaluator,
[P50](SQL_PARITY.md#p50) HAVING, [P51](SQL_PARITY.md#p51) `AND` in a SELECT
item does not parse (found by the matrix's own first run),
[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.
- **Slices, in the R10 pattern — no-op slices kept apart from the one that
changes answers:**
1. **Pin the divergences, no engine change.** A per-operator matrix run through
1. ✅ **Pin the divergences, no engine change.** *(Done 2026-09-13.)* A per-operator matrix run through
both evaluators — `=`, `<>`, `IN`, `NOT IN`, `BETWEEN`, `LIKE`, `NOT`,
`AND`/`OR`, `CASE WHEN` — each with NULL on either side, asserting the two
agree, with today's disagreements marked. Plus corpus cases for the
Expand Down Expand Up @@ -626,3 +652,4 @@ AGREE count — which makes it safe to land well before the semantics change.
| 2026-09-06 | R12 filed by parity P41: two aggregate registries, nine functions implemented twice with the newer one shadowing the older. P41's fix was written against the dead copy first and changed nothing — the entry records the disjointness assertion as step 1 | — |
| 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 | — |
111 changes: 104 additions & 7 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, **197 cases** (168 AGREE / 15 DIFFER /
11 GAP / 1 OURS_ONLY / 2 BOTH_ERR as of 2026-09-13, after the [P46](#p46) fix
added nine and moved eleven — see that entry and [P49](#p49)). The largest single movement so far remains the 2026-09-05
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
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 Down Expand Up @@ -2245,10 +2245,32 @@ out of date.

---

### P48 — A comparison with a NULL operand projects `false` instead of NULL
- **Status:** 🔴 OPEN — silent, and it disagrees with our own `WHERE` evaluator
- **Corpus:** `01_select.toml :: select_col_vs_col_null_operand`
(`expect = "DIFFER"`).
### 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.**
- **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`.
- **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
from DuckDB. The WHERE evaluator agrees on **36 of 36**; the value evaluator
(SELECT, HAVING, join conditions) on **4** — `IS NULL`, `IS NOT NULL` and the
two `CASE` rows. Six faults, one evaluator:

| Fault | Example | Value evaluator | SQL |
|---|---|---|---|
| NULL vs a value is false | `a < b`, `a` NULL | `false` | NULL |
| **NULL vs NULL compares equal** | `a = NULL`, `a = b` both NULL | **`true`** | NULL |
| `NOT` negates that false to true | `NOT (a < b)`, `a` NULL | **`true`** | NULL |
| `IN`/`NOT IN` two-valued, NULL matches NULL | `a IN (1, NULL)`, `a` NULL | **`true`** | NULL |
| `BETWEEN`, `LIKE` two-valued | `a BETWEEN 1 AND b`, `b` NULL | `false` | NULL |
| `AND`/`OR` collapse through `to_bool` | `a > 5 AND b > 5`, both NULL | `false` | NULL |

The second row is [P18](#p18) exactly — `= NULL` behaving like `IS NULL` — the
bug [R10](ENGINE_REFACTORING.md#r10) fixed in WHERE in 2026-08, still live
wherever the value evaluator runs. The third is how it escapes projection and
becomes a wrong *filter*: see [P50](#p50).
- **Observed:** a projected comparison coerces a NULL operand to a definite
answer:

Expand Down Expand Up @@ -2346,6 +2368,81 @@ 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"`).
- **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
must drop. We keep it.
- **Cause:** HAVING runs on the value evaluator ([P48](#p48)), which answers the
comparison `false`; its `NOT` arm is three-valued but receives `false`, not
NULL, and negates it to `true`. HAVING then collapses with its own
`group_by_expressions::is_truthy` — a third "is this true?" rule, beside
WHERE's `Trilean::is_true` and the evaluator's `to_bool`.
- **Why a separate entry from P48:** P48 is a wrong *value*; this is a wrong
*row set*, and the same predicate is correct in WHERE. It is the
[P19](#p19) shape — UNKNOWN turned TRUE under negation — at the clause R10 did
not reach.
- **Found:** 2026-09-13, writing R13 — probed directly, then pinned. The first
draft of the case used `bonus` (all NULL), which DuckDB types as VARCHAR and
refuses to compare; `score` with one all-NULL group discriminates cleanly.
- **Decision:** fix as R13 slice 2 (the value evaluator becomes three-valued), with
slice 6 replacing `is_truthy` with the single sanctioned collapse. No spot fix.
- **Related:** [P48](#p48), [P19](#p19), [P9](#p9)/[P10](#p10) (earlier HAVING
findings from the same evaluator).

---

### P51 — `AND` / `OR` are not accepted in a `SELECT` item unless parenthesised
- **Status:** 🔴 OPEN — hard error
- **Corpus:** `01_select.toml :: select_and_unparenthesised` (`expect = "GAP"`);
the parenthesised form is `select_and_with_null` (parses, DIFFER on P48).
- **Observed:** `SELECT id, score > 40 AND team = 'alpha' AS both FROM t` →
*"Unexpected keyword 'AND' after end of statement"*. `SELECT id, (score > 40
AND team = 'alpha') AS both` parses and evaluates. `NOT (…)` and
`CASE WHEN a AND b …` are fine.
- **Cause, likely:** the select-item parser enters the expression grammar below
the logical-operator level, so it stops at `AND` and hands the rest back to the
statement parser. Not yet confirmed in `recursive_parser.rs`.
- **Found:** 2026-09-13, by the R13 matrix itself — its first run could not parse
`SELECT a > 0 AND b > 1 AS v`. The matrix now parenthesises its SELECT form.
- **Decision:** fix; a parser change, independent of R13's evaluator work. Loud,
so it queues behind the silent findings.
- **Related:** [P29](#p29)/[P30](#p30) (where `IN` sat relative to the AND/OR
hierarchy — the same class of precedence placement).

---

### P52 — A join on NULL keys pairs the NULL rows with each other
- **Status:** 🔴 OPEN — silent, too many rows. [R13](ENGINE_REFACTORING.md#r13) slice 6.
- **Corpus:** `10_aggregate_nulls.toml :: join_on_null_key_both_sides`
(`expect = "DIFFER"`). Its neighbour `join_on_null_key` AGREEs and could not
see this: it has NULL on only one side.
- **Observed:** `null_edges a JOIN null_edges b ON a.label = b.label` returns
**32** rows; DuckDB **7**. Seven distinct non-NULL labels match themselves;
the five NULL-label rows then pair with each other, 5 × 5 = 25.
- **Cause — both join paths:**
- **Hash path** (single-column equi-join, which this is):
`hash_join::canonical_join_key` passes `DataValue::Null` through as an
ordinary `HashMap` key, so a NULL probe finds the NULL build rows.
- **Nested-loop path:** `compare_values` defers to `compare_with_op`, and
`value_comparisons::compare_values` reports `(Null, Null)` as `Equal` — by
design, because `ORDER BY` needs NULLs to group (see R10 1c).
- **Why it matters beyond this table:** joining two extracts on an optional key
(a missing counterparty id, an unset parent) silently multiplies every
keyless row by every other — and a `SUM` over the join inherits the inflation.
- **Found:** 2026-09-13, writing R13.
- **Decision:** fix in R13 slice 6, where join matching moves onto the single
predicate collapse; the hash path must skip NULL keys on build *and* probe (a
`LEFT JOIN` still emits the left row, unmatched). Check both paths against the
corpus case — only one of them is exercised by it today.
- **Related:** [P18](#p18) (the same `NULL = NULL` answer, fixed in WHERE by
testing for NULL at the predicate layer rather than in the comparator),
[P48](#p48).

---

## Deferred / won't fix (intentional)

### D1 — Recursive CTEs (`WITH RECURSIVE`)
Expand Down
40 changes: 40 additions & 0 deletions tests/comparison/corpus/01_select.toml
Original file line number Diff line number Diff line change
Expand Up @@ -83,3 +83,43 @@ expect = "DIFFER"
# the IS NOT NULL guard: for a NULL operand we project false, DuckDB projects
# NULL. The three-valued-logic fix of P18/P19 landed in the WHERE evaluator;
# this is the same rule missing at the projection site.

# --- R13 slice 1 (2026-09-13): the value evaluator is two-valued ---
# tests/evaluator_matrix_tests.rs runs 36 predicates through both evaluators;
# the WHERE evaluator agrees with DuckDB on all 36 and the value evaluator on 4.
# These pin the projection-visible faults end to end. All are R13 slice 2.

[[case]]
id = "select_eq_null_literal"
data = "null_edges.csv"
sql = "SELECT id, score = NULL AS eq FROM null_edges ORDER BY id"
expect = "DIFFER"
# P48. `= NULL` is UNKNOWN for every row. The value evaluator compares NULL to
# NULL as *equal*, so the NULL-scored rows project TRUE - P18, the bug R10 fixed
# in WHERE, still live in SELECT.

[[case]]
id = "select_in_list_with_null"
data = "null_edges.csv"
sql = "SELECT id, score IN (50, NULL) AS hit FROM null_edges ORDER BY id"
expect = "DIFFER"
# P48. A NULL probe matches the NULL item (TRUE, should be NULL), and a
# non-matching score against a list holding NULL is FALSE (should be NULL).

[[case]]
id = "select_and_with_null"
data = "null_edges.csv"
sql = "SELECT id, (score > 40 AND team = 'alpha') AS both FROM null_edges ORDER BY id"
expect = "DIFFER"
# P48. AND collapses each side through to_bool (NULL -> false), so TRUE AND
# UNKNOWN projects false instead of NULL. Parenthesised because the bare form
# does not parse: see select_and_unparenthesised (P51).

[[case]]
id = "select_and_unparenthesised"
data = "null_edges.csv"
sql = "SELECT id, score > 40 AND team = 'alpha' AS both FROM null_edges ORDER BY id"
expect = "GAP"
# P51, found 2026-09-13 building the R13 matrix. AND/OR are not accepted in a
# SELECT item: "Unexpected keyword 'AND' after end of statement". The
# parenthesised form above parses. A parser gap, not an evaluator one.
11 changes: 11 additions & 0 deletions tests/comparison/corpus/07_grouping.toml
Original file line number Diff line number Diff line change
Expand Up @@ -62,3 +62,14 @@ sql = "SELECT region, COUNT(*) AS n FROM international_sales GROUP BY region HAV
# Was P10: the aggregate IS rewritten here, but the arithmetic evaluator had no
# arm for Not, so this errored. Fixed 2026-07-25 by adding the Not arm
# (three-valued: NOT NULL -> NULL). Now AGREE.

[[case]]
id = "having_not_over_null"
data = "null_edges.csv"
sql = "SELECT team, COUNT(*) AS n FROM null_edges GROUP BY team HAVING NOT (MIN(score) > 40) ORDER BY team"
expect = "DIFFER"
# P50, found 2026-09-13 writing R13. The `delta` group's scores are all NULL, so
# MIN(score) > 40 is UNKNOWN there and NOT of it stays UNKNOWN: DuckDB keeps
# beta, gamma and the NULL team, and drops delta. HAVING runs on the value
# evaluator, which answers the comparison false, negates it to TRUE, and admits
# delta. The same predicate in WHERE is right - see tests/evaluator_matrix_tests.rs.
13 changes: 13 additions & 0 deletions tests/comparison/corpus/10_aggregate_nulls.toml
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,19 @@ data = "null_edges.csv"
sql = "SELECT a.id AS aid, b.id AS bid FROM null_edges a JOIN null_edges b ON a.partner_id = b.id ORDER BY a.id"
# NULL never equals NULL in a join key, so the 5 NULL partner_id rows drop out.

[[case]]
id = "join_on_null_key_both_sides"
data = "null_edges.csv"
sql = "SELECT a.id AS aid, b.id AS bid FROM null_edges a JOIN null_edges b ON a.label = b.label ORDER BY a.id, b.id"
expect = "DIFFER"
# P52, found 2026-09-13 writing R13. The case above only has NULL on ONE side
# (partner_id against the never-NULL id), so it cannot see this: with NULL keys
# on both sides, the 5 NULL-label rows pair with each other. We return 32 rows
# (7 + 5x5), DuckDB 7. Both join paths do it: this equi-join takes the hash
# path, where `canonical_join_key` passes NULL through as an ordinary HashMap
# key, so a NULL probe finds the NULL build rows; the nested-loop path uses
# compare_with_op, which also reports NULL = NULL as equal. R13 slice 6.

[[case]]
id = "null_arithmetic"
data = "null_edges.csv"
Expand Down
Loading
Loading