alamb commented on a change in pull request #1128:
URL: https://github.com/apache/arrow-datafusion/pull/1128#discussion_r731293828
##########
File path: datafusion/src/optimizer/constant_folding.rs
##########
@@ -127,164 +124,365 @@ impl<'a> ConstantRewriter<'a> {
false
}
-}
-impl<'a> ExprRewriter for ConstantRewriter<'a> {
- /// rewrite the expression simplifying any constant expressions
- fn mutate(&mut self, expr: Expr) -> Result<Expr> {
- let new_expr = match expr {
- Expr::BinaryExpr { left, op, right } => match op {
- Operator::Eq => match (left.as_ref(), right.as_ref()) {
- (
- Expr::Literal(ScalarValue::Boolean(l)),
- Expr::Literal(ScalarValue::Boolean(r)),
- ) => match (l, r) {
- (Some(l), Some(r)) => {
- Expr::Literal(ScalarValue::Boolean(Some(l == r)))
- }
- _ => Expr::Literal(ScalarValue::Boolean(None)),
- },
- (Expr::Literal(ScalarValue::Boolean(b)), _)
- if self.is_boolean_type(&right) =>
- {
- match b {
- Some(true) => *right,
- Some(false) => Expr::Not(right),
- None => Expr::Literal(ScalarValue::Boolean(None)),
+ pub fn rewrite(&mut self, mut expr: Expr) -> Expr {
+ let name = match &expr {
+ Expr::Alias(_, name) => Some(name.clone()),
+ _ => None,
+ };
+
+ let rewrite_root = self.rewrite_const_expr(&mut expr);
+ if rewrite_root {
+ match evaluate_const_expr_unchecked(&expr) {
+ Ok(s) => expr = Expr::Literal(s),
+ Err(_) => return expr,
+ }
+ }
+ match name {
+ Some(name) => {
+ let existing_alias = match &expr {
+ Expr::Alias(_, new_alias) => Some(new_alias.as_str()),
+ _ => None,
+ };
+ let apply_new_alias = match existing_alias {
+ Some(new) => *new != name,
+ None => true,
+ };
+ if apply_new_alias {
+ expr = Expr::Alias(Box::new(expr), name);
+ }
+ expr
+ }
+ None => expr,
+ }
+ }
+
+ ///Evaluates all literal expressions in the list.
+ fn const_fold_list_eager(&mut self, args: &mut Vec<Expr>) {
+ for arg in args.iter_mut() {
+ if self.rewrite_const_expr(arg) {
+ if let Ok(s) = evaluate_const_expr_unchecked(arg) {
+ *arg = Expr::Literal(s);
+ }
+ }
+ }
+ }
+ ///Tests to see if the list passed in is all literal expressions, if they
are then it returns true.
+ ///If some expressions are not literal then the literal expressions are
evaluate_const_expr_uncheckedd and it returns false.
+ fn const_fold_list(&mut self, args: &mut Vec<Expr>) -> bool {
+ let can_rewrite = args
+ .iter_mut()
+ .map(|e| self.rewrite_const_expr(e))
+ .collect::<Vec<bool>>();
+ if can_rewrite.iter().all(|f| *f) {
+ return true;
+ } else {
+ for (rewrite_expr, expr) in can_rewrite.iter().zip(args) {
+ if *rewrite_expr {
+ if let Ok(s) = evaluate_const_expr_unchecked(expr) {
+ *expr = Expr::Literal(s);
+ }
+ }
+ }
+ }
+ false
+ }
+ ///This attempts to simplify expressions of the form col(Boolean) =
Boolean and col(Boolean) != Boolean
+ /// e.g. col(Boolean) = Some(true) -> col(Boolean). It also handles == and
!= between two boolean literals as
+ /// the binary operator physical expression currently doesn't handle them.
Review comment:
I think if we take the approach on
https://github.com/apache/arrow-datafusion/pull/1145, we can remove this code
that handles constants
--
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: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
