Repository navigation
refactor(r13): WHERE IS NULL and LIKE delegate; closes P55 (R13 slice 4, part 3) - #91
Merged
Merged
Conversation
…aluator (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) <[email protected]>
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) <[email protected]>
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) <[email protected]>
… (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) <[email protected]>
…tors (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) <[email protected]>
…e 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) <[email protected]>
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
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) <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
R13 slice 4, third part. WHERE's
IS NULL,IS NOT NULLandLIKEnow go through the value evaluator, so every WHERE operator now delegates. What's left of slice 4 (AND/OR/NOT/CASE) waits on slice 5, method calls.Closes P55: a LIKE pattern in WHERE was read as a regex
WHERE built its matcher by replacing
%and_and escaping nothing else, so any regex syntax in a pattern took effect:LIKE 'a.c'matchedabctoo (soLIKE '%.csv'matchesxcsv)LIKE '[a]bc'matchedabcbut not the literal[a]bcLIKE 'a(b%'failed the whole query: regex parse error … unclosed group%stopped at a newlineThe bug was in both the cached path (
EvaluationContext) and the uncached fallback.Commits
Nullliteral on the right. Also deletes theIS,IS NOTand==operator arms, which the parser never produces.EXPECTED_LIKE), which records six wrong WHERE answers. Three corpus cases over newdata/like_patterns.csvpin it end to end (DIFFER, DIFFER, GAP).'%a%a%a%a%a%a%a%b'ran for over a minute (SELECT has had this all along). Replaced with the iterative wildcard algorithm, O(text × pattern), before WHERE switched to it. Adds 28 unit cases plus a 2,000-character case with a 1 s budget; the CLI case now takes 57 ms.5.0displays as5). Recorded inSQL_PARITY.mdas a deliberate difference from DuckDB. Pinned as a fifth matrix set whose expected answers are this decision; WHERE's old "always FALSE" is recorded against it.sql_like, called by the value evaluator and by WHERE's one remaining legacy arm (a method call on the left). Exactly the ten pinned WHERE answers changed, and parity moved by exactly the three P55 cases: 176 → 179 AGREE / 209.EvaluationContext, WHERE's'ctxlifetime and context constructors,with_config(no callers) and the always-zero cache stats. 147 lines out, parity byte-identical.Verification
./run_all_tests.shgreen: 830 lib + 501 integration Rust tests, 572 Python tests, all FORMAL examples. The one SMOKE failure (expander_rewritersquery 7) also fails on main.trades_100000.csv, WHEREIS NULLand both LIKE shapes stay at CSV load time without the regex cache.Next: slice 5, method calls
WHERE's own
Contains,StartsWithandEndsWithalways ignore case and answer FALSE for NULL, and it has its own left-sideLength,IndexOfandTrim*. These need pinning against the registry functions the value evaluator maps them to, then moving over.🤖 Generated with Claude Code