diff --git a/docs/ENGINE_REFACTORING.md b/docs/ENGINE_REFACTORING.md index 2968753d..10ac9c6e 100644 --- a/docs/ENGINE_REFACTORING.md +++ b/docs/ENGINE_REFACTORING.md @@ -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 โ†’ @@ -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 @@ -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 @@ -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 | โ€” | diff --git a/docs/SQL_PARITY.md b/docs/SQL_PARITY.md index 906ae317..9085f610 100644 --- a/docs/SQL_PARITY.md +++ b/docs/SQL_PARITY.md @@ -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 @@ -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: @@ -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`) diff --git a/tests/comparison/corpus/01_select.toml b/tests/comparison/corpus/01_select.toml index 0f8c01b5..dab203b8 100644 --- a/tests/comparison/corpus/01_select.toml +++ b/tests/comparison/corpus/01_select.toml @@ -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. diff --git a/tests/comparison/corpus/07_grouping.toml b/tests/comparison/corpus/07_grouping.toml index 921eff24..52461fb3 100644 --- a/tests/comparison/corpus/07_grouping.toml +++ b/tests/comparison/corpus/07_grouping.toml @@ -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. diff --git a/tests/comparison/corpus/10_aggregate_nulls.toml b/tests/comparison/corpus/10_aggregate_nulls.toml index a730e98d..7b2124dc 100644 --- a/tests/comparison/corpus/10_aggregate_nulls.toml +++ b/tests/comparison/corpus/10_aggregate_nulls.toml @@ -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" diff --git a/tests/evaluator_matrix_tests.rs b/tests/evaluator_matrix_tests.rs new file mode 100644 index 00000000..d8eb5715 --- /dev/null +++ b/tests/evaluator_matrix_tests.rs @@ -0,0 +1,447 @@ +//! R13 slice 1 โ€” the evaluator matrix. +//! +//! The engine has two evaluators for the same expression tree, and they do not +//! share rules (docs/ENGINE_REFACTORING.md ยง R13): +//! +//! - `RecursiveWhereEvaluator` โ€” predicate โ†’ `Trilean`, serves `WHERE`. +//! - `ArithmeticEvaluator` โ€” expression โ†’ `DataValue`, serves `SELECT`, +//! `HAVING`, join conditions and, since P46, WHERE operands. +//! +//! This runs every predicate below through **both**, on every row of a table +//! built so that each operand is NULL somewhere, and compares each answer with +//! the three-valued SQL result. The expected strings were produced by DuckDB +//! (the parity reference), not derived by hand โ€” see `EXPECTED`. +//! +//! The contract is the same as the parity corpus: a divergence is *recorded*, +//! with what we actually return, in `KNOWN`. The test fails if an unrecorded +//! divergence appears **or** if a recorded one changes โ€” including being fixed, +//! so R13 slice 2 has to update this file to prove what it moved. +//! +//! Reading a row string: one character per table row, in row order. +//! `T`/`F`/`U` are TRUE/FALSE/UNKNOWN (NULL); `E` means the evaluator errored, +//! and `V` means the value evaluator returned a non-boolean, non-NULL value. + +use sql_cli::data::arithmetic_evaluator::ArithmeticEvaluator; +use sql_cli::data::datatable::{DataColumn, DataRow, DataTable, DataValue}; +use sql_cli::data::recursive_where_evaluator::RecursiveWhereEvaluator; +use sql_cli::data::trilean::Trilean; +use sql_cli::sql::parser::ast::SelectItem; +use sql_cli::sql::recursive_parser::Parser; + +/// Rows chosen so every binary shape sees a non-NULL pair both ways round, NULL +/// on the left, NULL on the right, and NULL on both. +/// +/// | row | a | b | s | +/// |-----|------|------|-------| +/// | 0 | 1 | 2 | 'abc' | +/// | 1 | 2 | 1 | 'xyz' | +/// | 2 | NULL | 1 | NULL | +/// | 3 | 1 | NULL | 'abc' | +/// | 4 | NULL | NULL | NULL | +fn table() -> DataTable { + let mut t = DataTable::new("t"); + t.add_column(DataColumn::new("a")); + t.add_column(DataColumn::new("b")); + t.add_column(DataColumn::new("s")); + let int = |v: Option| v.map_or(DataValue::Null, DataValue::Integer); + let text = |v: Option<&str>| v.map_or(DataValue::Null, |s| DataValue::String(s.to_string())); + for (a, b, s) in [ + (Some(1), Some(2), Some("abc")), + (Some(2), Some(1), Some("xyz")), + (None, Some(1), None), + (Some(1), None, Some("abc")), + (None, None, None), + ] { + t.add_row(DataRow::new(vec![int(a), int(b), text(s)])) + .unwrap(); + } + t +} + +const ROWS: usize = 5; + +/// `(predicate, SQL answer per row)`, generated by DuckDB from the same table: +/// `SELECT CASE WHEN (p) IS NULL THEN 'U' WHEN (p) THEN 'T' ELSE 'F' END`. +const EXPECTED: &[(&str, &str)] = &[ + // Comparisons, with NULL on each side. + ("a < b", "TFUUU"), + ("a = b", "FFUUU"), + ("a <> b", "TTUUU"), + ("a >= b", "FTUUU"), + ("a = 1", "TFUTU"), + ("1 < b", "TFFUU"), + ("a = NULL", "UUUUU"), + ("s = 'abc'", "TFUTU"), + // NULL tests stay two-valued. + ("a IS NULL", "FFTFT"), + ("a IS NOT NULL", "TTFTF"), + // Negation: NOT UNKNOWN is UNKNOWN. + ("NOT (a < b)", "FTUUU"), + ("NOT (a = 1)", "FTUFU"), + // IN / NOT IN, NULL in the probe, in the list, and a column item. + ("a IN (1, 3)", "TFUTU"), + ("a IN (1, NULL)", "TUUTU"), + ("a IN (b, 3)", "FFUUU"), + ("a NOT IN (3, 4)", "TTUTU"), + ("a NOT IN (1, NULL)", "FUUFU"), + ("NOT (a IN (1, 3))", "FTUFU"), + // BETWEEN, NULL in the value and in either bound. + ("a BETWEEN 1 AND 5", "TTUTU"), + ("a BETWEEN 1 AND b", "TFUUU"), + ("a BETWEEN b AND 5", "FTUUU"), + ("NOT (a BETWEEN 1 AND b)", "FTUUU"), + // AND / OR: FALSE dominates AND, TRUE dominates OR, even over UNKNOWN. + ("a > 0 AND b > 1", "TFFUU"), + ("a > 1 OR b > 1", "TTUUU"), + ("a > 5 AND b > 5", "FFFFU"), + ("a > 5 OR b IS NULL", "FFUTT"), + ("NOT (a > 0 AND b > 1)", "FTTUU"), + ("NOT (a > 1 OR b > 1)", "FFUUU"), + // LIKE. + ("s LIKE 'a%'", "TFUTU"), + ("NOT (s LIKE 'a%')", "FTUFU"), + // Expression operands. + ("a + 0 < b", "TFUUU"), + ("a + b > 2", "TTUUU"), + ("COALESCE(a, 0) < b", "TFTUU"), + ("NOT (a + 0 < b)", "FTUUU"), + // CASE: an UNKNOWN condition takes the ELSE branch. + ("CASE WHEN a < b THEN 1 ELSE 0 END = 1", "TFFFF"), + ("CASE WHEN a < b THEN true ELSE false END", "TFFFF"), +]; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum Evaluator { + /// `RecursiveWhereEvaluator`, as `WHERE `. + Where, + /// `ArithmeticEvaluator`, as `SELECT () AS v`. + Value, +} + +/// A recorded divergence from `EXPECTED`: what this evaluator returns today, +/// and why. Remove an entry when a change makes the evaluator agree. +struct Known { + evaluator: Evaluator, + predicate: &'static str, + observed: &'static str, + why: &'static str, +} + +const COMPARISON: &str = "value evaluator compares without a NULL guard (P48)"; +const NOT_OVER_FALSE: &str = + "NOT receives false for a NULL comparison and negates it to TRUE (P48, P50)"; +const IN_LIST: &str = "value evaluator IN/NOT IN is two-valued and matches NULL to NULL (P48)"; +const BETWEEN: &str = "value evaluator BETWEEN is two-valued (P48)"; +const LIKE: &str = "value evaluator LIKE answers false for NULL (P48)"; +const AND_OR: &str = "value evaluator AND/OR collapse NULL to false via to_bool (P48)"; + +const KNOWN: &[Known] = &[ + // Comparisons. `compare_with_op` with no NULL guard: NULL against a value is + // false, and NULL against NULL compares *equal*, so `=`, `<=`, `>=` answer TRUE + // (the P18 rule R10 fixed in WHERE, still live here). P48. + Known { + evaluator: Evaluator::Value, + predicate: "a < b", + observed: "TFFFF", + why: COMPARISON, + }, + Known { + evaluator: Evaluator::Value, + predicate: "a = b", + observed: "FFFFT", + why: COMPARISON, + }, + Known { + evaluator: Evaluator::Value, + predicate: "a <> b", + observed: "TTFFF", + why: COMPARISON, + }, + Known { + evaluator: Evaluator::Value, + predicate: "a >= b", + observed: "FTFFT", + why: COMPARISON, + }, + Known { + evaluator: Evaluator::Value, + predicate: "a = 1", + observed: "TFFTF", + why: COMPARISON, + }, + Known { + evaluator: Evaluator::Value, + predicate: "1 < b", + observed: "TFFFF", + why: COMPARISON, + }, + Known { + evaluator: Evaluator::Value, + predicate: "a = NULL", + observed: "FFTFT", + why: COMPARISON, + }, + Known { + evaluator: Evaluator::Value, + predicate: "s = 'abc'", + observed: "TFFTF", + why: COMPARISON, + }, + Known { + evaluator: Evaluator::Value, + predicate: "a + 0 < b", + observed: "TFFFF", + why: COMPARISON, + }, + Known { + evaluator: Evaluator::Value, + predicate: "a + b > 2", + observed: "TTFFF", + why: COMPARISON, + }, + Known { + evaluator: Evaluator::Value, + predicate: "COALESCE(a, 0) < b", + observed: "TFTFF", + why: COMPARISON, + }, + // NOT itself is three-valued in the value evaluator, but its operand arrives + // as `false`, so every NULL row negates to TRUE. This is how HAVING admits + // groups it must exclude (P50). + Known { + evaluator: Evaluator::Value, + predicate: "NOT (a < b)", + observed: "FTTTT", + why: NOT_OVER_FALSE, + }, + Known { + evaluator: Evaluator::Value, + predicate: "NOT (a = 1)", + observed: "FTTFT", + why: NOT_OVER_FALSE, + }, + Known { + evaluator: Evaluator::Value, + predicate: "NOT (a + 0 < b)", + observed: "FTTTT", + why: NOT_OVER_FALSE, + }, + // IN / NOT IN: two-valued, built on the same equality, so a NULL probe matches + // a NULL item and a miss is FALSE rather than UNKNOWN. P48. + Known { + evaluator: Evaluator::Value, + predicate: "a IN (1, 3)", + observed: "TFFTF", + why: IN_LIST, + }, + Known { + evaluator: Evaluator::Value, + predicate: "a IN (1, NULL)", + observed: "TFTTT", + why: IN_LIST, + }, + Known { + evaluator: Evaluator::Value, + predicate: "a IN (b, 3)", + observed: "FFFFT", + why: IN_LIST, + }, + Known { + evaluator: Evaluator::Value, + predicate: "a NOT IN (3, 4)", + observed: "TTTTT", + why: IN_LIST, + }, + Known { + evaluator: Evaluator::Value, + predicate: "a NOT IN (1, NULL)", + observed: "FTFFF", + why: IN_LIST, + }, + Known { + evaluator: Evaluator::Value, + predicate: "NOT (a IN (1, 3))", + observed: "FTTFT", + why: IN_LIST, + }, + // BETWEEN: two plain comparisons joined with `&&`. P48. + Known { + evaluator: Evaluator::Value, + predicate: "a BETWEEN 1 AND 5", + observed: "TTFTF", + why: BETWEEN, + }, + Known { + evaluator: Evaluator::Value, + predicate: "a BETWEEN 1 AND b", + observed: "TFFFF", + why: BETWEEN, + }, + Known { + evaluator: Evaluator::Value, + predicate: "a BETWEEN b AND 5", + observed: "FTFFF", + why: BETWEEN, + }, + Known { + evaluator: Evaluator::Value, + predicate: "NOT (a BETWEEN 1 AND b)", + observed: "FTTTT", + why: BETWEEN, + }, + // LIKE: a NULL operand answers false. P48. + Known { + evaluator: Evaluator::Value, + predicate: "s LIKE 'a%'", + observed: "TFFTF", + why: LIKE, + }, + Known { + evaluator: Evaluator::Value, + predicate: "NOT (s LIKE 'a%')", + observed: "FTTFT", + why: LIKE, + }, + // AND / OR: operands collapse through `to_bool` (NULL -> false), so there are + // no truth tables and UNKNOWN can never survive. P48. + Known { + evaluator: Evaluator::Value, + predicate: "a > 0 AND b > 1", + observed: "TFFFF", + why: AND_OR, + }, + Known { + evaluator: Evaluator::Value, + predicate: "a > 1 OR b > 1", + observed: "TTFFF", + why: AND_OR, + }, + Known { + evaluator: Evaluator::Value, + predicate: "a > 5 AND b > 5", + observed: "FFFFF", + why: AND_OR, + }, + Known { + evaluator: Evaluator::Value, + predicate: "a > 5 OR b IS NULL", + observed: "FFFTT", + why: AND_OR, + }, + Known { + evaluator: Evaluator::Value, + predicate: "NOT (a > 0 AND b > 1)", + observed: "FTTTT", + why: AND_OR, + }, + Known { + evaluator: Evaluator::Value, + predicate: "NOT (a > 1 OR b > 1)", + observed: "FFTTT", + why: AND_OR, + }, +]; + +fn trilean_char(t: Trilean) -> char { + match t { + Trilean::True => 'T', + Trilean::False => 'F', + Trilean::Unknown => 'U', + } +} + +fn observe_where(table: &DataTable, predicate: &str) -> String { + let sql = format!("SELECT * FROM t WHERE {predicate}"); + let statement = Parser::new(&sql) + .parse() + .unwrap_or_else(|e| panic!("parse failed for {sql}: {e}")); + let where_clause = statement.where_clause.expect("expected a WHERE clause"); + let mut evaluator = RecursiveWhereEvaluator::new(table); + (0..ROWS) + .map(|row| match evaluator.evaluate(&where_clause, row) { + Ok(t) => trilean_char(t), + Err(_) => 'E', + }) + .collect() +} + +fn observe_value(table: &DataTable, predicate: &str) -> String { + // Parenthesised, because a bare `SELECT a AND b` does not parse (logged as + // a parity gap of its own); the parentheses do not change the expression. + let sql = format!("SELECT ({predicate}) AS v FROM t"); + let statement = Parser::new(&sql) + .parse() + .unwrap_or_else(|e| panic!("parse failed for {sql}: {e}")); + let expr = match statement.select_items.first() { + Some(SelectItem::Expression { expr, .. }) => expr.clone(), + other => panic!("expected one expression select item for {sql}, got {other:?}"), + }; + let mut evaluator = ArithmeticEvaluator::new(table); + (0..ROWS) + .map(|row| match evaluator.evaluate(&expr, row) { + Ok(DataValue::Boolean(true)) => 'T', + Ok(DataValue::Boolean(false)) => 'F', + Ok(DataValue::Null) => 'U', + Ok(_) => 'V', + Err(_) => 'E', + }) + .collect() +} + +#[test] +fn every_predicate_through_both_evaluators() { + let t = table(); + let mut problems = Vec::new(); + let mut matrix = Vec::new(); + + for &(predicate, expected) in EXPECTED { + let where_got = observe_where(&t, predicate); + let value_got = observe_value(&t, predicate); + matrix.push(format!( + "{predicate:<44} expected {expected} where {where_got} value {value_got}" + )); + + for (evaluator, got) in [ + (Evaluator::Where, &where_got), + (Evaluator::Value, &value_got), + ] { + let known = KNOWN + .iter() + .find(|k| k.evaluator == evaluator && k.predicate == predicate); + match known { + None if got != expected => problems.push(format!( + "UNRECORDED {evaluator:?} `{predicate}`: expected {expected}, got {got}" + )), + Some(k) if k.observed == expected => problems.push(format!( + "STALE ENTRY {evaluator:?} `{predicate}`: KNOWN records the correct answer {expected}; remove it" + )), + Some(k) if got == expected => problems.push(format!( + "FIXED {evaluator:?} `{predicate}` now agrees ({expected}); remove its KNOWN entry ({})", + k.why + )), + Some(k) if got != k.observed => problems.push(format!( + "CHANGED {evaluator:?} `{predicate}`: KNOWN records {}, now {got} (expected {expected})", + k.observed + )), + _ => {} + } + } + } + + // Every KNOWN entry must name a predicate that is still in the matrix. + for k in KNOWN { + if !EXPECTED.iter().any(|(p, _)| *p == k.predicate) { + problems.push(format!( + "ORPHAN {:?} `{}` is in KNOWN but not in EXPECTED", + k.evaluator, k.predicate + )); + } + } + + assert!( + problems.is_empty(), + "\n{}\n\nFull matrix:\n{}\n", + problems.join("\n"), + matrix.join("\n") + ); +} diff --git a/tests/main.rs b/tests/main.rs index 0fc1ed65..d286d717 100644 --- a/tests/main.rs +++ b/tests/main.rs @@ -228,3 +228,6 @@ mod dotted_column_completion; #[path = "asterisk_expansion.rs"] mod asterisk_expansion; + +#[path = "evaluator_matrix_tests.rs"] +mod evaluator_matrix_tests;