Skip to content

Add config-matrix tests in enforce_distribution.rs for range-satisfaction settings#23627

Draft
blinding-pixels wants to merge 1 commit into
apache:mainfrom
blinding-pixels:agent/range-satisfaction-config-matrix
Draft

Add config-matrix tests in enforce_distribution.rs for range-satisfaction settings#23627
blinding-pixels wants to merge 1 commit into
apache:mainfrom
blinding-pixels:agent/range-satisfaction-config-matrix

Conversation

@blinding-pixels

Copy link
Copy Markdown

Which issue does this PR close?

Rationale for this change

The existing SQL logic tests exercise Range reuse through aggregates, joins, and windows. Repeating the same configuration permutations for every operator would make those tests large and difficult to maintain.

Issue #23572 describes six wildcard rules across four dimensions:

  • Exact, subset, or incompatible keys
  • Subset threshold met or not met
  • Preserve-file threshold met or not met
  • Target partition count equal to or greater than the input count

Those rules directly specify 15 of the 24 possible combinations. The remaining nine expectations are derived from the documented semantics rather than recorded from optimizer output:

  • Exact Range keys satisfy the requirement directly in range_satisfies_key_partitioning.
  • Subset keys require allow_subset_satisfy_partitioning.
  • Subset satisfaction is enabled when the subset threshold is met, or when the preserve threshold is met and increasing the target partition count is the only reason to repartition.
  • Incompatible keys never satisfy the requirement.

This explains why these cases have different outcomes:

  • Exact key, threshold not met, target equal: reuse
  • Subset key, threshold not met, preserve not met, target equal: hash

The equal partition count is not a global reuse shortcut. The input must first satisfy the distribution requirement. Exact keys satisfy directly, while subset keys require permission.

What changes are included in this PR?

The matrix uses one RangeSatisfactionConfigCase structure and eight configuration rows:

  • Two subset-threshold states
  • Two preserve-threshold states
  • Two target-partition states

Each row contains explicit expected results for exact, subset, and incompatible keys, producing all 24 combinations.

For every combination, the test:

  1. Creates the same four-partition Range input.
  2. Applies the selected key relationship and optimizer configuration.
  3. Runs the real physical optimizer.
  4. Checks whether the optimized plan contains a hash repartition.

The expected values remain explicit in the table rather than being calculated using a second implementation of the optimizer logic.

I also added KeyPartitioningRequirementExec under the existing physical-plan test utilities. It is a neutral single-input operator that requests keyed partitioning and enables the existing Range opt-in. This lets the matrix test the shared decision without aggregate, join, or window behavior affecting the result.

I considered several test harnesses before using the neutral operator:

  • An aggregate harness introduced aggregate-specific round-robin repartitioning.
  • A window harness worked but coupled the shared matrix to behavior already tested by window-specific cases.
  • OutputRequirementExec and the existing MockReqExec request ordinary keyed distributions but cannot enable the crate-private Range opt-in.

Are these changes tested?

Yes.

The following checks pass:

  • The complete 24-combination configuration matrix
  • All related Range physical-optimizer tests
  • The complete range_partitioning.slt file
  • Formatting and diff checks
  • Targeted Clippy checks

Matrix mutation testing

The nine expectations not directly specified by the issue were mutation-tested against their decision branches:

Mutation location and edit Result
datafusion/physical-plan/src/distribution_requirements.rs:459: changed PartitioningSatisfaction::Exact to PartitioningSatisfaction::NotSatisfied All three derived exact-key cells failed, along with the issue-specified exact cases using the same rule
datafusion/physical-optimizer/src/ensure_requirements/enforce_distribution.rs:1305: appended && false to allow_subset_satisfy_partitioning All five derived subset cells expecting reuse failed: the four threshold-met combinations and the preserve-met/greater-target combination
datafusion/physical-optimizer/src/ensure_requirements/enforce_distribution.rs:1303: changed current_partitions < target_partitions to current_partitions <= target_partitions Exactly one cell failed: subset key, threshold not met, preserve met, target equal

The final mutation changed one character and flipped only one matrix cell. This shows that the matrix distinguishes the preserve/target boundary rather than several cells merely depending on the same broad condition.

All mutations were restored, and the complete matrix passed again.

Operator opt-in mutation testing

I also independently removed each production Range opt-in and ran range_partitioning.slt:

Mutated operator and location SLT failures
AggregateExecdatafusion/physical-plan/src/aggregates/mod.rs:1952 5 — range_partitioning.slt:41, :101, :137, :218, and :551
HashJoinExecdatafusion/physical-plan/src/joins/hash_join/exec.rs:1297 5 — range_partitioning.slt:280, :476, :514, :551, and :607
BoundedWindowAggExecdatafusion/physical-plan/src/windows/bounded_window_agg_exec.rs:336 2 — range_partitioning.slt:701 and :821
WindowAggExecdatafusion/physical-plan/src/windows/window_agg_exec.rs:244 1 — range_partitioning.slt:730

Each mutation introduced unexpected hash repartitions. Line 551 protects both AggregateExec and HashJoinExec.

This confirms that the two test layers protect different failure points:

  • The neutral matrix protects the shared configuration decision.
  • The SLT cases protect each operator’s opt-in.

When the SLT permutations are slimmed, the load-bearing operator cases above should remain. I would appreciate guidance on whether they should be marked as contract tests in place or moved into a dedicated section or file.

Are there any user-facing changes?

No query-planning or runtime behavior changes are included.

The only new exposed type is a test utility under the existing datafusion_physical_plan::test module. I did not introduce a public Range-policy API because the current opt-in is documented as a temporary bridge pending #23266.

@github-actions github-actions Bot added core Core DataFusion crate physical-plan Changes to the physical-plan crate labels Jul 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Core DataFusion crate physical-plan Changes to the physical-plan crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add config-matrix tests in enforce_distribution.rs for range-satisfaction settings

1 participant