erratic-pattern commented on code in PR #10358:
URL: https://github.com/apache/datafusion/pull/10358#discussion_r1588555626
##########
datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs:
##########
@@ -181,24 +189,19 @@ 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()?
- // shorten inlist should be started after other inlist rules are
applied
- .rewrite(&mut shorten_in_list_simplifier)
- .data()
+ let mut num_iterations = 0;
+ loop {
+ let Transformed { data, transformed, .. } = expr
+ .rewrite(&mut const_evaluator)?
+ .transform_data(|expr| expr.rewrite(&mut simplifier))?
+ .transform_data(|expr| expr.rewrite(&mut guarantee_rewriter))?
Review Comment:
`guarantee_rewriter` and `shorten_in_list_simplifier` only ran once in the
previous version of this code. Should we continue only running them once here?
--
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
