-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Impl gather_filters_for_pushdown for CoalescePartitionsExec
#18046
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
adriangb
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add a test (via SLT or unit) that shows that a plan with a CoalesceBatchesExec can support dynamic filter pushdown?
Sure |
e333ac7 to
6cdc4e2
Compare
| Ok: | ||
| - SortExec: TopK(fetch=1), expr=[b@1 DESC NULLS LAST], preserve_partitioning=[false] | ||
| - CoalesceBatchesExec: target_batch_size=1024 | ||
| - DataSourceExec: file_groups={1 group: [[test.parquet]]}, projection=[a, b, c], file_type=test, pushdown_supported=true, predicate=DynamicFilter [ empty ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a test for CoalesceBatchesExec meanwhile
The key change: predicate=DynamicFilter [ empty ]
| Ok: | ||
| - SortExec: TopK(fetch=1), expr=[b@1 DESC NULLS LAST], preserve_partitioning=[false] | ||
| - CoalescePartitionsExec | ||
| - DataSourceExec: file_groups={2 groups: [[test1.parquet], [test2.parquet]]}, projection=[a, b, c], file_type=test, pushdown_supported=true, predicate=DynamicFilter [ empty ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The key change: predicate=DynamicFilter [ empty ]
…he#18046) * Impl gather_filters_for_pushdown for CoalescePartitionsExec * add tests
…he#18046) * Impl gather_filters_for_pushdown for CoalescePartitionsExec * add tests
Which issue does this PR close?
Rationale for this change
I found
CoalescePartitionsExecdoesn't implementgather_filters_for_pushdown, which means it'll block dynamic filter pushdown. I don't find a reason to forbid it. (Correct me if I misunderstood)What changes are included in this PR?
Are these changes tested?
Are there any user-facing changes?