alamb commented on code in PR #10358: URL: https://github.com/apache/datafusion/pull/10358#discussion_r1588557942
########## 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 -- 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