alamb commented on a change in pull request #764:
URL: https://github.com/apache/arrow-datafusion/pull/764#discussion_r674312486
##########
File path: datafusion/src/physical_optimizer/pruning.rs
##########
@@ -399,40 +406,63 @@ impl<'a> PruningExpressionBuilder<'a> {
Ok(Self {
column,
column_expr,
+ op: correct_operator,
scalar_expr,
field,
required_columns,
- reverse_operator,
})
}
- fn correct_operator(&self, op: Operator) -> Operator {
- if !self.reverse_operator {
- return op;
- }
-
- match op {
- Operator::Lt => Operator::Gt,
- Operator::Gt => Operator::Lt,
- Operator::LtEq => Operator::GtEq,
- Operator::GtEq => Operator::LtEq,
- _ => op,
- }
+ fn op(&self) -> Operator {
+ self.op
}
fn scalar_expr(&self) -> &Expr {
- self.scalar_expr
+ &self.scalar_expr
}
fn min_column_expr(&mut self) -> Result<Expr> {
self.required_columns
- .min_column_expr(&self.column, self.column_expr, self.field)
+ .min_column_expr(&self.column, &self.column_expr, self.field)
}
fn max_column_expr(&mut self) -> Result<Expr> {
self.required_columns
- .max_column_expr(&self.column, self.column_expr, self.field)
+ .max_column_expr(&self.column, &self.column_expr, self.field)
+ }
+}
+
+fn rewrite_expr_to_prunable(
+ column_expr: &Expr,
+ op: Operator,
+ scalar_expr: &Expr,
+) -> Result<(Expr, Operator, Expr)> {
Review comment:
Thank you @lvheyang -- given the current implementation gives incorrect
results, I suggest we take a conservative here. A wise mentor of mine once told
me "I can make it go as fast as you want if you don't constrain me to be
correct"
Thus, what I recommend is to update this PR so that it works for expr types
we know work (e.g. just the ones you list above is more than adequate).
Then we can expand the list of supported Expr types as future PRs.
If you try and special case as many Expr types as possible in this PR I
think it is going to be quite challenging.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
