Re-enabled null-equal join dynamic filters with an IS NULL predicate.#3
Draft
mdashti wants to merge 6 commits into
Draft
Re-enabled null-equal join dynamic filters with an IS NULL predicate.#3mdashti wants to merge 6 commits into
mdashti wants to merge 6 commits into
Conversation
The hash-join dynamic filter pushed `key IN build_keys` down to the probe scan for null-aware anti joins too. That drops the probe-side NULL, but `NOT IN` three-valued logic needs it to collapse the result to zero rows, so the join silently returned rows. OR `probe_key IS NULL` into the pushed predicate. Non-NULL probe rows still get filtered; only the NULL additionally survives.
Exercises the pushdown path the existing in-memory tests miss: parquet with row-level filtering, so the pushed dynamic filter actually drops rows. Without the fix `id NOT IN (SELECT eid ...)` returns 1 and 3 instead of zero rows.
mdashti
force-pushed
the
moe/null-equal-dynamic-filter
branch
from
3721aa9 to
9f4e40c
Compare
It's cheap, so it short-circuits NULL rows before the costlier filter. The `debug_assert` pins the single-key invariant the `on_right[0]` indexing relies on.
RESET restores the defaults at the end instead of re-setting explicit values. A probe can hold several NULLs, so the comments read as plural.
build-side predicate prunes a probe-side NULL that can null-match a build-side NULL. Push the filter with `OR key IS NULL` over the nullable probe keys instead, the way apache#23104 does for null-aware anti joins. A NOT NULL key never widens the filter, so an all-NOT-NULL join keeps full selectivity.
The `unwrap_or(true)` widening on an unresolved nullability check wasn't obvious. An extra NULL row is safe; dropping a needed one isn't.
mdashti
force-pushed
the
moe/null-equal-dynamic-filter
branch
from
9f4e40c to
9620b97
Compare
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
1 participant
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.
Runs the fork's CI for the upstream PR apache#23106. Same branch and commits (the apache#23104 probe-NULL helper plus the null-equal change); opened against the fork's
mainso the full suite runs here too.Re-enables hash-join dynamic filter pushdown for null-equal joins (reverting apache#22965's
return false) by pushing the filter withOR key IS NULLover every nullable probe key. A NOT NULL key never widens it. Tests: apache#22965's SLT now shows the filter back on the probe with the result unchanged, plus a multi-key case; the reject unit test flips to assert-allowed; andpreserve_probe_nullsunit tests cover the nullable-vs-NOT NULL paths.