andygrove opened a new issue, #26299: URL: https://github.com/apache/datafusion/issues/26299
### Is your feature request related to a problem or challenge? `EnforceDistribution` concentrates most of its decision-making in two long functions in [enforce_distribution.rs](https://github.com/apache/datafusion/blob/0d79252f61bd8777990b2749c216ffd07079c30d/datafusion/physical-optimizer/src/ensure_requirements/enforce_distribution.rs), which has had 21 commits in the last 12 months: - [`ensure_distribution_with_stats` (L1359-L1800)](https://github.com/apache/datafusion/blob/0d79252f61bd8777990b2749c216ffd07079c30d/datafusion/physical-optimizer/src/ensure_requirements/enforce_distribution.rs#L1359-L1800): cognitive complexity 82, cyclomatic 52, nesting depth 6. A large closure passed to `map()` (roughly L1445-L1595) computes the subset-satisfaction flags, file-partition preservation, round-robin decisions and the Range/Hash repartition choice inline. Range-partition scaling with its fallback is nested five levels deep inside the HashPartitioned arm. A second closure (about L1610-L1700) mixes ordering-requirement handling with distribution-dependent branching. - [`enforce_distribution_relationships` (L1120-L1323)](https://github.com/apache/datafusion/blob/0d79252f61bd8777990b2749c216ffd07079c30d/datafusion/physical-optimizer/src/ensure_requirements/enforce_distribution.rs#L1120-L1323): cognitive complexity 65. It mixes reference-child selection with the child rewrite that unwraps or wraps a `RepartitionExec` using `mem::replace`/`pop`. The boolean flags (`allow_subset_satisfy_partitioning`, `preserve_satisfying_file_partitioning`, `needs_repartition`, ...) interact subtly and cannot be tested on their own. Interactions in this rule have caused real bugs (for example the case referenced via #18341 and the partitioned-join panic described in a code comment). ### Describe the solution you'd like - Compute the per-child policy flags in a small `ChildPolicy` struct built by a dedicated function with unit tests. - Extract the per-child closure into `ensure_child_distribution(...)`, the Range scaling block into something like `scaled_or_fallback_partitioning(...)`, and the ordering closure into `enforce_child_ordering(...)`. - In `enforce_distribution_relationships`, extract `find_reference_child`, `partitioning_already_satisfied` and `insert_repartition` so the main loop only drives the fixpoint. Each step can be a behaviour-preserving PR relying on the existing `enforce_distribution` tests and plan snapshots. ### Describe alternatives you've considered None. The logic is correct today; the goal is to make the next change to it safe. ### Additional context Line references are pinned to commit 0d79252f61bd. -- 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]
