Fix page index pruning fail on complex_expr#4387
Conversation
Signed-off-by: yangjiang <yangjiang@ebay.com>
| "SELECT * FROM t where abs(f - 1) <= 0.000001 and f >= 0.1", | ||
| Some(0), | ||
| Some(2), | ||
| Some(10), |
There was a problem hiding this comment.
only use "f >= 0" to prune it will prune two pages (each 5 rows), so make it 10 make sense
| ); | ||
| } else { | ||
| trace!( | ||
| // when building `PruningPredicate`, some single column filter like `abs(i) = 1` |
There was a problem hiding this comment.
I think we can support rewrite abs(i) = 1 to i = 1 or i = -1🤔 further.
|
@alamb PTAL, and sorry for taking such a long time to fix this(we are doing bugBash in our system😂 , have limited time on datafusion recently). I think now page pruning has the same amount of test case as row group pruning, do we need any more test case so that we can enable Page Filtering by default 🤔 |
| } else { | ||
| trace!( | ||
| // when building `PruningPredicate`, some single column filter like `abs(i) = 1` | ||
| // will be rewrite to `lit(true)`, so may have an empty required_columns. |
There was a problem hiding this comment.
this is the why it fails.
There was a problem hiding this comment.
Thank you for these comments, BTW -- they help both to review the code as well as read it in the future 👍
alamb
left a comment
alamb
left a comment
There was a problem hiding this comment.
Makes sense to me -- thanks @Ted-Jiang 👍
Looking at this PR with whitespace blind diff helped me: https://github.com/apache/arrow-datafusion/pull/4387/files?w=1
|
Benchmark runs are scheduled for baseline = da55f93 and contender = 9139183. 9139183 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
3 participants
Signed-off-by: yangjiang yangjiang@ebay.com
Which issue does this PR close?
Closes #4317.
Rationale for this change
After fix all the ignore issue, I think now page pruning has the same amount test case like row group pruning✌️.
What changes are included in this PR?
Are these changes tested?
Are there any user-facing changes?