return absent stats when filters are pushed down #12471
Conversation
github-actions
bot
added
physical-expr
waruto210
force-pushed
the
return_inexact_stats_with_filter_pushdown
branch
2 times, most recently
from
90cd6b5
to
20c74b9
Compare
waruto210
changed the title
There was a problem hiding this comment.
Thank you @waruto210 -- I think this is a great start
I left some comments -- let us know what you think
| // When filters are pushed down, we have no way of knowing the exact statistics. | ||
| // Note that pruning predicate is also a kind of filter pushdown. | ||
| let stats = if self.pruning_predicate.is_some() | ||
| || self.page_pruning_predicate.is_some() |
There was a problem hiding this comment.
I think bloom filters should also belong in this list 🤔
There was a problem hiding this comment.
Bloom filters are used by pruning_predicate .
There was a problem hiding this comment.
Makes sense, maybe we can update the comments to help future readers. I left a suggestion
return absent stats when filters are pushed down
waruto210
force-pushed
the
return_inexact_stats_with_filter_pushdown
branch
from
20c74b9
to
a7606b4
Compare
|
@alamb PTAL |
There was a problem hiding this comment.
Thanks @waruto210 this looks good to me now
I do think it is worth considering more explicit test coverage for partition column pushdown, but I don't think it is required.
Thanks again for the contribution
| // When filters are pushed down, we have no way of knowing the exact statistics. | ||
| // Note that pruning predicate is also a kind of filter pushdown. | ||
| let stats = if self.pruning_predicate.is_some() | ||
| || self.page_pruning_predicate.is_some() |
There was a problem hiding this comment.
Makes sense, maybe we can update the comments to help future readers. I left a suggestion
Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
|
Thank you again @waruto210 |
* do not pushdown filters that can be resolved only using partition cols return absent stats when filters are pushed down * fix and add test * Update datafusion/core/src/datasource/physical_plan/parquet/mod.rs Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org> * add test for partition pruning filters --------- Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
Which issue does this PR close?
Closes #12416.
Rationale for this change
Fix the bug mentioned in #12416.
What changes are included in this PR?
Unlike what was mentioned in #12416, I chose to return absent stats because it's hard to know the selectivity of the filters, and also, for filters that can be resolved using only partition cols, there's no need to pushdown them to the TableScanExec, which would otherwise produce useless
unhandledpruning predicate.Are these changes tested?
Yes
Are there any user-facing changes?