fix: disable aggregate dynamic filter when unsupported expressions are present (#24816) - #24818
Closed
AboEl3iz wants to merge 2 commits into
Closed
fix: disable aggregate dynamic filter when unsupported expressions are present (#24816)#24818AboEl3iz wants to merge 2 commits into
AboEl3iz wants to merge 2 commits into
Conversation
Contributor
|
It's a duplicate of an earlier PR #24817 without coordination, so close it for now. Please reopen if you can communicate with the author/reviewers and agree on proceeding with this PR instead. |
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?
Rationale for this change
When an aggregate query contains both supported expressions (such as
MIN(a)) and unsupported expressions (such asMIN(c + 1)), DataFusion previously built a dynamic filter using only the supported expressions.Because all aggregate expressions in an
AggregateExecshare the same input scan stream, pushing a dynamic filter constructed from a subset of aggregate expressions prunes rows required by the unsupported expressions, producing incorrect aggregate results.Aggregate dynamic filter pushdown must be an all-or-nothing optimization: if any aggregate expression in the
AggregateExeccannot produce a valid dynamic filter predicate, dynamic filtering must be disabled (dynamic_filter = None) for that node.What changes are included in this PR?
AggregateExec::init_dynamic_filterindatafusion/physical-plan/src/aggregates/mod.rsto return early when an aggregate argument is not a singleColumnreference.test_dynamic_filter_disabled_when_unsupported_expr_presentindatafusion/physical-plan/src/aggregates/mod.rs.dynamic_rg_pruning_disabled_when_unsupported_aggregate_presentindatafusion/core/tests/parquet/dynamic_row_group_pruning.rsreproducing issue Aggregate dynamic filter can prune rows required by unsupported expressions #24816.What is the testing strategy for this PR?
test_dynamic_filter_disabled_when_unsupported_expr_presentindatafusion-physical-planchecks thatdynamic_filterremainsNonewhen an unsupported expression likeMIN(a + 1)is present.dynamic_rg_pruning_disabled_when_unsupported_aggregate_presentindatafusion/core/tests/parquet/dynamic_row_group_pruning.rsverifies that queries withMIN(a), MAX(a), MAX(b), MIN(c + 1)evaluate correctly (71) without row groups being pruned.Are there any user-facing changes?
No breaking API or user-facing changes. Correctness fix for aggregate queries with dynamic filter pushdown.