alamb commented on code in PR #9948:
URL: https://github.com/apache/arrow-datafusion/pull/9948#discussion_r1559194542
##########
datafusion/optimizer/src/test/mod.rs:
##########
@@ -150,22 +150,19 @@ pub fn assert_analyzer_check_err(
}
}
}
+
+fn observe(_plan: &LogicalPlan, _rule: &dyn OptimizerRule) {}
+
pub fn assert_optimized_plan_eq(
rule: Arc<dyn OptimizerRule + Send + Sync>,
- plan: &LogicalPlan,
+ plan: LogicalPlan,
expected: &str,
) -> Result<()> {
- let optimizer = Optimizer::with_rules(vec![rule.clone()]);
- let optimized_plan = optimizer
- .optimize_recursively(
- optimizer.rules.first().unwrap(),
- plan,
- &OptimizerContext::new(),
- )?
- .unwrap_or_else(|| plan.clone());
+ // Apply the rule once
+ let opt_context = OptimizerContext::new().with_max_passes(1);
Review Comment:
My explanation goes like: The loop that applies the rule more than once
calls `optimize_recursively` each time
https://github.com/apache/arrow-datafusion/blob/75c399ce7d4d5360140c64089dd7b05ffd7c49ef/datafusion/optimizer/src/optimizer.rs#L298-L303
This test only called `optimze_recursively` once (directly) and thus the
`OptimizeRule` is only applied once
When I rewrote the test to use `Optimizer::optimize` the loop will now kick
in and so the `OptimizeRule` will be run several times unless we set
`with_max_passes`
This same reasoning applies to the other tests, but apparently they get the
same answer when applied more than once
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
