diff --git a/datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs b/datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs index 64b04c38ef6c..c87475ef19d7 100644 --- a/datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs +++ b/datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs @@ -1682,11 +1682,10 @@ impl<'a, S: SimplifyInfo> TreeNodeRewriter for Simplifier<'a, S> { fn has_common_conjunction(lhs: &Expr, rhs: &Expr) -> Result { let lhs: HashSet<&Expr> = iter_conjunction(lhs).collect(); iter_conjunction(rhs).try_fold(false, |acc, e| { - if lhs.contains(&e) { - let count = count_volatile_calls(e)?; - Ok::(acc || count == 0) + if lhs.contains(&e) && e.is_volatile()? { + Ok(false) } else { - Ok(acc) + Ok(acc || lhs.contains(&e)) } }) } diff --git a/datafusion/optimizer/src/simplify_expressions/utils.rs b/datafusion/optimizer/src/simplify_expressions/utils.rs index 3d9a7b659f17..dd824926c407 100644 --- a/datafusion/optimizer/src/simplify_expressions/utils.rs +++ b/datafusion/optimizer/src/simplify_expressions/utils.rs @@ -83,10 +83,7 @@ fn expr_contains_inner(expr: &Expr, needle: &Expr, search_op: Operator) -> bool /// check volatile calls and return if expr contains needle pub fn expr_contains(expr: &Expr, needle: &Expr, search_op: Operator) -> Result { - Ok( - expr_contains_inner(expr, needle, search_op) - && count_volatile_calls(needle)? == 0, - ) + Ok(expr_contains_inner(expr, needle, search_op) && !needle.is_volatile()?) } /// Deletes all 'needles' or remains one 'needle' that are found in a chain of xor @@ -219,7 +216,7 @@ pub fn is_false(expr: &Expr) -> bool { /// returns true if `haystack` looks like (needle OP X) or (X OP needle) pub fn is_op_with(target_op: Operator, haystack: &Expr, needle: &Expr) -> Result { Ok( - matches!(haystack, Expr::BinaryExpr(BinaryExpr { left, op, right }) if op == &target_op && (needle == left.as_ref() || needle == right.as_ref()) && count_volatile_calls(needle)? == 0), + matches!(haystack, Expr::BinaryExpr(BinaryExpr { left, op, right }) if op == &target_op && (needle == left.as_ref() || needle == right.as_ref()) && !needle.is_volatile()?), ) } @@ -355,49 +352,3 @@ pub fn distribute_negation(expr: Expr) -> Expr { _ => Expr::Negative(Box::new(expr)), } } - -struct VolatileFunctionCounter { - counter: usize, -} - -impl VolatileFunctionCounter { - pub fn get_count(&self) -> usize { - self.counter - } - - pub fn new() -> Self { - Self { counter: 0 } - } -} - -impl<'n> TreeNodeVisitor<'n> for VolatileFunctionCounter { - type Node = Expr; - fn f_up( - &mut self, - expr: &'n Self::Node, - ) -> Result { - match expr { - Expr::ScalarFunction(func) - if matches!(func.func.signature().volatility, Volatility::Volatile) => - { - self.counter += 1; - } - _ => {} - } - Ok(datafusion_common::tree_node::TreeNodeRecursion::Continue) - } - - fn f_down( - &mut self, - _node: &'n Self::Node, - ) -> Result { - Ok(datafusion_common::tree_node::TreeNodeRecursion::Continue) - } -} - -// get the number of volatile call in a expression -pub fn count_volatile_calls(expr: &Expr) -> Result { - let mut volatile_visitor = VolatileFunctionCounter::new(); - expr.visit(&mut volatile_visitor)?; - Ok(volatile_visitor.get_count()) -} diff --git a/datafusion/sqllogictest/test_files/explain.slt b/datafusion/sqllogictest/test_files/explain.slt index 5f023bb8f34e..3e42b7edc63f 100644 --- a/datafusion/sqllogictest/test_files/explain.slt +++ b/datafusion/sqllogictest/test_files/explain.slt @@ -436,3 +436,18 @@ physical_plan 01)CoalesceBatchesExec: target_batch_size=8192 02)--FilterExec: column1@0 = 2 03)----ValuesExec + + +query TT +explain select * from VALUES (1), (2) where column1 = 2 AND random() = 0 OR column1 = 2 AND random() = 0; +---- +logical_plan +01)Projection: column1 +02)--Filter: __common_expr_4 AND random() = Float64(0) OR __common_expr_4 AND random() = Float64(0) +03)----Projection: column1 = Int64(2) AS __common_expr_4, column1 +04)------Values: (Int64(1)), (Int64(2)) +physical_plan +01)CoalesceBatchesExec: target_batch_size=8192 +02)--FilterExec: __common_expr_4@0 AND random() = 0 OR __common_expr_4@0 AND random() = 0, projection=[column1@1] +03)----ProjectionExec: expr=[column1@0 = 2 as __common_expr_4, column1@0 as column1] +04)------ValuesExec