Fix parquet filter_pushdown: respect parquet filter pushdown config in scan#16646
Fix parquet filter_pushdown: respect parquet filter pushdown config in scan#16646alamb merged 2 commits intoapache:mainfrom
Conversation
|
I'm trying to understand why this isn't caught by |
|
I can confirm this new test fails without the change: |
| /// Defaults to false | ||
| /// | ||
| /// [`Expr`]: datafusion_expr::Expr | ||
| /// Defaults to false. |
There was a problem hiding this comment.
How did this result in wrong results?
Is it because the planner assumed filters would be pushed down but they weren't?
There was a problem hiding this comment.
Well this bit is just a comment 😛
But this produced wrong results because we'd return Supported(filter) up to the parents -> FilterExec removed from the plan but ParquetOpener.pushdown_filters = false -> pruning did not actually happen during the scan.
| None => conjunction(allowed_filters.iter().cloned()), | ||
| }; | ||
| source.predicate = Some(predicate); | ||
| source = source.with_pushdown_filters(pushdown_filters); |
There was a problem hiding this comment.
@alamb this is the relevant LOC that fixes things
ianthetechie
left a comment
ianthetechie
left a comment
There was a problem hiding this comment.
I can confirm as well that it works when pointing at this branch using my original code. Thanks!
| // If both are disabled, we will not push down the filters. | ||
| // By default they are both disabled. | ||
| // Regardless of pushdown, we will update the predicate to include the filters | ||
| // because even if scan pushdown is disabled we can still use the filters for stats pruning. |
There was a problem hiding this comment.
I had wondered about this; thanks for updating this comment!
There was a problem hiding this comment.
Thank you @adriangb and @ianthetechie
With his and several other issues, I think we should make a 48.0.1 release:
I will begin work on that later today
alamb
changed the title
|
Thank you @adriangb -- I plan to backport this to the |
…n scan (apache#16646) * respect parquet filter pushdown config in scan * Add test
3 participants
pushdown_filtersis enabled (starting in 48.0.0) #16588