asolimando commented on code in PR #24716:
URL: https://github.com/apache/datafusion/pull/24716#discussion_r3896053438


##########
datafusion/physical-optimizer/src/join_selection.rs:
##########
@@ -152,19 +151,15 @@ impl PhysicalOptimizerRule for JoinSelection {
         plan: Arc<dyn ExecutionPlan>,
         context: &dyn PhysicalOptimizerContext,
     ) -> Result<Arc<dyn ExecutionPlan>> {
-        let config = context.config_options();
-        let registry = context.statistics_registry();
         let subrules: Vec<Box<PipelineFixerSubrule>> = vec![
             Box::new(hash_join_convert_symmetric_subrule),
             Box::new(hash_join_swap_subrule),
         ];
         let new_plan = plan
-            .transform_up(|p| apply_subrules(p, &subrules, config))
+            .transform_up(|p| apply_subrules(p, &subrules, 
context.config_options()))
             .data()?;
         new_plan
-            .transform_up(|plan| {
-                statistical_join_selection_subrule(plan, config, registry)
-            })
+            .transform_up(|plan| statistical_join_selection_subrule(plan, 
context))

Review Comment:
   Thanks for your review! It's indeed a good chance to improve coverage on 
`JoinSelection::optimize_with_context` itself, I have proposed a test along the 
line you proposed in 
https://github.com/apache/datafusion/pull/24716/commits/809b30b192992745b03fe4a82a441c70780a5c6d,
 let me know if that works for you.



-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to