From 5bc3a2f7d779269031dc1c3f0df2c1120c55832f Mon Sep 17 00:00:00 2001 From: TimelordUK Date: Sat, 19 Sep 2026 10:15:09 +0100 Subject: [PATCH 1/8] refactor(r13): WHERE's IS NULL / IS NOT NULL delegate to the value evaluator (R13 slice 4) Both NULL tests join the comparisons in `delegates_to_value_evaluator` (renamed from `is_comparison_operator`). A genuine no-op: the parser always puts a `Null` literal on the right, and a NULL test only asks whether the left operand is NULL, which no difference between the two operand readers can change. The legacy method-call-on-the-left exception still applies. Deleted WHERE's bare `IS` / `IS NOT` arms, and dropped `==`, `IS` and `IS NOT` from `is_predicate_operator`: the parser never emits them, and an operator nothing produces should fail loudly rather than be half-handled. Parity report byte-identical, matrix green (all three sets); WHERE comments IS NULL over trades_100000.csv unchanged at load time. Co-Authored-By: Claude Opus 5 (1M context) --- src/data/recursive_where_evaluator.rs | 44 +++++++++------------------ 1 file changed, 14 insertions(+), 30 deletions(-) diff --git a/src/data/recursive_where_evaluator.rs b/src/data/recursive_where_evaluator.rs index 137f25d2..04babf0c 100644 --- a/src/data/recursive_where_evaluator.rs +++ b/src/data/recursive_where_evaluator.rs @@ -606,12 +606,14 @@ impl<'a, 'ctx, 'exec> RecursiveWhereEvaluator<'a, 'ctx, 'exec> { return self.evaluate_value_as_predicate(expr, row_index); } - // A comparison is the value evaluator's (R13 slice 4): its operand - // readers and three-valued `compare_trilean` are the one + // Comparisons and NULL tests are the value evaluator's (R13 slice 4): + // its operand readers and three-valued `compare_trilean` are the one // implementation. Two shapes stay below for now - a legacy method call // on the left (`Length()`, `IndexOf()`, `Trim*()`), whose registry - // twins are slice 5's to check, and the operators with their own arms. - if is_comparison_operator(&op_upper) && !matches!(left, SqlExpression::MethodCall { .. }) { + // twins are slice 5's to check, and LIKE. + if delegates_to_value_evaluator(&op_upper) + && !matches!(left, SqlExpression::MethodCall { .. }) + { reject_unlifted_window(left)?; reject_unlifted_window(right)?; return self.evaluate_delegated(expr, row_index); @@ -733,16 +735,6 @@ impl<'a, 'ctx, 'exec> RecursiveWhereEvaluator<'a, 'ctx, 'exec> { cell_value.is_some() && !matches!(cell_value, Some(DataValue::Null)), )), - // `x IS NULL` written with an explicit NULL operand. Matched on the - // expression, not the value: `a IS b` where b happens to be NULL on - // this row is not a NULL test. - "IS" if matches!(right, SqlExpression::Null) => Ok(Trilean::from_bool( - cell_value.is_none() || matches!(cell_value, Some(DataValue::Null)), - )), - "IS NOT" if matches!(right, SqlExpression::Null) => Ok(Trilean::from_bool( - cell_value.is_some() && !matches!(cell_value, Some(DataValue::Null)), - )), - // Standard comparison operators - use centralized logic _ => { let table_value = cell_value.unwrap_or(DataValue::Null); @@ -1162,10 +1154,13 @@ fn reject_unlifted_window(expr: &SqlExpression) -> Result<()> { } } -/// The comparison operators the value evaluator implements, and so the ones a -/// WHERE comparison delegates. Expects the operator already upper-cased. -fn is_comparison_operator(op_upper: &str) -> bool { - matches!(op_upper, "=" | "!=" | "<>" | "<" | "<=" | ">" | ">=") +/// The binary predicates WHERE hands whole to the value evaluator: the +/// comparisons and the NULL tests. Expects the operator already upper-cased. +fn delegates_to_value_evaluator(op_upper: &str) -> bool { + matches!( + op_upper, + "=" | "!=" | "<>" | "<" | "<=" | ">" | ">=" | "IS NULL" | "IS NOT NULL" + ) } /// Operators whose result is a truth value, handled by `evaluate_binary_op`'s @@ -1173,18 +1168,7 @@ fn is_comparison_operator(op_upper: &str) -> bool { fn is_predicate_operator(op_upper: &str) -> bool { matches!( op_upper, - "=" | "==" - | "!=" - | "<>" - | "<" - | "<=" - | ">" - | ">=" - | "LIKE" - | "IS" - | "IS NOT" - | "IS NULL" - | "IS NOT NULL" + "=" | "!=" | "<>" | "<" | "<=" | ">" | ">=" | "LIKE" | "IS NULL" | "IS NOT NULL" ) } From 084cf683c15a1452e34b2dafab95212cdc7ae907 Mon Sep 17 00:00:00 2001 From: TimelordUK Date: Sat, 19 Sep 2026 10:17:16 +0100 Subject: [PATCH 2/8] test(r13): pin LIKE before it delegates; file P55 (R13 slice 4) A fourth matrix, generated by DuckDB, aimed at what separates the two LIKE matchers: pattern characters that are regex syntax, and text across a newline. The value evaluator gets all ten right. WHERE gets six wrong: P55 (filed): WHERE compiles a LIKE pattern to a regex by replacing `%` and `_` and escaping nothing else. `LIKE 'a.c'` matches `abc`, `LIKE '[a]bc'` matches `abc` but not `[a]bc`, `LIKE 'a(b%'` fails the whole query with a regex parse error, and `%` does not cross a newline. Same bug in the cached path every query takes (EvaluationContext) and the uncached fallback. Three corpus cases pin it end to end over new data/like_patterns.csv: where_like_dot_is_literal, where_like_bracket_is_literal (DIFFER), where_like_paren_is_literal (GAP). Newline shapes stay matrix-only. Parity 176 AGREE / 209, contract holds. Co-Authored-By: Claude Opus 5 (1M context) --- docs/SQL_PARITY.md | 25 +++++++ tests/comparison/corpus/02_where.toml | 26 ++++++++ tests/evaluator_matrix_tests.rs | 96 +++++++++++++++++++++++++++ 3 files changed, 147 insertions(+) diff --git a/docs/SQL_PARITY.md b/docs/SQL_PARITY.md index d95887b6..e24e6f88 100644 --- a/docs/SQL_PARITY.md +++ b/docs/SQL_PARITY.md @@ -2510,6 +2510,31 @@ out of date. the next such change is seen. - **Found:** 2026-09-18, pinning the operand readers for R13 slice 4. +### P55 — A `LIKE` pattern in `WHERE` is read as a regex +- **Status:** 🔴 OPEN — silent wrong rows, or a failed query. + [R13](ENGINE_REFACTORING.md#r13) slice 4 (LIKE delegates) closes it. +- **Corpus:** `02_where.toml :: where_like_dot_is_literal`, + `where_like_bracket_is_literal` (`expect = "DIFFER"`), + `where_like_paren_is_literal` (`expect = "GAP"`). The newline shapes are + pinned in the evaluator matrix only (`EXPECTED_LIKE`); a CSV value with an + embedded newline would test the loader as much as LIKE. +- **Observed:** `WHERE s LIKE 'a.c'` returns `abc` as well as `a.c`; + `LIKE '[a]bc'` returns `abc` and misses `[a]bc`; `LIKE 'a(b%'` fails the + query with "regex parse error â€Ļ unclosed group"; `LIKE '%'` misses a value + containing a newline. +- **Cause:** WHERE builds its matcher by string replacement — + `pattern.replace('%', ".*").replace('_', ".")` — in both + `EvaluationContext::get_or_compile_like_regex` (the cached path every query + takes) and the uncached fallback in `recursive_where_evaluator.rs`. Nothing + else is escaped, so every regex metacharacter (`. [ ] ( ) + * ? ^ $ | \ { }`) + in a pattern is live, and `.` does not match `\n`. The value evaluator's + `sql_like_match` walks characters and treats only `%` and `_` specially — it + gets all ten matrix patterns right. +- **Why it matters:** `.` and `(` are ordinary in the data people filter — + file names (`LIKE '%.csv'` matches `xcsv`), versions, email addresses, + `'price (usd)%'`. +- **Found:** 2026-09-19, pinning LIKE for R13 slice 4. + --- ## Deferred / won't fix (intentional) diff --git a/tests/comparison/corpus/02_where.toml b/tests/comparison/corpus/02_where.toml index b62be2c1..9ea2297f 100644 --- a/tests/comparison/corpus/02_where.toml +++ b/tests/comparison/corpus/02_where.toml @@ -297,3 +297,29 @@ sql = "SELECT id, name FROM big_ids WHERE id IN (9007199254740993, 12) ORDER BY # The control for the case above. Already AGREE: R13 slice 4 part 1 delegated # IN to the value evaluator, which fixed it for IN and BETWEEN without anyone # noticing - no case held an integer this large. + +# --- P55: LIKE patterns were compiled as regexes, unescaped --- + +[[case]] +id = "where_like_dot_is_literal" +data = "like_patterns.csv" +sql = "SELECT id, s FROM like_patterns WHERE s LIKE 'a.c' ORDER BY id" +expect = "DIFFER" +# P55. WHERE turned `%` into `.*` and `_` into `.` and passed every other +# character to the regex engine as-is, so `.` matched any character: `abc` +# came back alongside `a.c`. + +[[case]] +id = "where_like_bracket_is_literal" +data = "like_patterns.csv" +sql = "SELECT id, s FROM like_patterns WHERE s LIKE '[a]bc' ORDER BY id" +expect = "DIFFER" +# P55. `[a]` was a regex character class: matched `abc`, missed `[a]bc` itself. + +[[case]] +id = "where_like_paren_is_literal" +data = "like_patterns.csv" +sql = "SELECT id, s FROM like_patterns WHERE s LIKE 'a(b%' ORDER BY id" +expect = "GAP" +# P55. `(` opened a regex group, so the query failed: "Invalid LIKE pattern +# 'a(b%': regex parse error ... unclosed group". diff --git a/tests/evaluator_matrix_tests.rs b/tests/evaluator_matrix_tests.rs index eecd4fc6..041ebced 100644 --- a/tests/evaluator_matrix_tests.rs +++ b/tests/evaluator_matrix_tests.rs @@ -231,6 +231,97 @@ const KNOWN_OPERANDS: &[Known] = &[ }, ]; +/// LIKE (R13 slice 4). WHERE and the value evaluator each have their own +/// matcher - WHERE a regex built from the pattern, the value evaluator a +/// character walk - so these rows aim at what separates the two: pattern +/// characters that mean something to a regex, and text across a newline. +/// +/// | row | s | +/// |-----|-----------------| +/// | 0 | 'abc' | +/// | 1 | 'a.c' | +/// | 2 | 'a(bc' | +/// | 3 | 'line1\nline2' | +/// | 4 | NULL | +/// | 5 | 'ABC' | +/// | 6 | '[a]bc' | +fn like_table() -> DataTable { + let mut t = DataTable::new("t"); + t.add_column(DataColumn::new("s")); + for s in [ + Some("abc"), + Some("a.c"), + Some("a(bc"), + Some("line1\nline2"), + None, + Some("ABC"), + Some("[a]bc"), + ] { + let v = s.map_or(DataValue::Null, |s| DataValue::String(s.to_string())); + t.add_row(DataRow::new(vec![v])).unwrap(); + } + t +} + +/// Generated by DuckDB from the same table (`s VARCHAR`), as for `EXPECTED`. +const EXPECTED_LIKE: &[(&str, &str)] = &[ + // Characters that are syntax to a regex are literal in a LIKE pattern. + ("s LIKE 'a.c'", "FTFFUFF"), + ("s LIKE 'a(b%'", "FFTFUFF"), + ("s LIKE '[a]bc'", "FFFFUFT"), + // `%` and `_` are the only wildcards, and `%` spans a newline. + ("s LIKE 'a_c'", "TTFFUFF"), + ("s LIKE '%'", "TTTTUTT"), + ("s LIKE 'line1%'", "FFFTUFF"), + ("s LIKE '%2'", "FFFTUFF"), + // Anchored at both ends, and case-sensitive outside --case-insensitive. + ("s LIKE 'abc'", "TFFFUFF"), + ("s LIKE 'A%'", "FFFFUTF"), + ("s LIKE ''", "FFFFUFF"), +]; + +const LIKE_AS_REGEX: &str = "P55: WHERE compiles a LIKE pattern to a regex without escaping it, \ + so regex syntax in the pattern is live and `.` does not cross a newline"; + +const KNOWN_LIKE: &[Known] = &[ + Known { + evaluator: Evaluator::Where, + predicate: "s LIKE 'a.c'", + observed: "TTFFUFF", + why: LIKE_AS_REGEX, + }, + Known { + evaluator: Evaluator::Where, + predicate: "s LIKE 'a(b%'", + observed: "EEEEUEE", + why: LIKE_AS_REGEX, + }, + Known { + evaluator: Evaluator::Where, + predicate: "s LIKE '[a]bc'", + observed: "TFFFUFF", + why: LIKE_AS_REGEX, + }, + Known { + evaluator: Evaluator::Where, + predicate: "s LIKE '%'", + observed: "TTTFUTT", + why: LIKE_AS_REGEX, + }, + Known { + evaluator: Evaluator::Where, + predicate: "s LIKE 'line1%'", + observed: "FFFFUFF", + why: LIKE_AS_REGEX, + }, + Known { + evaluator: Evaluator::Where, + predicate: "s LIKE '%2'", + observed: "FFFFUFF", + why: LIKE_AS_REGEX, + }, +]; + fn trilean_char(t: Trilean) -> char { match t { Trilean::True => 'T', @@ -297,6 +388,11 @@ fn every_operand_reading_through_both_evaluators() { check_matrix(&operand_table(), EXPECTED_OPERANDS, KNOWN_OPERANDS, false); } +#[test] +fn every_like_pattern_through_both_evaluators() { + check_matrix(&like_table(), EXPECTED_LIKE, KNOWN_LIKE, false); +} + fn check_matrix( t: &DataTable, expected_answers: &[(&str, &str)], From 7bdc85a17a44a951188f0fdb1111d6ea5531e925 Mon Sep 17 00:00:00 2001 From: TimelordUK Date: Sat, 19 Sep 2026 10:22:51 +0100 Subject: [PATCH 3/8] fix(r13): LIKE matcher without exponential backtracking (R13 slice 4) The value evaluator's LIKE matched recursively, trying every split for every `%`. One value against `'%a%a%a%a%a%a%a%b'`: 60 characters took 7 s, 80 characters over a minute. It already serves SELECT, HAVING and CASE, and WHERE is about to delegate LIKE to it - where today's regex, whatever its other faults (P55), never backtracks like this. Replaced with the iterative wildcard algorithm: on a failed literal after a `%`, only the most recent `%` absorbs one more character. Sufficient because a later `%` can take whatever an earlier one would have, so the worst case is O(text x pattern). Semantics unchanged: `%` and `_` the only wildcards, `%` spans newlines, `_` is one char. 28 unit cases plus a 2,000-character pathological one with a 1 s budget. Via the CLI the 80-character case went from >60 s to 57 ms (process start). Matrix green (all four sets), parity report byte-identical. Co-Authored-By: Claude Opus 5 (1M context) --- src/data/arithmetic_evaluator.rs | 157 ++++++++++++++++++++----------- 1 file changed, 103 insertions(+), 54 deletions(-) diff --git a/src/data/arithmetic_evaluator.rs b/src/data/arithmetic_evaluator.rs index 43791c78..a8791770 100644 --- a/src/data/arithmetic_evaluator.rs +++ b/src/data/arithmetic_evaluator.rs @@ -378,7 +378,7 @@ impl<'a> ArithmeticEvaluator<'a> { text = text.to_lowercase(); pattern = pattern.to_lowercase(); } - let matches = self.sql_like_match(&text, &pattern); + let matches = like_match(&text, &pattern); Ok(DataValue::Boolean(matches)) } _ => Err(anyhow!("Unsupported arithmetic operator: {}", op)), @@ -485,59 +485,6 @@ impl<'a> ArithmeticEvaluator<'a> { } } - /// SQL LIKE pattern matching - /// Supports % (any chars) and _ (single char) - fn sql_like_match(&self, text: &str, pattern: &str) -> bool { - let pattern_chars: Vec = pattern.chars().collect(); - let text_chars: Vec = text.chars().collect(); - - self.like_match_recursive(&text_chars, 0, &pattern_chars, 0) - } - - /// Recursive helper for LIKE matching - fn like_match_recursive( - &self, - text: &[char], - text_pos: usize, - pattern: &[char], - pattern_pos: usize, - ) -> bool { - // If we've consumed both text and pattern, it's a match - if pattern_pos >= pattern.len() { - return text_pos >= text.len(); - } - - // Handle % wildcard (matches zero or more characters) - if pattern[pattern_pos] == '%' { - // Try matching zero characters (skip the %) - if self.like_match_recursive(text, text_pos, pattern, pattern_pos + 1) { - return true; - } - // Try matching one or more characters - if text_pos < text.len() { - return self.like_match_recursive(text, text_pos + 1, pattern, pattern_pos); - } - return false; - } - - // If text is consumed but pattern isn't, no match - if text_pos >= text.len() { - return false; - } - - // Handle _ wildcard (matches exactly one character) - if pattern[pattern_pos] == '_' { - return self.like_match_recursive(text, text_pos + 1, pattern, pattern_pos + 1); - } - - // Handle literal character match - if text[text_pos] == pattern[pattern_pos] { - return self.like_match_recursive(text, text_pos + 1, pattern, pattern_pos + 1); - } - - false - } - /// Evaluate a function call fn evaluate_function_with_distinct( &mut self, @@ -1628,6 +1575,49 @@ fn compare_trilean( Trilean::from_bool(compare_with_op(left, right, op, case_insensitive)) } +/// SQL `LIKE`: `%` matches any run of characters (including none, and across +/// newlines), `_` exactly one character, and every other character only +/// itself - nothing in a pattern is regex syntax (P55). +/// +/// Iterative, backtracking only to the most recent `%`: when a literal fails +/// after a `%`, the `%` absorbs one more character and matching resumes from +/// there. That is enough because a later `%` can always absorb whatever an +/// earlier one would have, so earlier choices never need revisiting - which +/// keeps the worst case at O(text × pattern). The recursive matcher this +/// replaced tried every split for every `%`: one 60-character value against +/// `'%a%a%a%a%a%a%a%b'` took 7 s, and 80 characters over a minute. +fn like_match(text: &str, pattern: &str) -> bool { + let text: Vec = text.chars().collect(); + let pattern: Vec = pattern.chars().collect(); + let (mut t, mut p) = (0, 0); + // Pattern position just after the most recent `%`, and the text position + // that `%` has absorbed up to. + let mut resume: Option<(usize, usize)> = None; + + while t < text.len() { + match pattern.get(p) { + Some(&'%') => { + p += 1; + resume = Some((p, t)); + } + Some(&c) if c == '_' || c == text[t] => { + t += 1; + p += 1; + } + _ => match resume { + Some((after_percent, absorbed)) => { + p = after_percent; + t = absorbed + 1; + resume = Some((after_percent, t)); + } + None => return false, + }, + } + } + // Text used up: only trailing `%`s may remain. + pattern[p..].iter().all(|&c| c == '%') +} + /// The truth value of an operand of AND / OR / NOT. A value with no truth value /// (a string, a date) is an error, as it was before three-valued logic. fn truth_of(value: &DataValue) -> Result { @@ -1779,6 +1769,65 @@ mod tests { assert_eq!(result, DataValue::Float(10.0 / 3.0)); } + #[test] + fn like_match_semantics() { + for (text, pattern, expected) in [ + ("abc", "abc", true), + ("abc", "a%", true), + ("abc", "%c", true), + ("abc", "%b%", true), + ("abc", "a_c", true), + ("abc", "a_", false), + ("abc", "", false), + ("", "", true), + ("", "%", true), + ("", "_", false), + ("abc", "%%%", true), + ("abcd", "_%_", true), + ("a", "_%_", false), + // Only % and _ are special. + ("a.c", "a.c", true), + ("abc", "a.c", false), + ("[a]bc", "[a]bc", true), + ("abc", "[a]bc", false), + ("a(bc", "a(b%", true), + // A literal % in the data is matched by the wildcard. + ("50%", "50%", true), + // % spans a newline. + ("line1\nline2", "%2", true), + ("line1\nline2", "line1%", true), + // Backtracking to the most recent % is enough. + ("aab", "%a%b", true), + ("ab", "%a%a", false), + ("mississippi", "%iss%pi", true), + ("mississippi", "m%ss%ss%", true), + ("mississippi", "m%ss%ss%ss%", false), + // _ is one character, not one byte. + ("Ê", "_", true), + ("Êa", "_a", true), + ] { + assert_eq!( + like_match(text, pattern), + expected, + "{text:?} LIKE {pattern:?}" + ); + } + } + + #[test] + fn like_match_has_no_exponential_backtracking() { + // The recursive matcher this replaced took 7 s for 60 characters and + // over a minute for 80, on one value. Here: 2,000 characters. + let text = "a".repeat(2000); + let start = std::time::Instant::now(); + assert!(!like_match(&text, "%a%a%a%a%a%a%a%b")); + assert!( + start.elapsed() < std::time::Duration::from_secs(1), + "took {:?}", + start.elapsed() + ); + } + #[test] fn test_division_by_zero() { let table = create_test_table(); From 0b7e0e563da5b04e15a19f82169d8f0939cf7d08 Mon Sep 17 00:00:00 2001 From: TimelordUK Date: Sat, 19 Sep 2026 10:23:28 +0100 Subject: [PATCH 4/8] test(r13): pin LIKE over a number as the decided behaviour; record D2 (R13 slice 4) DuckDB rejects LIKE on a number, so there is no reference answer. Decision (recorded as D2 in docs/SQL_PARITY.md): a number is matched as the text sql-cli displays for it - what the value evaluator already does in SELECT, HAVING and CASE. `5.0` displays as `5`, so it matches '5%' but not '5.0'. A fifth matrix set, over the operand table, states that decision as the expected answer. WHERE answers FALSE for every non-text operand today: four entries recorded, to flip when LIKE delegates. Co-Authored-By: Claude Opus 5 (1M context) --- docs/SQL_PARITY.md | 19 ++++++++++++ tests/evaluator_matrix_tests.rs | 54 +++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+) diff --git a/docs/SQL_PARITY.md b/docs/SQL_PARITY.md index e24e6f88..44c2601e 100644 --- a/docs/SQL_PARITY.md +++ b/docs/SQL_PARITY.md @@ -2550,6 +2550,25 @@ out of date. for the current "vanilla SQL consistency" effort. Revisit if/when that scope layer is pursued. Not a bug; do not let the corpus case churn — keep `expect = "GAP"`. +### D2 — `LIKE` over a number matches the number's displayed text +- **Status:** âšĒ DELIBERATE (decided 2026-09-19, R13 slice 4). +- **Corpus:** none — DuckDB errors, so there is no reference answer to agree + with. Pinned in the evaluator matrix (`EXPECTED_NUMBER_LIKE`), whose expected + answers for these rows are this decision, not DuckDB output. +- **Behaviour:** a non-text operand of `LIKE` is matched as the text sql-cli + displays for it: `51.5 LIKE '5%'` is TRUE; `5.0` displays as `5`, so it + matches `'5%'` but not `'5.0'`. DuckDB rejects the query (no + `LIKE(DOUBLE, VARCHAR)`) and asks for an explicit cast. +- **Rationale:** the tool is for exploring data, often loosely typed CSVs where + an id or code column is inferred as a number; "does this id start with + `9007`" should just work. Matching the displayed text means the pattern + matches what the user can see. The value evaluator (SELECT, HAVING, CASE) + already behaved this way; before R13 slice 4, WHERE answered FALSE for every + number, so the same predicate disagreed with itself between clauses. +- **Considered:** erroring like DuckDB (strict, but breaks existing SELECTs that + rely on it), and FALSE everywhere (a silent non-match is the least useful + answer). + _When we consciously diverge from the reference engine on results (rather than simply not implementing a feature), record it here with the rationale so the DIFFER is understood, not mistaken for a bug._ diff --git a/tests/evaluator_matrix_tests.rs b/tests/evaluator_matrix_tests.rs index 041ebced..bdd4439e 100644 --- a/tests/evaluator_matrix_tests.rs +++ b/tests/evaluator_matrix_tests.rs @@ -280,6 +280,50 @@ const EXPECTED_LIKE: &[(&str, &str)] = &[ ("s LIKE ''", "FFFFUFF"), ]; +/// LIKE over a number, on the operand table. DuckDB refuses these outright +/// (no `LIKE(DOUBLE, VARCHAR)`), so the expected answers are **not** DuckDB's: +/// they are the deliberate choice recorded as D2 in docs/SQL_PARITY.md - a +/// number is matched as the text the tool displays for it. `5.0` displays as +/// `5`, so it matches `'5%'` but not `'5.0'`. +const EXPECTED_NUMBER_LIKE: &[(&str, &str)] = &[ + ("n LIKE '5%'", "TUF"), + ("n LIKE '5.0'", "FUF"), + ("n LIKE '7._'", "FUT"), + ("big LIKE '9007%'", "TTU"), + ("big LIKE '%993'", "TFU"), +]; + +const NUMBER_LIKE_NEVER_MATCHES: &str = + "D2: WHERE's LIKE answers FALSE for any non-text operand; the decision is to \ + match the displayed text, as the value evaluator already does"; + +const KNOWN_NUMBER_LIKE: &[Known] = &[ + Known { + evaluator: Evaluator::Where, + predicate: "n LIKE '5%'", + observed: "FUF", + why: NUMBER_LIKE_NEVER_MATCHES, + }, + Known { + evaluator: Evaluator::Where, + predicate: "n LIKE '7._'", + observed: "FUF", + why: NUMBER_LIKE_NEVER_MATCHES, + }, + Known { + evaluator: Evaluator::Where, + predicate: "big LIKE '9007%'", + observed: "FFU", + why: NUMBER_LIKE_NEVER_MATCHES, + }, + Known { + evaluator: Evaluator::Where, + predicate: "big LIKE '%993'", + observed: "FFU", + why: NUMBER_LIKE_NEVER_MATCHES, + }, +]; + const LIKE_AS_REGEX: &str = "P55: WHERE compiles a LIKE pattern to a regex without escaping it, \ so regex syntax in the pattern is live and `.` does not cross a newline"; @@ -393,6 +437,16 @@ fn every_like_pattern_through_both_evaluators() { check_matrix(&like_table(), EXPECTED_LIKE, KNOWN_LIKE, false); } +#[test] +fn like_over_a_number_through_both_evaluators() { + check_matrix( + &operand_table(), + EXPECTED_NUMBER_LIKE, + KNOWN_NUMBER_LIKE, + false, + ); +} + fn check_matrix( t: &DataTable, expected_answers: &[(&str, &str)], From 68aea76b4bb982d5620d3729778a44580da544c0 Mon Sep 17 00:00:00 2001 From: TimelordUK Date: Sat, 19 Sep 2026 10:32:21 +0100 Subject: [PATCH 5/8] refactor(r13): WHERE's LIKE delegates; one `sql_like` for both evaluators (R13 slice 4) LIKE joins the comparisons and NULL tests in `delegates_to_value_evaluator`. The whole of LIKE - NULL handling, the text a non-text operand is matched as (D2), case mode and matching - is now one function, `sql_like`, which the value evaluator's arm and WHERE's remaining legacy arm (a method call on the left) both call. WHERE's regex construction, cached and uncached, is gone. Closes P55: the six LIKE-matrix entries and four number-LIKE entries went FIXED and nothing else moved; the three corpus cases moved to AGREE, parity 176 -> 179 AGREE over 209. WHERE LIKE over trades_100000.csv unchanged at load time without the regex cache. Leaves `RecursiveWhereEvaluator::context` unread (it only carried the regex cache); the next commit removes that plumbing. Co-Authored-By: Claude Opus 5 (1M context) --- docs/SQL_PARITY.md | 19 +++++-- src/data/arithmetic_evaluator.rs | 68 ++++++++++++----------- src/data/recursive_where_evaluator.rs | 57 +++++-------------- tests/comparison/corpus/02_where.toml | 5 +- tests/evaluator_matrix_tests.rs | 80 +++------------------------ 5 files changed, 73 insertions(+), 156 deletions(-) diff --git a/docs/SQL_PARITY.md b/docs/SQL_PARITY.md index 44c2601e..dd7e3440 100644 --- a/docs/SQL_PARITY.md +++ b/docs/SQL_PARITY.md @@ -2511,13 +2511,20 @@ out of date. - **Found:** 2026-09-18, pinning the operand readers for R13 slice 4. ### P55 — A `LIKE` pattern in `WHERE` is read as a regex -- **Status:** 🔴 OPEN — silent wrong rows, or a failed query. - [R13](ENGINE_REFACTORING.md#r13) slice 4 (LIKE delegates) closes it. +- **Status:** ✅ FIXED 2026-09-19 by [R13](ENGINE_REFACTORING.md#r13) slice 4: + WHERE's LIKE delegates to the value evaluator, and both now call one + function, `sql_like`, which treats only `%` and `_` as special. The regex + path is gone. 176 → **179 AGREE**; the six matrix entries went FIXED and + nothing else moved. - **Corpus:** `02_where.toml :: where_like_dot_is_literal`, - `where_like_bracket_is_literal` (`expect = "DIFFER"`), - `where_like_paren_is_literal` (`expect = "GAP"`). The newline shapes are - pinned in the evaluator matrix only (`EXPECTED_LIKE`); a CSV value with an - embedded newline would test the loader as much as LIKE. + `where_like_bracket_is_literal`, `where_like_paren_is_literal` (all AGREE). + The newline shapes are pinned in the evaluator matrix only (`EXPECTED_LIKE`); + a CSV value with an embedded newline would test the loader as much as LIKE. +- **Found on the way:** the value evaluator's own matcher, correct on all ten + patterns, was exponential — one 80-character value against + `'%a%a%a%a%a%a%a%b'` ran for over a minute. Replaced with an iterative + matcher before WHERE delegated to it, so the fix did not trade wrong rows for + a hang. - **Observed:** `WHERE s LIKE 'a.c'` returns `abc` as well as `a.c`; `LIKE '[a]bc'` returns `abc` and misses `[a]bc`; `LIKE 'a(b%'` fails the query with "regex parse error â€Ļ unclosed group"; `LIKE '%'` misses a value diff --git a/src/data/arithmetic_evaluator.rs b/src/data/arithmetic_evaluator.rs index a8791770..c05b0c32 100644 --- a/src/data/arithmetic_evaluator.rs +++ b/src/data/arithmetic_evaluator.rs @@ -367,20 +367,7 @@ impl<'a> ArithmeticEvaluator<'a> { "IS NOT NULL" => Ok(DataValue::Boolean(!matches!(left_val, DataValue::Null))), "AND" => Ok(truth_of(&left_val)?.and(truth_of(&right_val)?).to_value()), "OR" => Ok(truth_of(&left_val)?.or(truth_of(&right_val)?).to_value()), - // LIKE operator - SQL pattern matching - "LIKE" => { - if matches!(left_val, DataValue::Null) || matches!(right_val, DataValue::Null) { - return Ok(DataValue::Null); - } - let mut text = self.value_to_string(&left_val); - let mut pattern = self.value_to_string(&right_val); - if self.case_insensitive { - text = text.to_lowercase(); - pattern = pattern.to_lowercase(); - } - let matches = like_match(&text, &pattern); - Ok(DataValue::Boolean(matches)) - } + "LIKE" => Ok(sql_like(&left_val, &right_val, self.case_insensitive).to_value()), _ => Err(anyhow!("Unsupported arithmetic operator: {}", op)), } } @@ -467,24 +454,6 @@ impl<'a> ArithmeticEvaluator<'a> { } } - /// Convert DataValue to string for pattern matching - fn value_to_string(&self, value: &DataValue) -> String { - match value { - DataValue::String(s) => s.clone(), - DataValue::InternedString(s) => s.to_string(), - DataValue::Integer(i) => i.to_string(), - DataValue::Float(f) => f.to_string(), - DataValue::Boolean(b) => b.to_string(), - DataValue::DateTime(dt) => dt.to_string(), - DataValue::Vector(v) => { - // Format as "[x,y,z]" - let components: Vec = v.iter().map(|f| f.to_string()).collect(); - format!("[{}]", components.join(",")) - } - DataValue::Null => String::new(), - } - } - /// Evaluate a function call fn evaluate_function_with_distinct( &mut self, @@ -1575,6 +1544,41 @@ fn compare_trilean( Trilean::from_bool(compare_with_op(left, right, op, case_insensitive)) } +/// `value LIKE pattern`, three-valued: the one implementation of LIKE, used by +/// the value evaluator and by the WHERE evaluator's remaining legacy arm. +/// NULL on either side is UNKNOWN. A non-text operand is matched as the text +/// sql-cli displays for it (D2 in docs/SQL_PARITY.md - DuckDB rejects LIKE on +/// a number instead). +pub(crate) fn sql_like(value: &DataValue, pattern: &DataValue, case_insensitive: bool) -> Trilean { + if matches!(value, DataValue::Null) || matches!(pattern, DataValue::Null) { + return Trilean::Unknown; + } + let (text, pattern) = (like_text(value), like_text(pattern)); + Trilean::from_bool(if case_insensitive { + like_match(&text.to_lowercase(), &pattern.to_lowercase()) + } else { + like_match(&text, &pattern) + }) +} + +/// The text a LIKE operand is matched as: what sql-cli displays for it. +fn like_text(value: &DataValue) -> String { + match value { + DataValue::String(s) => s.clone(), + DataValue::InternedString(s) => s.to_string(), + DataValue::Integer(i) => i.to_string(), + DataValue::Float(f) => f.to_string(), + DataValue::Boolean(b) => b.to_string(), + DataValue::DateTime(dt) => dt.to_string(), + DataValue::Vector(v) => { + // Format as "[x,y,z]" + let components: Vec = v.iter().map(|f| f.to_string()).collect(); + format!("[{}]", components.join(",")) + } + DataValue::Null => String::new(), + } +} + /// SQL `LIKE`: `%` matches any run of characters (including none, and across /// newlines), `_` exactly one character, and every other character only /// itself - nothing in a pattern is regex syntax (P55). diff --git a/src/data/recursive_where_evaluator.rs b/src/data/recursive_where_evaluator.rs index 04babf0c..8d84a806 100644 --- a/src/data/recursive_where_evaluator.rs +++ b/src/data/recursive_where_evaluator.rs @@ -1,4 +1,4 @@ -use crate::data::arithmetic_evaluator::ArithmeticEvaluator; +use crate::data::arithmetic_evaluator::{sql_like, ArithmeticEvaluator}; use crate::data::datatable::{DataTable, DataValue}; use crate::data::evaluation_context::EvaluationContext; use crate::data::query_engine::ExecutionContext; @@ -606,11 +606,11 @@ impl<'a, 'ctx, 'exec> RecursiveWhereEvaluator<'a, 'ctx, 'exec> { return self.evaluate_value_as_predicate(expr, row_index); } - // Comparisons and NULL tests are the value evaluator's (R13 slice 4): - // its operand readers and three-valued `compare_trilean` are the one - // implementation. Two shapes stay below for now - a legacy method call - // on the left (`Length()`, `IndexOf()`, `Trim*()`), whose registry - // twins are slice 5's to check, and LIKE. + // Comparisons, NULL tests and LIKE are the value evaluator's (R13 + // slice 4): its operand readers, three-valued `compare_trilean` and + // `sql_like` are the one implementation. One shape stays below for + // now - a legacy method call on the left (`Length()`, `IndexOf()`, + // `Trim*()`), whose registry twins are slice 5's to check. if delegates_to_value_evaluator(&op_upper) && !matches!(left, SqlExpression::MethodCall { .. }) { @@ -690,42 +690,13 @@ impl<'a, 'ctx, 'exec> RecursiveWhereEvaluator<'a, 'ctx, 'exec> { // Handle special operators that aren't standard comparisons match op_upper.as_str() { // LIKE operator - handle specially - "LIKE" => { - let table_value = cell_value.unwrap_or(DataValue::Null); - let pattern = match compare_value { - DataValue::String(s) => s, - DataValue::InternedString(s) => s.to_string(), - // A NULL pattern makes the whole predicate UNKNOWN; any - // other non-string pattern is simply not a match. - DataValue::Null => return Ok(Trilean::Unknown), - _ => return Ok(Trilean::False), - }; - - let text = match &table_value { - DataValue::String(s) => s.as_str(), - DataValue::InternedString(s) => s.as_str(), - // Likewise on the value side: NULL LIKE '...' is UNKNOWN, - // which matters under NOT — see P19. - DataValue::Null => return Ok(Trilean::Unknown), - _ => return Ok(Trilean::False), - }; - - // Use cached regex if context is available, otherwise compile fresh - if let Some(ctx) = &mut self.context { - let regex = ctx - .get_or_compile_like_regex(&pattern) - .map_err(|e| anyhow::anyhow!("{}", e))?; - Ok(Trilean::from_bool(regex.is_match(text))) - } else { - // Fallback to compiling regex each time (old behavior) - let regex_pattern = pattern.replace('%', ".*").replace('_', "."); - let regex = regex::RegexBuilder::new(&format!("^{regex_pattern}$")) - .case_insensitive(self.case_insensitive) - .build() - .map_err(|e| anyhow::anyhow!("Invalid LIKE pattern: {}", e))?; - Ok(Trilean::from_bool(regex.is_match(text))) - } - } + // Reached only with a legacy method call on the left; every other + // LIKE delegated above. Same function, so the same rules. + "LIKE" => Ok(sql_like( + &cell_value.unwrap_or(DataValue::Null), + &compare_value, + self.case_insensitive, + )), // IS NULL / IS NOT NULL "IS NULL" => Ok(Trilean::from_bool( @@ -1159,7 +1130,7 @@ fn reject_unlifted_window(expr: &SqlExpression) -> Result<()> { fn delegates_to_value_evaluator(op_upper: &str) -> bool { matches!( op_upper, - "=" | "!=" | "<>" | "<" | "<=" | ">" | ">=" | "IS NULL" | "IS NOT NULL" + "=" | "!=" | "<>" | "<" | "<=" | ">" | ">=" | "IS NULL" | "IS NOT NULL" | "LIKE" ) } diff --git a/tests/comparison/corpus/02_where.toml b/tests/comparison/corpus/02_where.toml index 9ea2297f..5d191ee6 100644 --- a/tests/comparison/corpus/02_where.toml +++ b/tests/comparison/corpus/02_where.toml @@ -299,12 +299,13 @@ sql = "SELECT id, name FROM big_ids WHERE id IN (9007199254740993, 12) ORDER BY # noticing - no case held an integer this large. # --- P55: LIKE patterns were compiled as regexes, unescaped --- +# Fixed by R13 slice 4: WHERE's LIKE delegates to `sql_like`, which treats +# only % and _ as special. The comments below describe the old behaviour. [[case]] id = "where_like_dot_is_literal" data = "like_patterns.csv" sql = "SELECT id, s FROM like_patterns WHERE s LIKE 'a.c' ORDER BY id" -expect = "DIFFER" # P55. WHERE turned `%` into `.*` and `_` into `.` and passed every other # character to the regex engine as-is, so `.` matched any character: `abc` # came back alongside `a.c`. @@ -313,13 +314,11 @@ expect = "DIFFER" id = "where_like_bracket_is_literal" data = "like_patterns.csv" sql = "SELECT id, s FROM like_patterns WHERE s LIKE '[a]bc' ORDER BY id" -expect = "DIFFER" # P55. `[a]` was a regex character class: matched `abc`, missed `[a]bc` itself. [[case]] id = "where_like_paren_is_literal" data = "like_patterns.csv" sql = "SELECT id, s FROM like_patterns WHERE s LIKE 'a(b%' ORDER BY id" -expect = "GAP" # P55. `(` opened a regex group, so the query failed: "Invalid LIKE pattern # 'a(b%': regex parse error ... unclosed group". diff --git a/tests/evaluator_matrix_tests.rs b/tests/evaluator_matrix_tests.rs index bdd4439e..221ac617 100644 --- a/tests/evaluator_matrix_tests.rs +++ b/tests/evaluator_matrix_tests.rs @@ -293,78 +293,14 @@ const EXPECTED_NUMBER_LIKE: &[(&str, &str)] = &[ ("big LIKE '%993'", "TFU"), ]; -const NUMBER_LIKE_NEVER_MATCHES: &str = - "D2: WHERE's LIKE answers FALSE for any non-text operand; the decision is to \ - match the displayed text, as the value evaluator already does"; - -const KNOWN_NUMBER_LIKE: &[Known] = &[ - Known { - evaluator: Evaluator::Where, - predicate: "n LIKE '5%'", - observed: "FUF", - why: NUMBER_LIKE_NEVER_MATCHES, - }, - Known { - evaluator: Evaluator::Where, - predicate: "n LIKE '7._'", - observed: "FUF", - why: NUMBER_LIKE_NEVER_MATCHES, - }, - Known { - evaluator: Evaluator::Where, - predicate: "big LIKE '9007%'", - observed: "FFU", - why: NUMBER_LIKE_NEVER_MATCHES, - }, - Known { - evaluator: Evaluator::Where, - predicate: "big LIKE '%993'", - observed: "FFU", - why: NUMBER_LIKE_NEVER_MATCHES, - }, -]; - -const LIKE_AS_REGEX: &str = "P55: WHERE compiles a LIKE pattern to a regex without escaping it, \ - so regex syntax in the pattern is live and `.` does not cross a newline"; - -const KNOWN_LIKE: &[Known] = &[ - Known { - evaluator: Evaluator::Where, - predicate: "s LIKE 'a.c'", - observed: "TTFFUFF", - why: LIKE_AS_REGEX, - }, - Known { - evaluator: Evaluator::Where, - predicate: "s LIKE 'a(b%'", - observed: "EEEEUEE", - why: LIKE_AS_REGEX, - }, - Known { - evaluator: Evaluator::Where, - predicate: "s LIKE '[a]bc'", - observed: "TFFFUFF", - why: LIKE_AS_REGEX, - }, - Known { - evaluator: Evaluator::Where, - predicate: "s LIKE '%'", - observed: "TTTFUTT", - why: LIKE_AS_REGEX, - }, - Known { - evaluator: Evaluator::Where, - predicate: "s LIKE 'line1%'", - observed: "FFFFUFF", - why: LIKE_AS_REGEX, - }, - Known { - evaluator: Evaluator::Where, - predicate: "s LIKE '%2'", - observed: "FFFFUFF", - why: LIKE_AS_REGEX, - }, -]; +// R13 slice 4 recorded four WHERE entries here - WHERE's LIKE answered FALSE +// for every non-text operand. All four went FIXED when LIKE delegated. +const KNOWN_NUMBER_LIKE: &[Known] = &[]; + +// R13 slice 4 recorded six WHERE entries here (P55: the pattern compiled to a +// regex, unescaped - `.`, `[a]` and `(` were live, and `%` stopped at a +// newline). All six went FIXED when LIKE delegated to `sql_like`. +const KNOWN_LIKE: &[Known] = &[]; fn trilean_char(t: Trilean) -> char { match t { From 313c2538ea6e93c3a476f6c3ee468ee2849c2716 Mon Sep 17 00:00:00 2001 From: TimelordUK Date: Sat, 19 Sep 2026 10:35:57 +0100 Subject: [PATCH 6/8] refactor(r13): remove the LIKE regex cache and its plumbing (R13 slice 4) With LIKE delegated, `EvaluationContext` carried a regex cache nothing read and a case-insensitive flag already passed in directly. Removed: - `data::evaluation_context` (the module, 69 lines); - `RecursiveWhereEvaluator`'s `context` field and `'ctx` lifetime, and the `with_context` / `with_both_contexts` constructors that took one; - `with_config`, which had no callers and an ignored date-notation argument (the same dead plumbing slice 3 removed from the value evaluator); - the query engine's cache-statistics logging, now always zero. The query engine builds the WHERE evaluator with `with_exec_context` or `with_case_insensitive`. No-op: parity report byte-identical, 830 lib + 501 integration tests pass. 147 lines out, 13 in. Co-Authored-By: Claude Opus 5 (1M context) --- src/data/evaluation_context.rs | 69 ------------------------- src/data/mod.rs | 1 - src/data/query_engine.rs | 17 ++----- src/data/recursive_where_evaluator.rs | 73 ++++----------------------- 4 files changed, 13 insertions(+), 147 deletions(-) delete mode 100644 src/data/evaluation_context.rs diff --git a/src/data/evaluation_context.rs b/src/data/evaluation_context.rs deleted file mode 100644 index 15a7d7ef..00000000 --- a/src/data/evaluation_context.rs +++ /dev/null @@ -1,69 +0,0 @@ -use regex::Regex; -use std::collections::HashMap; -use std::sync::Arc; - -/// Context for query evaluation that can cache expensive computations -/// like compiled regexes across multiple row evaluations -#[derive(Debug, Clone)] -pub struct EvaluationContext { - /// Cache of compiled regexes for LIKE patterns - /// Key is the SQL LIKE pattern, value is the compiled regex - regex_cache: HashMap>, - - /// Case insensitive mode - case_insensitive: bool, - - /// Statistics for debugging - regex_compilations: usize, - regex_cache_hits: usize, -} - -impl EvaluationContext { - pub fn new(case_insensitive: bool) -> Self { - Self { - regex_cache: HashMap::new(), - case_insensitive, - regex_compilations: 0, - regex_cache_hits: 0, - } - } - - /// Get or compile a regex for a SQL LIKE pattern - pub fn get_or_compile_like_regex(&mut self, like_pattern: &str) -> Result, String> { - // Check cache first - if let Some(regex) = self.regex_cache.get(like_pattern) { - self.regex_cache_hits += 1; - return Ok(Arc::clone(regex)); - } - - // Convert SQL LIKE pattern to regex pattern - let regex_pattern = like_pattern.replace('%', ".*").replace('_', "."); - - // Compile the regex - let regex = regex::RegexBuilder::new(&format!("^{}$", regex_pattern)) - .case_insensitive(self.case_insensitive) - .build() - .map_err(|e| format!("Invalid LIKE pattern '{}': {}", like_pattern, e))?; - - let regex_arc = Arc::new(regex); - self.regex_cache - .insert(like_pattern.to_string(), Arc::clone(®ex_arc)); - self.regex_compilations += 1; - - Ok(regex_arc) - } - - pub fn get_stats(&self) -> (usize, usize) { - (self.regex_compilations, self.regex_cache_hits) - } - - pub fn is_case_insensitive(&self) -> bool { - self.case_insensitive - } -} - -impl Default for EvaluationContext { - fn default() -> Self { - Self::new(false) - } -} diff --git a/src/data/mod.rs b/src/data/mod.rs index d180601b..fd08e923 100644 --- a/src/data/mod.rs +++ b/src/data/mod.rs @@ -36,7 +36,6 @@ pub mod stream_loader; pub mod arithmetic_evaluator; pub mod batch_window_evaluator; // Batch evaluation for window functions pub mod column_resolution_error; // One home for the 'a.path not found' message -pub mod evaluation_context; pub mod group_by_expressions; pub mod hash_join; pub mod query_engine; diff --git a/src/data/query_engine.rs b/src/data/query_engine.rs index 5eeb34ab..a4eef90d 100644 --- a/src/data/query_engine.rs +++ b/src/data/query_engine.rs @@ -10,7 +10,6 @@ use crate::config::config::BehaviorConfig; use crate::data::arithmetic_evaluator::ArithmeticEvaluator; use crate::data::data_view::DataView; use crate::data::datatable::{DataColumn, DataRow, DataTable, DataValue}; -use crate::data::evaluation_context::EvaluationContext; use crate::data::group_by_expressions::GroupByExpressions; use crate::data::hash_join::HashJoinExecutor; use crate::data::recursive_where_evaluator::RecursiveWhereEvaluator; @@ -1624,15 +1623,13 @@ impl QueryEngine { } let filter_start = Instant::now(); - // Create an evaluation context for caching compiled regexes - let mut eval_context = EvaluationContext::new(self.case_insensitive); // Create evaluator ONCE before the loop for performance let mut evaluator = if let Some(exec_ctx) = exec_context { - // Use both contexts: exec_context for alias resolution, eval_context for regex caching - RecursiveWhereEvaluator::with_both_contexts(&table, &mut eval_context, exec_ctx) + // The execution context is for alias resolution + RecursiveWhereEvaluator::with_exec_context(&table, exec_ctx, self.case_insensitive) } else { - RecursiveWhereEvaluator::with_context(&table, &mut eval_context) + RecursiveWhereEvaluator::with_case_insensitive(&table, self.case_insensitive) }; // Filter visible rows based on WHERE clause @@ -1670,14 +1667,6 @@ impl QueryEngine { } } - // Log regex cache statistics - let (compilations, cache_hits) = eval_context.get_stats(); - if compilations > 0 || cache_hits > 0 { - debug!( - "LIKE pattern cache: {} compilations, {} cache hits", - compilations, cache_hits - ); - } visible_rows = filtered_rows; let filter_duration = filter_start.elapsed(); info!( diff --git a/src/data/recursive_where_evaluator.rs b/src/data/recursive_where_evaluator.rs index 8d84a806..4e2e2bea 100644 --- a/src/data/recursive_where_evaluator.rs +++ b/src/data/recursive_where_evaluator.rs @@ -1,6 +1,5 @@ use crate::data::arithmetic_evaluator::{sql_like, ArithmeticEvaluator}; use crate::data::datatable::{DataTable, DataValue}; -use crate::data::evaluation_context::EvaluationContext; use crate::data::query_engine::ExecutionContext; use crate::data::trilean::Trilean; use crate::data::value_comparisons::compare_with_op; @@ -12,10 +11,9 @@ use std::cell::RefCell; use tracing::debug; /// Evaluates WHERE clauses from `recursive_parser` directly against `DataTable` -pub struct RecursiveWhereEvaluator<'a, 'ctx, 'exec> { +pub struct RecursiveWhereEvaluator<'a, 'exec> { table: &'a DataTable, case_insensitive: bool, - context: Option<&'ctx mut EvaluationContext>, exec_context: Option<&'exec ExecutionContext>, /// One `ArithmeticEvaluator` for the whole evaluation, built on first use. /// Constructing one builds the function and aggregate registries, and its @@ -24,25 +22,20 @@ pub struct RecursiveWhereEvaluator<'a, 'ctx, 'exec> { arithmetic: RefCell>>, } -impl<'a, 'ctx, 'exec> RecursiveWhereEvaluator<'a, 'ctx, 'exec> { +impl<'a, 'exec> RecursiveWhereEvaluator<'a, 'exec> { #[must_use] - pub fn new(table: &'a DataTable) -> RecursiveWhereEvaluator<'a, 'static, 'static> { - RecursiveWhereEvaluator { - table, - case_insensitive: false, - context: None, - exec_context: None, - arithmetic: RefCell::new(None), - } + pub fn new(table: &'a DataTable) -> RecursiveWhereEvaluator<'a, 'static> { + Self::with_case_insensitive(table, false) } - /// Create evaluator with an evaluation context for caching - pub fn with_context(table: &'a DataTable, context: &'ctx mut EvaluationContext) -> Self { - let case_insensitive = context.is_case_insensitive(); - Self { + #[must_use] + pub fn with_case_insensitive( + table: &'a DataTable, + case_insensitive: bool, + ) -> RecursiveWhereEvaluator<'a, 'static> { + RecursiveWhereEvaluator { table, case_insensitive, - context: Some(context), exec_context: None, arithmetic: RefCell::new(None), } @@ -57,23 +50,6 @@ impl<'a, 'ctx, 'exec> RecursiveWhereEvaluator<'a, 'ctx, 'exec> { Self { table, case_insensitive, - context: None, - exec_context: Some(exec_context), - arithmetic: RefCell::new(None), - } - } - - /// Create evaluator with both execution context (for alias resolution) and evaluation context (for regex caching) - pub fn with_both_contexts( - table: &'a DataTable, - context: &'ctx mut EvaluationContext, - exec_context: &'exec ExecutionContext, - ) -> Self { - let case_insensitive = context.is_case_insensitive(); - Self { - table, - case_insensitive, - context: Some(context), exec_context: Some(exec_context), arithmetic: RefCell::new(None), } @@ -132,35 +108,6 @@ impl<'a, 'ctx, 'exec> RecursiveWhereEvaluator<'a, 'ctx, 'exec> { matrix[len1][len2] } - #[must_use] - pub fn with_case_insensitive( - table: &'a DataTable, - case_insensitive: bool, - ) -> RecursiveWhereEvaluator<'a, 'static, 'static> { - RecursiveWhereEvaluator { - table, - case_insensitive, - context: None, - exec_context: None, - arithmetic: RefCell::new(None), - } - } - - #[must_use] - pub fn with_config( - table: &'a DataTable, - case_insensitive: bool, - _date_notation: String, // No longer needed since we use centralized parse_datetime - ) -> RecursiveWhereEvaluator<'a, 'static, 'static> { - RecursiveWhereEvaluator { - table, - case_insensitive, - context: None, - exec_context: None, - arithmetic: RefCell::new(None), - } - } - /// Compare two values under SQL three-valued logic: if **either** operand /// is NULL the answer is UNKNOWN, never TRUE or FALSE. /// From da374b2f09375291b3d64a46e5e26432e484b745 Mon Sep 17 00:00:00 2001 From: TimelordUK Date: Sat, 19 Sep 2026 10:36:34 +0100 Subject: [PATCH 7/8] docs(r13): record slice 4 third part (IS NULL, LIKE; P55, D2) Co-Authored-By: Claude Opus 5 (1M context) --- docs/ENGINE_REFACTORING.md | 51 ++++++++++++++++++++++++++++++-------- docs/SQL_PARITY.md | 10 ++++---- 2 files changed, 46 insertions(+), 15 deletions(-) diff --git a/docs/ENGINE_REFACTORING.md b/docs/ENGINE_REFACTORING.md index ee59baa5..585cea34 100644 --- a/docs/ENGINE_REFACTORING.md +++ b/docs/ENGINE_REFACTORING.md @@ -474,7 +474,9 @@ feature work**, and so we can tell the difference between "this is awkward" and - **Status:** 🟡 IN PROGRESS — filed 2026-09-13 out of [P46](SQL_PARITY.md#p46); **slices 1 and 2 done 2026-09-13, slice 3 (with 3b) done 2026-09-14**, slice 4 in progress (`BETWEEN`, `IN`, `NOT IN` delegated 2026-09-15; comparisons - 2026-09-18, closing P54). + 2026-09-18, closing P54; IS NULL and LIKE 2026-09-19, closing P55). Every + WHERE *operator* now delegates; what remains of slice 4 (AND / OR / NOT / + CASE) lands after slice 5. **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 → @@ -666,14 +668,42 @@ feature work**, and so we can tell the difference between "this is awkward" and parity moved by exactly one case (P54, 175 → **176 AGREE** / 206). `evaluate_binary_op` now gets the node itself, so neither the delegation nor the P37 value-as-predicate branch clones it per row. - - *Remaining in slice 4:* `LIKE` (WHERE compiles through `EvaluationContext`'s - regex cache; the value evaluator has its own matcher, and they disagree on a - non-string operand — `5.0 LIKE '5%'` is FALSE in WHERE, TRUE in the value - evaluator, and an error in DuckDB), `IS NULL` / `IS NOT NULL` (small), and - then `AND` / `OR` / `NOT` / `CASE`, which wait on slice 5 for their - method-call children. WHERE's own readers (`number_literal_value`, the - `DATETIME` arms) are now only reached through a legacy left-side method - call or LIKE. + - *Left after part 2:* LIKE and IS NULL (part 3, below), then `AND` / `OR` / + `NOT` / `CASE`, which wait on slice 5 for their method-call children. +- **Slice 4, third part (2026-09-19) — IS NULL and LIKE.** Six commits: + - *IS NULL / IS NOT NULL delegate* — a true no-op: the parser always puts a + `Null` literal on the right, and no reader difference changes whether the + left operand is NULL. WHERE's bare `IS` / `IS NOT` arms and `==` went with + it: the parser never emits them. + - *Pin LIKE.* A fourth matrix (`EXPECTED_LIKE`, DuckDB-generated) aimed at + what separated the two matchers. WHERE built its matcher by replacing `%` + and `_` in the pattern and escaping nothing else, so it got six of ten + wrong — **P55**: `LIKE 'a.c'` matched `abc`, `'[a]bc'` was a character + class, `'a(b%'` failed the whole query, and `%` stopped at a newline. Three + corpus cases over new `data/like_patterns.csv`. + - *The value evaluator's matcher was exponential.* Right on all ten patterns, + but recursive over every split for every `%`: one 80-character value + against `'%a%a%a%a%a%a%a%b'` ran for over a minute (SELECT has had this all + along). Replaced with the iterative matcher — backtrack only to the most + recent `%`, O(text × pattern) — **before** WHERE delegated to it, so the fix + did not swap wrong rows for a hang. 28 unit cases plus a 2,000-character + pathological one on a 1 s budget. + - *LIKE over a number — a decision, D2.* DuckDB rejects it, so there is no + reference answer. Chosen: match the number's displayed text, as the value + evaluator already did (`5.0` displays as `5`). Pinned as a fifth matrix set + whose expected answers are the decision, not DuckDB output; WHERE's + "always FALSE" recorded against it. + - *Delegate.* All of LIKE is now one function, `sql_like`, called by the + value evaluator and by WHERE's one remaining legacy arm. Exactly the ten + WHERE pins flipped; parity moved by exactly the three P55 cases, + 176 → **179 AGREE** / 209. Flat on the benchmarks without the regex cache. + - *Plumbing removed.* `EvaluationContext` (the regex cache), WHERE's + `'ctx` lifetime and context constructors, and `with_config` (no callers) — + 147 lines out, parity byte-identical. + - *Left in slice 4:* `AND` / `OR` / `NOT` / `CASE` and WHERE's legacy + left-side method-call arm, all waiting on slice 5. WHERE's readers + (`number_literal_value`, the `DATETIME` arms, `compare_trilean`) are now + reached only through that arm. - **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 @@ -739,7 +769,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; slices 1 (pin), 2 (3VL), 3 (construction + case mode) DONE; 4 IN PROGRESS (BETWEEN/IN, comparisons done; LIKE, IS NULL next) → 5–6 +R13 one evaluator ─── ACTIVE from 2026-09-13; slices 1 (pin), 2 (3VL), 3 (construction + case mode) DONE; 4 operators DONE (BETWEEN/IN, comparisons, IS NULL, LIKE) → 5 (method calls) → 4 tail (AND/OR/NOT/CASE) → 6 ``` **A note on ordering, from the P18/P19 work being next.** The WHERE evaluator @@ -782,3 +812,4 @@ AGREE count — which makes it safe to land well before the semantics change. | 2026-09-14 | R13 slice 3: registries built once per process; one `ArithmeticEvaluator` constructor, dead date-notation plumbing deleted — parity byte-identical, high-cardinality GROUP BY ~2× faster. 3b: case-insensitive mode pinned (11 matrix + 5 clause cases) then handed in at every construction site; all flipped, parity byte-identical | — | | 2026-09-15 | R13 slice 4 (first part): WHERE's `BETWEEN`, `IN`, `NOT IN` delegate to the value evaluator. Latent alias-resolution fault pinned and fixed first; per-node TRACE logging found to be most of the value evaluator's cost and removed (SELECT/GROUP BY expressions ~2 s faster over 100k rows). Parity byte-identical at every commit | — | | 2026-09-18 | R13 slice 4 (second part): WHERE comparisons delegate. Operand readers pinned first (third matrix); P53 and P54 filed; value evaluator's `DATETIME()` moved to local time before delegating. Closes P54 — 175 → **176 AGREE**. Part 1 found to have fixed P54 for IN/BETWEEN unnoticed | — | +| 2026-09-19 | R13 slice 4 (third part): IS NULL and LIKE delegate; one `sql_like` for both evaluators. P55 (LIKE compiled as an unescaped regex) filed and closed — 176 → **179 AGREE**; the value evaluator's exponential LIKE matcher replaced first; D2 (LIKE over a number matches its displayed text) decided; `EvaluationContext` removed | — | diff --git a/docs/SQL_PARITY.md b/docs/SQL_PARITY.md index dd7e3440..4fbb0426 100644 --- a/docs/SQL_PARITY.md +++ b/docs/SQL_PARITY.md @@ -65,10 +65,10 @@ 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, **206 cases** (176 AGREE / 15 DIFFER / -12 GAP / 1 OURS_ONLY / 2 BOTH_ERR as of 2026-09-18, after R13 slice 4's -comparison delegation closed [P54](#p54); [P51](#p51), [P52](#p52) open, -[P53](#p53) open at low priority). The largest single movement so far remains the 2026-09-05 +Corpus coverage today: tiers 01–10, **209 cases** (179 AGREE / 15 DIFFER / +12 GAP / 1 OURS_ONLY / 2 BOTH_ERR as of 2026-09-19, after R13 slice 4 closed +[P54](#p54) and [P55](#p55); [P51](#p51), [P52](#p52) open, [P53](#p53) open +at low priority). 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 @@ -93,7 +93,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** | **Slices 1–2 done 2026-09-13** (value evaluator three-valued, 168 → 174 AGREE, closed P48/P50); **slice 3 done 2026-09-14** (one construction path; case-insensitive mode now honoured outside WHERE, parity unmoved); **slice 4 in progress** (WHERE arms delegate, one operator per commit: BETWEEN/IN 2026-09-15, comparisons 2026-09-18 closing [P54](#p54), 175 → 176 AGREE; LIKE and IS NULL 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 | +| **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 done 2026-09-14** (one construction path; case-insensitive mode now honoured outside WHERE, parity unmoved); **slice 4 in progress** (WHERE arms delegate, one operator per commit: BETWEEN/IN 2026-09-15, comparisons 2026-09-18 closing [P54](#p54), IS NULL and LIKE 2026-09-19 closing [P55](#p55), 175 → 179 AGREE; every WHERE operator now delegates — AND/OR/NOT/CASE follow slice 5). 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 | From 38d75b528628fef4c51b1a63ab8bda27d398a986 Mon Sep 17 00:00:00 2001 From: TimelordUK Date: Sat, 19 Sep 2026 11:02:15 +0100 Subject: [PATCH 8/8] fix(parity): rename LIKE test data out of a .gitignore pattern The P55 corpus cases read data/like_patterns.csv, which `.gitignore`'s benchmark-output rule `like_*.csv` matched - so `git add -A` skipped it and the file never reached the branch. Locally the cases passed against the untracked copy; in CI both engines failed to find the file, and all three cases landed in BOTH_ERR instead of AGREE (run 35435295922). Renamed to data/pattern_matching.csv, which no ignore rule matches, rather than force-adding a file the rules would keep skipping. Checked the other six corpus data files are all tracked. Co-Authored-By: Claude Opus 5 (1M context) --- data/pattern_matching.csv | 7 +++++++ docs/ENGINE_REFACTORING.md | 2 +- tests/comparison/corpus/02_where.toml | 12 ++++++------ 3 files changed, 14 insertions(+), 7 deletions(-) create mode 100644 data/pattern_matching.csv diff --git a/data/pattern_matching.csv b/data/pattern_matching.csv new file mode 100644 index 00000000..94d256c4 --- /dev/null +++ b/data/pattern_matching.csv @@ -0,0 +1,7 @@ +id,s +1,abc +2,a.c +3,a(bc +4,ABC +5,[a]bc +6,a.b.c diff --git a/docs/ENGINE_REFACTORING.md b/docs/ENGINE_REFACTORING.md index 585cea34..4d816f41 100644 --- a/docs/ENGINE_REFACTORING.md +++ b/docs/ENGINE_REFACTORING.md @@ -680,7 +680,7 @@ feature work**, and so we can tell the difference between "this is awkward" and and `_` in the pattern and escaping nothing else, so it got six of ten wrong — **P55**: `LIKE 'a.c'` matched `abc`, `'[a]bc'` was a character class, `'a(b%'` failed the whole query, and `%` stopped at a newline. Three - corpus cases over new `data/like_patterns.csv`. + corpus cases over new `data/pattern_matching.csv`. - *The value evaluator's matcher was exponential.* Right on all ten patterns, but recursive over every split for every `%`: one 80-character value against `'%a%a%a%a%a%a%a%b'` ran for over a minute (SELECT has had this all diff --git a/tests/comparison/corpus/02_where.toml b/tests/comparison/corpus/02_where.toml index 5d191ee6..bd06ad1a 100644 --- a/tests/comparison/corpus/02_where.toml +++ b/tests/comparison/corpus/02_where.toml @@ -304,21 +304,21 @@ sql = "SELECT id, name FROM big_ids WHERE id IN (9007199254740993, 12) ORDER BY [[case]] id = "where_like_dot_is_literal" -data = "like_patterns.csv" -sql = "SELECT id, s FROM like_patterns WHERE s LIKE 'a.c' ORDER BY id" +data = "pattern_matching.csv" +sql = "SELECT id, s FROM pattern_matching WHERE s LIKE 'a.c' ORDER BY id" # P55. WHERE turned `%` into `.*` and `_` into `.` and passed every other # character to the regex engine as-is, so `.` matched any character: `abc` # came back alongside `a.c`. [[case]] id = "where_like_bracket_is_literal" -data = "like_patterns.csv" -sql = "SELECT id, s FROM like_patterns WHERE s LIKE '[a]bc' ORDER BY id" +data = "pattern_matching.csv" +sql = "SELECT id, s FROM pattern_matching WHERE s LIKE '[a]bc' ORDER BY id" # P55. `[a]` was a regex character class: matched `abc`, missed `[a]bc` itself. [[case]] id = "where_like_paren_is_literal" -data = "like_patterns.csv" -sql = "SELECT id, s FROM like_patterns WHERE s LIKE 'a(b%' ORDER BY id" +data = "pattern_matching.csv" +sql = "SELECT id, s FROM pattern_matching WHERE s LIKE 'a(b%' ORDER BY id" # P55. `(` opened a regex group, so the query failed: "Invalid LIKE pattern # 'a(b%': regex parse error ... unclosed group".