erratic-pattern commented on code in PR #10358:
URL: https://github.com/apache/datafusion/pull/10358#discussion_r1588462758
##########
datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs:
##########
@@ -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);
Review Comment:
Returning `(Expr, usize)` would be nice for testing. Especially if the
number of iterations becomes a configurable parameter since that opens up the
door for new simplification bugs; we might want to test that specifically X
number of iterations runs on a given expression.
--
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: github-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
