Repository navigation
fix: simplify IN lists containing NULL literals in EliminateFilter - #26548
Open
raghavyadav01 wants to merge 1 commit into
Open
raghavyadav01 wants to merge 1 commit into
raghavyadav01 wants to merge 1 commit into
Conversation
`x IN (a, NULL)` is TRUE exactly when x matches a: a NULL member can only turn FALSE into NULL, and NULL rejects a row just like FALSE. The NULL members can therefore be dropped, and a list of nothing but NULLs rejects every row. `x NOT IN (a, NULL)` is FALSE when x matches a and NULL otherwise, so it is never TRUE and also rejects every row. Leaving the NULLs in place kept filters that PruningPredicate does not handle well, which is the degradation described in apache#26542. Extend simplify_filter_predicate with an Expr::InList arm. This is sound only under filter semantics, where just the TRUE rows are kept; the same rewrite in ExprSimplifier would change SELECT 3 IN (1, NULL) from NULL to FALSE, so it stays in EliminateFilter. Closes apache#26542
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.
Which issue does this PR close?
Closes #26542.
Rationale for this change
Under filter semantics only rows for which the predicate is
TRUEare kept, soNULLrejects a row exactly likeFALSE. That makes two rewrites sound forINlists that holdNULLliterals:x IN (a, NULL)isTRUEexactly whenxmatchesa. ANULLmember can only turnFALSEintoNULL, and both reject. TheNULLmembers can be dropped, and a list of nothing butNULLs rejects every row.x NOT IN (a, NULL)isFALSEwhenxmatchesaandNULLotherwise, so it is neverTRUEand rejects every row.Leaving the
NULLs in place keeps a filter thatPruningPredicatedoes not handle well, which is the pruning degradation described in the issue. #24711 fixed this only for the compact path (lists over 20 members) and #25187 made short lists on several types reach the same path.What changes are included in this PR?
An
Expr::InListarm insimplify_filter_predicate, plus asimplify_filter_in_listhelper and anis_null_literalpredicate, indatafusion/optimizer/src/eliminate_filter.rs.The rewrite stays in
EliminateFilterdeliberately. It is not sound inExprSimplifier: in a general expression contextSELECT 3 IN (1, NULL)must remainNULL, and dropping theNULLwould make itFALSE.The negated case keeps the original
InListinsideFilterPredicate::RejectsAllrather than substituting a literal, so nothing is discarded that theAND/ORreconstruction may still need.Are these changes tested?
Yes — five tests in
eliminate_filter.rs:filter_in_list_with_null_drops_nulla IN (1, 2, NULL)→a IN (1, 2)filter_in_list_of_only_nulls_rejects_alla IN (NULL)→EmptyRelationfilter_not_in_list_with_null_rejects_alla NOT IN (1, NULL)→EmptyRelationfilter_in_list_without_null_is_untouchedNULL→ unchangedfilter_not_in_list_without_null_is_untouchedNULL→ unchangedI wrote the tests first and confirmed the three
NULLcases fail onmainbefore the change and pass after, while the two control tests pass in both states.Also run: the full
datafusion-optimizersuite (948 + 29 + 5 passing),cargo fmt --all -- --check, andcargo clippy -p datafusion-optimizer --all-targets -- -D warnings, all clean.in_list.sltand thenull_aware_*slt files pass.expr,predicates,scalarandaggregateslt files each fail identically with and without this change on my machine (a local test-data issue), so they are unaffected.Are there any user-facing changes?
Explain output for filters whose
INlist containsNULLliterals: theNULLmembers no longer appear, and a filter that can never beTRUEbecomes an empty relation. Query results are unchanged.