Skip to content

fix(parity): bare value as a WHERE predicate — P37 - #65

Merged
TimelordUK merged 1 commit into
mainfrom
fix/p37-bare-value-predicate
Sep 5, 2026
Merged

fix(parity): bare value as a WHERE predicate — P37#65
TimelordUK merged 1 commit into
mainfrom
fix/p37-bare-value-predicate

Conversation

@TimelordUK

Copy link
Copy Markdown
Owner

WHERE <window> <op> <val> returned zero rows with a success exit code. The recorded root cause was wrong, and the correction is most of the value here: ExpressionLifter does walk the WHERE clause, and does hoist the window into a CTE column, leaving the outer query as WHERE lifted_value.

The rows vanished one step later. evaluate_expression's catch-all arm answered FALSE for any expression shape it did not recognise, and a bare value used as a predicate is exactly such a shape. So the defect was never window-specific — against the unfixed binary, WHERE true returned zero rows too, on any table.

That arm now evaluates the expression for its value and coerces it, via a helper the file already had for CASE branch results and had simply never wired to the top level. The two copies of the coercion table are now one function. A raw WindowFunction reaching it errors rather than coercing: an unlifted window is a defect, not a value, and a loud failure beats a silent empty result. That covers both end states the P37 decision allowed — the lifted path answers correctly, the unlifted path errors like DuckDB.

Unifying the two copies changes one behaviour deliberately: a NULL-valued predicate now yields UNKNOWN rather than FALSE. The CASE path answered FALSE. Under WHERE the two are indistinguishable and diverge only under NOT, which is the P18/P19 trap, so both paths now agree with what P18/P19 settled.

The corpus cannot gate this fix. window_in_where_inline is OURS_ONLY before and after — DuckDB rejects the query either way — so --check stays green whether we return the right rows or none, and would stay green through a regression. Hence bare_value_predicate_tests, asserting the general shape rather than the window that exposed it. The corpus case now carries a note saying not to read a green run as evidence.

Verified by building the pre-fix binary alongside, since this arm is on every WHERE path in the engine:

  • cargo test: 755 + 469 passed, 0 failed
  • all 177 corpus cases, old vs new: byte-identical except window_in_where_inline (0 rows -> the correct 7, matching the CTE-with-rn ground truth)
  • all 153 examples/*.sql, old vs new: one meaningful change — expander_rewriters.sql goes [] -> correct top-3-per-region, which is what that file has always claimed to demonstrate
  • 33 formal expectation JSONs: zero churn

Does not close P15: inline QUALIFY still errors, and needs the opposite change (transformer ordering) rather than the same one. That entry is updated with why.

Also files P41 — MODE has no tie-break rule and picks a winner from HashMap order, giving a different answer run to run on the same binary. Found while verifying this fix; unrelated to it, but it is what the remaining old-vs-new example differences turned out to be.

Claude-Session: https://claude.ai/code/session_01FVGtqMfn8kWndjZGNRxKjM

`WHERE <window> <op> <val>` returned zero rows with a success exit code.
The recorded root cause was wrong, and the correction is most of the value
here: `ExpressionLifter` does walk the WHERE clause, and does hoist the
window into a CTE column, leaving the outer query as `WHERE lifted_value`.

The rows vanished one step later. `evaluate_expression`'s catch-all arm
answered FALSE for any expression shape it did not recognise, and a bare
value used as a predicate is exactly such a shape. So the defect was never
window-specific — against the unfixed binary, `WHERE true` returned zero
rows too, on any table.

That arm now evaluates the expression for its value and coerces it, via a
helper the file already had for CASE branch results and had simply never
wired to the top level. The two copies of the coercion table are now one
function. A raw WindowFunction reaching it errors rather than coercing: an
unlifted window is a defect, not a value, and a loud failure beats a silent
empty result. That covers both end states the P37 decision allowed — the
lifted path answers correctly, the unlifted path errors like DuckDB.

Unifying the two copies changes one behaviour deliberately: a NULL-valued
predicate now yields UNKNOWN rather than FALSE. The CASE path answered
FALSE. Under WHERE the two are indistinguishable and diverge only under
NOT, which is the P18/P19 trap, so both paths now agree with what P18/P19
settled.

The corpus cannot gate this fix. `window_in_where_inline` is OURS_ONLY
before and after — DuckDB rejects the query either way — so `--check` stays
green whether we return the right rows or none, and would stay green
through a regression. Hence `bare_value_predicate_tests`, asserting the
general shape rather than the window that exposed it. The corpus case now
carries a note saying not to read a green run as evidence.

Verified by building the pre-fix binary alongside, since this arm is on
every WHERE path in the engine:
  - cargo test: 755 + 469 passed, 0 failed
  - all 177 corpus cases, old vs new: byte-identical except
    window_in_where_inline (0 rows -> the correct 7, matching the
    CTE-with-rn ground truth)
  - all 153 examples/*.sql, old vs new: one meaningful change —
    expander_rewriters.sql goes [] -> correct top-3-per-region, which is
    what that file has always claimed to demonstrate
  - 33 formal expectation JSONs: zero churn

Does not close P15: inline QUALIFY still errors, and needs the opposite
change (transformer ordering) rather than the same one. That entry is
updated with why.

Also files P41 — MODE has no tie-break rule and picks a winner from
HashMap order, giving a different answer run to run on the same binary.
Found while verifying this fix; unrelated to it, but it is what the
remaining old-vs-new example differences turned out to be.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FVGtqMfn8kWndjZGNRxKjM
@TimelordUK
TimelordUK merged commit 1086c77 into main Sep 5, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant