Repository navigation
refactor(r13): WHERE comparisons delegate to the value evaluator; closes P54 (R13 slice 4, part 2) - #90
Merged
Conversation
…lice 4) WHERE and the value evaluator each turn a literal into a value with their own reader. A third matrix, generated by DuckDB, runs the comparison shapes where the two readers can disagree through both evaluators: - P54 (filed): WHERE reads a number literal through f64, so an integer beyond 2^53 is rounded onto its neighbour. `WHERE id = 9007199254740993` returns the row with id ...992. Three WHERE entries recorded. Corpus case where_big_int_literal_equals pins it end to end (DIFFER). - `s = 5.0` against the string '5.0': WHERE reads the literal as Integer 5 and compares as text. One WHERE entry. - P53 (filed, low priority): `'5.0' = 5` compares as text in both evaluators; DuckDB casts. Recorded for both; no corpus case (explained in the entry). IN and BETWEEN already answer correctly: delegating them in part 1 fixed P54 for those operators without anyone noticing. Control case where_big_int_literal_in_list AGREEs. check_matrix now takes its table. Parity 175 AGREE / 206, contract holds. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
…o (R13 slice 4) WHERE's operand reader took today's date from `Local`, the value evaluator from `Utc`. They differ for the hours where the local and UTC dates differ - 00:00-01:00 under BST - so `SELECT d >= DATETIME()` and `WHERE d >= DATETIME()` compared against different days. DuckDB's `current_date` uses the session time zone, local by default, so WHERE had it right. Lands before the comparison arms delegate: otherwise delegating would have moved WHERE onto the UTC date. Not pinned first, unlike the rest of the slice: the divergence exists only while the two dates differ, so a test could not fail on demand without injecting a clock. Parity report byte-identical. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
…13 slice 4) `=`, `<>`, `!=`, `<`, `<=`, `>`, `>=` in WHERE now evaluate through `ArithmeticEvaluator` and cross back in `evaluate_delegated`. The raw window check stays on WHERE's side. Two shapes keep WHERE's path for now: a legacy method call on the left (`Length()`, `IndexOf()`, `Trim*()`), whose registry twins are slice 5's, and the operators with their own arms (LIKE, IS NULL). `evaluate_binary_op` now receives the node as well as its parts, so neither the delegation nor the existing P37 value-as-predicate branch rebuilds (and clones) the expression per row. Closes P54: the four operand-matrix entries recorded by the pin commit went FIXED and nothing else moved; `where_big_int_literal_equals` DIFFER -> AGREE, parity 175 -> 176 AGREE over 206. Over trades_100000.csv, `WHERE price + 0 > quantity` 3.0 s -> 2.5 s (load time); other WHERE shapes unchanged. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
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, second part: WHERE's comparison operators (
=,<>,!=,<,<=,>,>=) now go through the value evaluator and are converted back to a WHERE truth value once, inevaluate_delegated. Comparisons with a legacy method call on the left (Length(),IndexOf(),Trim*()) keep WHERE's path until slice 5.Closes P54: a large integer literal matched the wrong row in WHERE
WHERE read every number literal through
f64, which can't hold every integer above 2^53. SoWHERE id = 9007199254740993returned the row with id 9007199254740992, a different record, not an empty result. On that same rowSELECT id = 9007199254740993saidfalse, so the query contradicted itself. The value evaluator readsi64first and was already right.Commits
EXPECTED_OPERANDS) for the cases where the two evaluators turn a literal into different values. It records four wrong WHERE answers (three for P54, plus'5.0' = 5.0). Also adds the corpus casewhere_big_int_literal_equals(DIFFER) with anINcontrol, anddata/big_ids.csv. Files P53:'5.0' = 5compares as text in both evaluators, where DuckDB casts the string to a number. It's low priority and has no corpus case; the entry explains why.DATETIME()in local time. WHERE took today's date from local time, the value evaluator from UTC, so they disagreed between 00:00 and 01:00 under BST. WHERE was right (DuckDB'scurrent_dateuses the session time zone, local by default), so the value evaluator changed before the switch-over. This is the one step without a failing test first: the two only disagree during that hour.evaluate_binary_opnow receives the whole expression node, so nothing rebuilds and clones it on each row.Things this turned up
INandBETWEENthrough the value evaluator also switched them to its literal reader, which fixed P54 for those two operators. The parity report couldn't show it because no case held an integer that large.9007199254740992.0and only thenamecolumn showed the wrong row. A case that selects only a large integer can't tell two ids apart. This is noted in the corpus case and in P54.Verification
--checkholds (206 cases). Reports compared against a saved baseline at each commit: the only change was the P54 case../run_all_tests.shgreen: Rust, 572 Python tests, all FORMAL examples. The one SMOKE failure (expander_rewritersquery 7) also fails on main.trades_100000.csv(~2.4 s of which is loading the CSV):WHERE price + 0 > quantity3.0 s → 2.5 s; the other WHERE shapes are unchanged.Next (slice 4 part 3)
LIKE: WHERE uses a regex cache, and the two evaluators disagree on a non-string operand (
5.0 LIKE '5%'is FALSE in WHERE, TRUE in the value evaluator, an error in DuckDB), so that needs a decision first. Then IS NULL. AND/OR/NOT/CASE wait on slice 5.🤖 Generated with Claude Code