Skip to content

Break up ensure_distribution_with_stats and enforce_distribution_relationships in EnforceDistribution #26299

Open

Description

@andygrove

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, which has had 21 commits in the last 12 months:

  • ensure_distribution_with_stats (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): 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 0d79252.

Metadata

Metadata

Assignees

Labels

enhancementNew feature or requesthelp wantedExtra attention is neededoptimizerOptimizer rulestech debtCode complexity, duplication or structure that makes the codebase harder to change

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions