feat: support co-partitioned range right-side equi hash joins by gmhelmold · Pull Request #23484 · apache/datafusion

gmhelmold · 2026-07-11T18:47:19Z

Which issue does this PR close?

Rationale for this change

#23184 let compatible range-partitioned inputs satisfy inner partitioned hash joins without repartitioning. Right-side partitioned equi joins have the same locality guarantee: Right, RightSemi, RightAnti and RightMark all anchor every output row on the probe (right) partition (on_lr_is_preserved is probe-side for all four), so when both inputs are co-partitioned by the join keys, execution stays partition-local and the hash repartition is unnecessary.

This removes unnecessary RepartitionExecs for already-co-located inputs, extending the inner-join behavior from #23184 to the right-side variants. No micro-benchmark included, consistent with #23184; correctness is demonstrated by matched/unmatched execution results below.

What changes are included in this PR?

The only production change is widening the existing inner-only gate in HashJoinExec::input_distribution_requirements from join_type == JoinType::Inner to matches!(join_type, Inner | Right | RightSemi | RightAnti | RightMark) (under PartitionMode::Partitioned). The co_partitioned / range-satisfaction machinery from #23184 is unchanged — the sanity checker and enforce_distribution consume range satisfaction join-type-agnostically.

One behavior note for #23376: range co-partitioned right joins now stay Range/Range, so partitioned dynamic filters are disabled for them (has_partitioned_dynamic_filter_routing returns false), the same safe delta #23184 introduced for inner joins. These variants are probe-not-preserved for pruning, so dynamic filters were not eligible to prune their probe rows regardless.

Are these changes tested?

Yes.

Optimizer (enforce_distribution.rs): 4 reuse tests (one per join type) proving compatible range/range inputs keep Range partitioning with no RepartitionExec; 4 incompatibility tests proving that mismatched split points, sort options, partition counts, or join-key expressions still insert a hash repartition; sanity-check pairs mirroring Support co-partitioned range inner equi joins #23184.
Execution (range_partitioning.slt): EXPLAIN plan pins plus matched/unmatched result checks for Right (incl. NULL left values), RightSemi, RightAnti, and an incompatible-layout Right join (repartitions, correct results).
The new reuse tests fail on main without the gate change (verified by reverting the production diff: exactly the 4 reuse tests fail, everything else passes).
RightMark testing note: RightMark is not reachable from SQL in sqllogictest (IN-subquery decorrelation emits LeftMark; physical RightMark only appears via a statistics-based swap), so its co-partitioning behavior is pinned at the optimizer/plan level, and mark null-marker semantics (matched/unmatched/NULL build keys) are pinned via the LeftMark path in the slt.

Are there any user-facing changes?

No API changes. Plans over compatible range-partitioned inputs avoid a hash repartition for right-side equi hash joins.

gmhelmold · 2026-07-13T14:44:16Z

Done in 852a619: the end-to-end matrix now lives in range_partitioning.slt (reuse cases for Right/RightSemi/RightAnti + rehash cases for split-points, partition-count, and non-range join keys — the last two needed one small narrow-fixture addition to the harness). Kept two unit tests in enforce_distribution.rs with comments explaining why they can't move: RightMark plans aren't SQL-producible, and the sort-direction axis needs a descending Range input the SLT fixtures don't declare. Net: unit block 8→2, no scenario lost.

gene-bordegaray

few nits but this is great, thank you. Feel free to ping on another round 😄

gene-bordegaray · 2026-07-14T12:41:42Z

+    // Same rows as `range_partitioned` but split into only three range
+    // partitions on `range_key`. Used to exercise the co-partition check when
+    // two Range inputs disagree on partition count.
+    let narrow_output_partitioning = Partitioning::Range(


I like it 👍

gene-bordegaray · 2026-07-14T14:50:56Z

+NULL 35 350
+
+##########
+# TEST 22: Mark Join Marker Semantics over Range Partitioned Inputs


nit: can we move this as the last test in this block?

Done — moved to the end of the partitioned-hash-join block (now the last test there, before the SMJ/SHJ cases that came in with the main merge).

gene-bordegaray · 2026-07-14T14:53:50Z

+physical_plan
+01)FilterExec: non_range_key@1 = 2 OR mark@3, projection=[range_key@0, value@2]
+02)--HashJoinExec: mode=Partitioned, join_type=LeftMark, on=[(range_key@0, range_key@0)]
+03)----RepartitionExec: partitioning=Hash([range_key@0], 4), input_partitions=4


to be clear this should disappear after #23453 ? Just putting here to show reationale on why the unit is good

Good question — no, this one stays. This PR is the implementation of #23453 (it closes it), and the plan pinned here is already generated with the change applied: the range-satisfaction gate in HashJoinExec::input_distribution_requirements covers Inner | Right | RightSemi | RightAnti | RightMark only, and SQL IN-subquery decorrelation produces a LeftMark join — a left-anchored type intentionally outside this PR's covered set — so its inputs keep the Hash repartition path. And exactly right on the rationale: since RightMark is not reachable from SQL (it only appears when a statistics-based swap flips a LeftMark), its reuse behavior is pinned by the enforce_distribution.rs unit test, while this case pins the mark-marker semantics end-to-end on the SQL-reachable path. It would only change if co-partition reuse is later extended to the left-anchored variants under the #22395 umbrella — happy to file that follow-up if there's interest.

gene-bordegaray · 2026-07-14T14:54:23Z

 30 600
 35 700

+##########


can we add one subset test for sanity check 👍

Added TEST 22: a RIGHT JOIN on the composite key (range_key, non_range_key) where Range([range_key]) covers only a subset — pins that, unlike aggregates (TEST 3), subset satisfaction stays disabled for partitioned joins, so both sides Hash-repartition on the full key even with the threshold met.

gmhelmold · 2026-07-16T19:37:17Z

Round done! All three nits addressed (mark-semantics test moved to the end of its block; TEST 22 added as the composite-key subset sanity check; rationale for the LeftMark repartition answered inline) — plus the branch is synced with main twice over (absorbed the SMJ/SHJ co-partition tests, #23584's plan changes, and #23623's InterleaveExec cases; full suite green, 98 unit + 44 SLT). Ready for another look @gene-bordegaray 😄

gene-bordegaray

sorry, trying to keep files clean and clear. Few more things 👍

gene-bordegaray · 2026-07-17T20:50:56Z

+physical_plan
+01)HashJoinExec: mode=Partitioned, join_type=RightAnti, on=[(range_key@0, range_key@0)]
+02)--FilterExec: value@1 <= 150, projection=[range_key@0]
+03)----DataSourceExec: file_groups={4 groups: [[WORKSPACE_ROOT/datafusion/sqllogictest/test_files/scratch_range_partitioning/range_partitioned/part-0.csv], [WORKSPACE_ROOT/datafusion/sqllogictest/test_files/scratch_range_partitioning/range_partitioned/part-1.csv], [WORKSPACE_ROOT/datafusion/sqllogictest/test_files/scratch_range_partitioning/range_partitioned/part-2.csv], [WORKSPACE_ROOT/datafusion/sqllogictest/test_files/scratch_range_partitioning/range_partitioned/part-3.csv]]}, projection=[range_key, value], output_partitioning=Range([range_key@0 ASC], [(10), (20), (30)], 4), file_type=csv, has_header=false


can we ignore files 👍

gene-bordegaray · 2026-07-17T20:51:40Z

 ##########
-# TEST 18: Sort Merge Join Avoids Repartition for Compatible Range Inputs
+# TEST 18: Right Join on Range Partition Column
+# Right-side partitioned hash joins also opt in to Range satisfying


lets keep descriptions generic so they remain valid after the PR merges

gene-bordegaray · 2026-07-17T20:52:32Z

+
+##########
+# TEST 22: Right Join on Composite Key With Range Subset Rehashes
+# The join key (range_key, non_range_key) is a strict superset of the exposed


coudl we keep explanations concise 👍

gene-bordegaray · 2026-07-17T20:53:27Z

+50 35 350
+
+##########
+# TEST 25: Mark Join Marker Semantics over Range Partitioned Inputs


lets just keep descriptions specific to what this test is showing. Not mentioning other things in teh codebase

please replace with something like:

# TEST 25: Mark Join Marker Semantics # Mark joins preserve matched, unmatched, and NULL-key marker behavior over # range-partitioned inputs.

Use file_groups=<slt:ignore> for the remaining plan assertions so the file no longer pins absolute scratch paths, matching the rest of the file. Trim the TEST 18/22/25 descriptions to be concise and specific to what each test shows, without cross-references or PR-relative wording.

gmhelmold · 2026-07-17T21:10:45Z

All addressed in 04e31b2 👍

Ignore files: the remaining plan assertions now use file_groups=<slt:ignore>, so the file no longer pins absolute scratch paths (matches the rest of the file).
Descriptions: trimmed TEST 18/22/25 to be concise and specific to what each test shows — no cross-references or PR-relative wording.

Verified locally: cargo test -p datafusion-sqllogictest --test sqllogictests -- range_partitioning passes.

gene-bordegaray

thanks, one last thing 👍

A descending Range requires descending-ordered split points, so the right input's [30, 20, 10] DESC encodes the same partition boundaries as the left's [10, 20, 30] ASC -- only the sort direction differs. Add a comment so the test's intent (isolating incompatible sort options) is clear to future readers.

gmhelmold · 2026-07-19T13:29:25Z

Good catch that this deserves clarifying 👍 I tried the metadata-only change first, but a descending Range requires descending-ordered split points — [10, 20, 30] with descending: true fails constructor validation:

Range partitioning split points must be strictly ordered: split point 0 ((10)) must be less than split point 1 ((20))

So [30, 20, 10] DESC isn't a second, independent flip — it's the same partition boundaries (10/20/30) as the left's [10, 20, 30] ASC, just encoded in the sort's order. The only real difference between the two inputs is the sort direction, which is exactly the incompatibility under test. Added a comment above the right input in 64aba77 spelling that out for the next reader. Happy to switch to a different construction if you'd prefer.

gene-bordegaray

These are all non-blockinng.

Thank you for the contribution 🙇 . I kindly ask if you are using AI tools to aid coding to review the comments and output you are getting. It is important to keep comments and documentation clear in the codebase for future readers and maintainability

cc: @gabotechs for when these are fixed

gene-bordegaray · 2026-07-20T13:32:53Z

+// Kept as a unit test: `RightMark` join plans cannot be produced from SQL (they
+// only arise when a statistical swap flips a `LeftMark` join around), so this


Lets try to keep very PR specific comments out of the code.

Usually phrases like this "Kept as a unit test: ..." does not have any value outside of the scopoe of this PR becuase a new reaader will not have the ocntext that you or I have about this test. Rather lets omit the comments and allow the code to do the talking 👍

gene-bordegaray · 2026-07-20T13:33:01Z

+    Ok(())
+}
+
+// Kept as a unit test: a descending Range input cannot be registered through the


gene-bordegaray · 2026-07-20T13:34:15Z

+    // A descending Range requires descending-ordered split points (the constructor rejects
+    // ascending points under a descending sort), so [30, 20, 10] DESC encodes the same
+    // partition boundaries as the left's [10, 20, 30] ASC -- only the sort direction differs,
+    // which is the incompatibility under test.


ok I think we got confused here. We are specifically tring to isolate incompatible sort options based on metadata. To isolate this is would be best to use a single split point and just change the descending metadata rather than both 👍

this test would still fail if the sort otpions check was removed

And I don't think this warrants a long comments

gene-bordegaray · 2026-07-20T13:37:58Z

+NULL 35 350
+
+##########
+# TEST 22: Right Join on Composite Key With Range Subset Rehashes


please resplace with something like:

# TEST 22: Composite-Key Right Join Repartitions # Range([range_key]) does not satisfy a partitioned join on # (range_key, non_range_key), so both sides repartition on the full key.

gene-bordegaray · 2026-07-20T13:38:36Z

+50 35 350
+
+##########
+# TEST 25: Mark Join Marker Semantics over Range Partitioned Inputs


please replace with something like:

# TEST 25: Mark Join Marker Semantics # Mark joins preserve matched, unmatched, and NULL-key marker behavior over # range-partitioned inputs.

Remove the 'Kept as a unit test' comments and let the code speak. Isolate the incompatible-sort-options case with a single split point and only the descending flag flipped, and tighten the TEST 22/25 slt descriptions.

gmhelmold · 2026-07-20T17:53:42Z

Thanks for the careful review, and fair point on the comments 🙏 You're right that phrasing like "Kept as a unit test: …" only carries meaning with the context you and I have — a future reader doesn't have it. Stripped those and let the code speak. In 82a68b9:

Removed the PR-specific comments on both unit tests.
Restructured the sort-options test to a single split point with only the descending flag flipped, as you suggested — it's self-explanatory now, and still fails if the sort-options check is removed.
Tightened the TEST 22 / TEST 25 descriptions to your wording.

I'll keep comments lean and reader-focused going forward — thanks for pushing on clarity.

gene-bordegaray · 2026-07-20T18:09:20Z

could you merge this with origin main. I want to make sure it is correct witht the recent changes in #23454 🙇

…ge-copartition # Conflicts: # datafusion/physical-plan/src/joins/hash_join/exec.rs # datafusion/sqllogictest/src/test_context/range_partitioning.rs # datafusion/sqllogictest/test_files/range_partitioning.slt

gmhelmold · 2026-07-20T18:34:16Z

Merged upstream/main (5b816da). The overlap with #23583 was the co-partition allowlist — reconciled it to the union: Inner | Full | Right | RightSemi | RightAnti | RightMark, so the Full-join reuse and the right-side reuse coexist. Kept both range fixtures (narrow + sparse) and renumbered the slt tests sequentially. range_partitioning.slt and the enforce_distribution tests pass locally.

gene-bordegaray · 2026-07-20T19:16:34Z

thank you, this is good now cc: @gabotechs

gabotechs

Thanks @gmhelmold and @gene-bordegaray!

codecov-commenter · 2026-07-21T06:12:26Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.70%. Comparing base (4184b07) to head (5b816da).
⚠️ Report is 7 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #23484   +/-   ##
=======================================
  Coverage   80.70%   80.70%           
=======================================
  Files        1089     1089           
  Lines      368038   368061   +23     
  Branches   368038   368061   +23     
=======================================
+ Hits       297031   297055   +24     
+ Misses      53308    53305    -3     
- Partials    17699    17701    +2

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

feat: support co-partitioned range right-side equi hash joins

c4981a0

github-actions Bot added core Core DataFusion crate sqllogictest SQL Logic Tests (.slt) physical-plan Changes to the physical-plan crate labels Jul 11, 2026

Merge branch 'main' into feat/right-join-range-copartition

190a6a6

gene-bordegaray reviewed Jul 13, 2026

View reviewed changes

Comment thread datafusion/core/tests/physical_optimizer/enforce_distribution.rs Outdated

test: move range co-partitioning coverage to range_partitioning.slt

852a619

gene-bordegaray reviewed Jul 14, 2026

View reviewed changes

JSOD11 mentioned this pull request Jul 15, 2026

feat(physical-plan): Allow co-partitioned Partitioning::Range inputs for left-side hash joins #23487

Draft

gmhelmold added 3 commits July 16, 2026 14:02

test: address review nits in range_partitioning.slt

a3c950a

Merge branch 'main' into feat/right-join-range-copartition

8d63244

Merge branch 'main' into feat/right-join-range-copartition

17566a7

gene-bordegaray reviewed Jul 17, 2026

View reviewed changes

gene-bordegaray approved these changes Jul 19, 2026

View reviewed changes

gene-bordegaray approved these changes Jul 20, 2026

View reviewed changes

test: drop PR-specific comments and simplify sort-options test

82a68b9

Remove the 'Kept as a unit test' comments and let the code speak. Isolate the incompatible-sort-options case with a single split point and only the descending flag flipped, and tighten the TEST 22/25 slt descriptions.

Merge remote-tracking branch 'upstream/main' into feat/right-join-ran…

5b816da

…ge-copartition # Conflicts: # datafusion/physical-plan/src/joins/hash_join/exec.rs # datafusion/sqllogictest/src/test_context/range_partitioning.rs # datafusion/sqllogictest/test_files/range_partitioning.slt

gabotechs approved these changes Jul 21, 2026

View reviewed changes

		// Kept as a unit test: `RightMark` join plans cannot be produced from SQL (they
		// only arise when a statistical swap flips a `LeftMark` join around), so this

Uh oh!

Conversation

gmhelmold commented Jul 11, 2026

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Uh oh!

Uh oh!

gmhelmold commented Jul 13, 2026

Uh oh!

gene-bordegaray left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gmhelmold commented Jul 16, 2026

Uh oh!

gene-bordegaray left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gmhelmold commented Jul 17, 2026

Uh oh!

gene-bordegaray left a comment

Choose a reason for hiding this comment

Uh oh!

gmhelmold commented Jul 19, 2026

Uh oh!

gene-bordegaray left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gmhelmold commented Jul 20, 2026

Uh oh!

gene-bordegaray commented Jul 20, 2026

Uh oh!

gmhelmold commented Jul 20, 2026

Uh oh!

gene-bordegaray commented Jul 20, 2026

Uh oh!

gabotechs left a comment

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented Jul 21, 2026