fix(scan): keep the predicate when a partition set cannot express it - #886
Open
jackylee-ch wants to merge 1 commit into
Open
jackylee-ch wants to merge 1 commit into
jackylee-ch wants to merge 1 commit into
Conversation
collect_eq_candidates recognised only And plus Eq/In/IsNull leaves; every
other node fell through `_ => {}` and was discarded. Once every partition
field had a candidate, from_predicate built a PartitionSet, and
matches_entry is then pure HashSet membership -- the predicate is never
consulted again. A partition-only filter is Exact, so DataFusion drops
its residual and the discarded conjuncts are never enforced.
collect_eq_candidates now reports whether it consumed the whole tree, and
from_predicate keeps the predicate when it did not. A second conjunct on
an already-pinned field also bails: the assignment used to overwrite the
first, so the set kept whichever came last, which can be the wider one.
This is a regression rather than original behaviour. 513942e (apache#224) added
is_exact_filter_pushdown_for_schema while every manifest entry was still
tested with eval_row against the whole partition predicate; 391349d
(apache#269) replaced that with PartitionFilter::matches_entry two days later.
bucket_filter's copy of this walk is left alone on purpose. Its result is
consumed as a pre-filter -- table_scan.rs:364-369 and its three siblings
drop a bucket only when it is absent from the target set -- so discarding
a conjunct there can only widen the candidate set, never drop a matching
file.
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.
Defect.
from_predicaterecognised onlyAndplusEq/In/IsNullleaves; everything else fell through_ => {}, discarded. With every field covered it built aPartitionSet, andmatches_entryis then pureHashSetmembership — the predicate is never read again. A partition-only filter isExact, so DataFusion drops its residual:WHERE dt='2024-01-01' AND dt>='2024-01-02'returned 1 row, not 0.Fix.
collect_eq_candidatesreports whether it consumed the whole tree;from_predicatekeeps the predicate when it did not. A second conjunct on an already-pinned field also bails: the old code overwrote it, keeping whichever came last — possibly the wider one.Cost. The fallback pays
from_serialized_bytes+eval_rowper entry instead of a hash lookup, and anInleaf loses its min/max bound (stats_filter.rs:73). Only on shapes answered wrongly before.