alamb commented on a change in pull request #1245:
URL: https://github.com/apache/arrow-datafusion/pull/1245#discussion_r743542556
##########
File path: datafusion/src/optimizer/constant_folding.rs
##########
@@ -135,6 +135,98 @@ impl<'a> Simplifier<'a> {
false
}
+
+ fn boolean_folding_for_or(
+ const_bool: &Option<bool>,
+ bool_expr: Box<Expr>,
+ left_right_order: bool,
+ ) -> Expr {
+ // See if we can fold 'const_bool OR bool_expr' to a constant boolean
+ match const_bool {
+ // TRUE or expr (including NULL) = TRUE
+ Some(true) => Expr::Literal(ScalarValue::Boolean(Some(true))),
+ // FALSE or expr (including NULL) = expr
+ Some(false) => *bool_expr,
+ None => match *bool_expr {
+ // NULL or TRUE = TRUE
+ Expr::Literal(ScalarValue::Boolean(Some(true))) => {
+ Expr::Literal(ScalarValue::Boolean(Some(true)))
+ }
+ // NULL or FALSE = NULL
+ Expr::Literal(ScalarValue::Boolean(Some(false))) => {
+ Expr::Literal(ScalarValue::Boolean(None))
+ }
+ // NULL or NULL = NULL
+ Expr::Literal(ScalarValue::Boolean(None)) => {
+ Expr::Literal(ScalarValue::Boolean(None))
+ }
+ // NULL or expr can be either NULL or TRUE
+ // So let us not rewrite it
+ _ => {
+ let mut left =
+
Box::new(Expr::Literal(ScalarValue::Boolean(*const_bool)));
+ let mut right = bool_expr;
+ if !left_right_order {
+ left = right;
+ right =
+
Box::new(Expr::Literal(ScalarValue::Boolean(*const_bool)));
Review comment:
I think you might be able to use `swap` here if you wanted:
https://doc.rust-lang.org/stable/std/mem/fn.swap.html
so something like `std::mem::swap(&mut left, &mut right)`
##########
File path: datafusion/src/optimizer/constant_folding.rs
##########
@@ -135,6 +135,98 @@ impl<'a> Simplifier<'a> {
false
}
+
+ fn boolean_folding_for_or(
+ const_bool: &Option<bool>,
+ bool_expr: Box<Expr>,
+ left_right_order: bool,
+ ) -> Expr {
+ // See if we can fold 'const_bool OR bool_expr' to a constant boolean
+ match const_bool {
+ // TRUE or expr (including NULL) = TRUE
+ Some(true) => Expr::Literal(ScalarValue::Boolean(Some(true))),
+ // FALSE or expr (including NULL) = expr
+ Some(false) => *bool_expr,
+ None => match *bool_expr {
+ // NULL or TRUE = TRUE
+ Expr::Literal(ScalarValue::Boolean(Some(true))) => {
+ Expr::Literal(ScalarValue::Boolean(Some(true)))
+ }
+ // NULL or FALSE = NULL
+ Expr::Literal(ScalarValue::Boolean(Some(false))) => {
+ Expr::Literal(ScalarValue::Boolean(None))
+ }
+ // NULL or NULL = NULL
+ Expr::Literal(ScalarValue::Boolean(None)) => {
+ Expr::Literal(ScalarValue::Boolean(None))
+ }
+ // NULL or expr can be either NULL or TRUE
+ // So let us not rewrite it
+ _ => {
+ let mut left =
+
Box::new(Expr::Literal(ScalarValue::Boolean(*const_bool)));
+ let mut right = bool_expr;
+ if !left_right_order {
+ left = right;
+ right =
+
Box::new(Expr::Literal(ScalarValue::Boolean(*const_bool)));
+ }
+
+ Expr::BinaryExpr {
+ left,
+ op: Operator::Or,
+ right,
+ }
+ }
+ },
+ }
+ }
+
+ fn boolean_folding_for_and(
+ const_bool: &Option<bool>,
+ bool_expr: Box<Expr>,
+ left_right_order: bool,
+ ) -> Expr {
+ // See if we can fold 'const_bool AND bool_expr' to a constant boolean
+ match const_bool {
+ // TRUE and expr (including NULL) = expr
+ Some(true) => *bool_expr,
+ // FALSE and expr (including NULL) = FALSE
+ Some(false) => Expr::Literal(ScalarValue::Boolean(Some(false))),
+ None => match *bool_expr {
+ // NULL and TRUE = NULL
Review comment:
FWIW I am working on getting these constant cases to work with the
`ConstEvaluator` (so they wouldn't be strictly necessary) but it turns out
these types of expressions don't actually work in the general evaluator
I am working on it in https://github.com/apache/arrow-datafusion/pull/1163
But in any event I think this code is good 👍
--
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]
