[arrow-datafusion] branch master updated: Remove panics from simplify_expressions optimizer rule (#3343)

[agrove]

This is an automated email from the ASF dual-hosted git repository.

agrove pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/arrow-datafusion.git


The following commit(s) were added to refs/heads/master by this push:
     new ac5ec3ce5 Remove panics from simplify_expressions optimizer rule 
(#3343)
ac5ec3ce5 is described below

commit ac5ec3ce58e517212d0422f00f439e284a2a5fb6
Author: Andy Grove <andygrov...@gmail.com>
AuthorDate: Fri Sep 2 17:12:42 2022 -0600

    Remove panics from simplify_expressions optimizer rule (#3343)
---
 datafusion/optimizer/src/simplify_expressions.rs | 28 ++++++++++++++----------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/datafusion/optimizer/src/simplify_expressions.rs 
b/datafusion/optimizer/src/simplify_expressions.rs
index 16746a42f..334ec6182 100644
--- a/datafusion/optimizer/src/simplify_expressions.rs
+++ b/datafusion/optimizer/src/simplify_expressions.rs
@@ -166,10 +166,13 @@ fn is_op_with(target_op: Operator, haystack: &Expr, 
needle: &Expr) -> bool {
 /// `Expr::Literal(ScalarValue::Boolean(v))`.
 ///
 /// panics if expr is not a literal boolean
-fn as_bool_lit(expr: Expr) -> Option<bool> {
+fn as_bool_lit(expr: Expr) -> Result<Option<bool>> {
     match expr {
-        Expr::Literal(ScalarValue::Boolean(v)) => v,
-        _ => panic!("Expected boolean literal, got {:?}", expr),
+        Expr::Literal(ScalarValue::Boolean(v)) => Ok(v),
+        _ => Err(DataFusionError::Internal(format!(
+            "Expected boolean literal, got {:?}",
+            expr
+        ))),
     }
 }
 
@@ -390,11 +393,12 @@ impl<'a> ExprRewriter for ConstEvaluator<'a> {
     }
 
     fn mutate(&mut self, expr: Expr) -> Result<Expr> {
-        if self.can_evaluate.pop().unwrap() {
-            let scalar = self.evaluate_to_scalar(expr)?;
-            Ok(Expr::Literal(scalar))
-        } else {
-            Ok(expr)
+        match self.can_evaluate.pop() {
+            Some(true) => Ok(Expr::Literal(self.evaluate_to_scalar(expr)?)),
+            Some(false) => Ok(expr),
+            _ => Err(DataFusionError::Internal(
+                "Failed to pop can_evaluate".to_string(),
+            )),
         }
     }
 }
@@ -549,7 +553,7 @@ impl<'a, S: SimplifyInfo> ExprRewriter for Simplifier<'a, 
S> {
                 op: Eq,
                 right,
             } if is_bool_lit(&left) && info.is_boolean_type(&right)? => {
-                match as_bool_lit(*left) {
+                match as_bool_lit(*left)? {
                     Some(true) => *right,
                     Some(false) => Not(right),
                     None => lit_bool_null(),
@@ -563,7 +567,7 @@ impl<'a, S: SimplifyInfo> ExprRewriter for Simplifier<'a, 
S> {
                 op: Eq,
                 right,
             } if is_bool_lit(&right) && info.is_boolean_type(&left)? => {
-                match as_bool_lit(*right) {
+                match as_bool_lit(*right)? {
                     Some(true) => *left,
                     Some(false) => Not(left),
                     None => lit_bool_null(),
@@ -582,7 +586,7 @@ impl<'a, S: SimplifyInfo> ExprRewriter for Simplifier<'a, 
S> {
                 op: NotEq,
                 right,
             } if is_bool_lit(&left) && info.is_boolean_type(&right)? => {
-                match as_bool_lit(*left) {
+                match as_bool_lit(*left)? {
                     Some(true) => Not(right),
                     Some(false) => *right,
                     None => lit_bool_null(),
@@ -596,7 +600,7 @@ impl<'a, S: SimplifyInfo> ExprRewriter for Simplifier<'a, 
S> {
                 op: NotEq,
                 right,
             } if is_bool_lit(&right) && info.is_boolean_type(&left)? => {
-                match as_bool_lit(*right) {
+                match as_bool_lit(*right)? {
                     Some(true) => Not(left),
                     Some(false) => *left,
                     None => lit_bool_null(),