andygrove opened a new pull request, #2086:
URL: https://github.com/apache/datafusion-ballista/pull/2086

   # Which issue does this PR close?
   
   Closes #2085.
   
   # Rationale for this change
   
   `ballista.optimizer.broadcast_join_threshold_bytes` (default 10 MiB) is only 
consumed by the static distributed planner 
(`DefaultDistributedPlanner::maybe_promote_to_broadcast`). When adaptive query 
planning is enabled (`ballista.planner.adaptive.enabled=true`), broadcast (BHJ 
/ `CollectLeft`) selection instead happens in 
`DynamicJoinSelectionExec::to_actual_join`, which used DataFusion's 
`optimizer.hash_join_single_partition_threshold` (default 1 MiB). As a result 
the Ballista key had no effect under AQE, and the effective broadcast cutoff 
was silently a different, DataFusion-namespaced value. This makes the one 
Ballista broadcast-threshold config behave consistently under both planners.
   
   # What changes are included in this PR?
   
   - `DynamicJoinSelectionExec::to_actual_join` now reads the byte threshold 
from `BallistaConfig::broadcast_join_threshold_bytes()` instead of DataFusion's 
`hash_join_single_partition_threshold`.
   - DataFusion's `hash_join_single_partition_threshold_rows` is kept as the 
row threshold, which is only used as a fallback when byte statistics are absent.
   - A threshold of `0` disables broadcast promotion entirely, matching the 
static planner. The `!= 0` guard is applied at the `under_threshold` level so 
it disables both the byte and row-fallback broadcast paths.
   - Updated the config-entry description to note the key governs broadcast 
selection under both the static planner and AQE.
   - Added unit tests: a side under the threshold collects; a threshold set 
below the side size repartitions (proving the Ballista value drives the cut); 
`0` disables broadcast even for a tiny side.
   
   The static planner path (`maybe_promote_to_broadcast`) is unchanged. The 
separate concern that shuffle `total_byte_size` is an on-disk Arrow IPC size 
(rather than an in-memory build-side size) is intentionally left as a follow-up.
   
   # Are there any user-facing changes?
   
   Yes, a behavioral change when AQE is enabled: 
`ballista.optimizer.broadcast_join_threshold_bytes` now takes effect 
(previously ignored). Under AQE the default broadcast byte cutoff moves from 
DataFusion's 1 MiB to Ballista's 10 MiB default. No public API changes.
   


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