feat: run expression simplifier in a loop

datafusion/core/tests/simplification.rs

@@ -29,7 +29,7 @@ use datafusion_expr::logical_plan::builder::table_scan_with_filters;
 use datafusion_expr::simplify::SimplifyInfo;
 use datafusion_expr::{
     expr, table_scan, Cast, ColumnarValue, ExprSchemable, LogicalPlan,
-    LogicalPlanBuilder, ScalarUDF, Volatility,
+    LogicalPlanBuilder, Operator, ScalarUDF, Volatility,
 };
 use datafusion_functions::{math, string};
 use datafusion_optimizer::optimizer::Optimizer;
@@ -658,3 +658,18 @@ fn test_simplify_concat() {
     let expected = concat(vec![col("c0"), lit("hello rust"), col("c1")]);
     test_simplify(expr, expected)
 }
+
+#[test]
+fn test_simplify_iterations() {
+    let expr = binary_expr(
+        cast(now(), DataType::Int64),
+        Operator::Lt,
+        binary_expr(
+            cast(to_timestamp(vec![lit(0)]), DataType::Int64),
+            Operator::Plus,
+            lit(i64::MAX),
+        ),
+    );
+    let expected = lit(true);
+    test_simplify(expr, expected)
+}

datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs

@@ -92,9 +92,12 @@ pub struct ExprSimplifier<S> {
     /// Should expressions be canonicalized before simplification? Defaults to
     /// true
     canonicalize: bool,
+    /// Maximum number of simplifier iterations
+    max_simplifier_iterations: usize,
 }
 
 pub const THRESHOLD_INLINE_INLIST: usize = 3;
+pub const DEFAULT_MAX_SIMPLIFIER_ITERATIONS: usize = 3;
 
 impl<S: SimplifyInfo> ExprSimplifier<S> {
     /// Create a new `ExprSimplifier` with the given `info` such as an
@@ -107,6 +110,7 @@ impl<S: SimplifyInfo> ExprSimplifier<S> {
             info,
             guarantees: vec![],
             canonicalize: true,
+            max_simplifier_iterations: DEFAULT_MAX_SIMPLIFIER_ITERATIONS
         }
     }
 
@@ -181,24 +185,30 @@ impl<S: SimplifyInfo> ExprSimplifier<S> {
             expr = expr.rewrite(&mut Canonicalizer::new()).data()?
         }
 
-        // TODO iterate until no changes are made during rewrite
-        // (evaluating constants can enable new simplifications and
-        // simplifications can enable new constant evaluation)
-        // https://github.com/apache/datafusion/issues/1160
-        expr.rewrite(&mut const_evaluator)
-            .data()?
-            .rewrite(&mut simplifier)
-            .data()?
-            .rewrite(&mut guarantee_rewriter)
-            .data()?
-            // run both passes twice to try an minimize simplifications that we missed
-            .rewrite(&mut const_evaluator)
-            .data()?
-            .rewrite(&mut simplifier)
-            .data()?
+        let mut i = 0;
+        loop {
+            let result = expr.rewrite(&mut const_evaluator)?;
+            let mut transformed = result.transformed;
+            expr = result.data;
+
+            let result = expr.rewrite(&mut simplifier)?;
+            transformed |= result.transformed;
+            expr = result.data;
+
+            let result = expr.rewrite(&mut guarantee_rewriter)?;
+            transformed |= result.transformed;
+            expr = result.data;
+
             // shorten inlist should be started after other inlist rules are applied
-            .rewrite(&mut shorten_in_list_simplifier)
-            .data()
+            let result = expr.rewrite(&mut shorten_in_list_simplifier)?;
+            transformed |= result.transformed;
+            expr = result.data;
+
+            i += 1;
+            if !transformed || i >= self.max_simplifier_iterations {
+              return Ok(expr);
+            }
+        }
     }
 
     /// Apply type coercion to an [`Expr`] so that it can be