erratic-pattern commented on code in PR #10358:
URL: https://github.com/apache/datafusion/pull/10358#discussion_r1588587257
##########
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:
> Yeah, that would probably be a good idea
I can put `shorten_in_list_simplifier` at the end of the loop, since that's
where it was previously. I am unsure where `guarantee_rewriter` is supposed to
go as it previously ran once inbetween the simplifier/constant evaluator
passes.
--
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
