Skip to content

Commit

Permalink
Remove panics from simplify_expressions optimizer rule (apache#3343)
Browse files Browse the repository at this point in the history
  • Loading branch information
andygrove authored and kmitchener committed Sep 4, 2022
1 parent 9ab7377 commit 312b95e
Showing 1 changed file with 16 additions and 12 deletions.
28 changes: 16 additions & 12 deletions datafusion/optimizer/src/simplify_expressions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
))),
}
}

Expand Down Expand Up @@ -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(),
)),
}
}
}
Expand Down Expand Up @@ -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(),
Expand All @@ -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(),
Expand All @@ -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(),
Expand All @@ -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(),
Expand Down

0 comments on commit 312b95e

Please sign in to comment.