andygrove opened a new issue, #2601:
URL: https://github.com/apache/datafusion-ballista/issues/2601

   **Is your feature request related to a problem or challenge? Please describe 
what you are trying to do.**
   
   [`SelectJoinRule::optimize` 
(aqe/optimizer_rule/join_selection.rs#L192-L435)](https://github.com/apache/datafusion-ballista/blob/ea1126c0649b2bc08e1e233b1972b64318821c16/ballista/scheduler/src/state/aqe/optimizer_rule/join_selection.rs#L192-L435)
 is a 219-line function with cognitive complexity 77 and nesting depth 9. Every 
`JoinSelectionAction` arm (LateCollectLeft, CollectLeft, StageBuildSide, 
Repartition, Hash, Sort) is inlined inside a `transform_up` closure under two 
`if let`s and the `adaptive_join_enabled` check, each ending in an identical 
`Ok(Transformed::no(node))`.
   
   Fragments are copy-pasted across arms:
   
   - `!null_aware && supports_swap_join_order(...)` at about L212, L270 and 
L406;
   - the `downcast_ref::<ExchangeExec>().ok_or(...)?.to_broadcast(plan_id)` 
sequence at about L221 and L247;
   - swap-then-CollectLeft at about L243 and L275;
   - `replace_children(vec![l, r], ReplaceChildrenOptions::new(Recompute))` 
four or more times.
   
   The CollectLeft arm also runs a second nested `transform_up` with a 
three-way if/else chain (about L281-L338). AQE join work keeps landing in this 
function, and each new strategy makes it deeper.
   
   **Describe the solution you'd like**
   
   - Turn the closure body into a method that returns early for non-dynamic or 
unresolved nodes (removing about three nesting levels), and early-return when 
`!adaptive_join_enabled()`.
   - Give each action its own method (`late_collect_left`, `collect_left`, 
`stage_build_side`, `repartition`, `hash`).
   - Add helpers `should_swap(&HashJoinExec)`, `broadcast_exchange(plan, 
plan_id)` and `with_children(plan, l, r)`.
   - Move the inner CollectLeft `transform_up` body into its own function.
   
   Keep behaviour identical; the existing tests in the file cover the rule.
   
   **Describe alternatives you've considered**
   
   None.
   
   **Additional context**
   
   Line references are pinned to commit ea1126c0649b.
   


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