feat: allow Full joins to reuse range co-partitioning in HashJoinExec#23583
Open
mattp5657 wants to merge 3 commits into
Open
feat: allow Full joins to reuse range co-partitioning in HashJoinExec#23583mattp5657 wants to merge 3 commits into
mattp5657 wants to merge 3 commits into
Conversation
github-actions
Bot
added
core
mattp5657
changed the title
gene-bordegaray
left a comment
gene-bordegaray
left a comment
Contributor
There was a problem hiding this comment.
this looks good, just rebase and ping me for a re-review before I approve 👍
mattp5657
force-pushed
the
feat/full-join-range-copartitioning
branch
from
d2ca127 to
8e7a4da
Compare
Contributor
Author
|
@gene-bordegaray This should be ready. |
gene-bordegaray
approved these changes
Jul 16, 2026
gene-bordegaray
left a comment
gene-bordegaray
left a comment
Contributor
There was a problem hiding this comment.
this should be good after next addresses 👍
| # TEST 12: Non-Inner Range Join Repartitions | ||
| # Only inner partitioned hash joins opt in to Range satisfying KeyPartitioned | ||
| # requirements. Non-inner joins keep using Hash repartitioning. | ||
| # TEST 12: Non-Inner, Non-Full Range Join Repartitions |
Contributor
There was a problem hiding this comment.
lets just have this say "Unsupported"
| 02)--RepartitionExec: partitioning=Hash([range_key@0], 4), input_partitions=4 | ||
| 03)----DataSourceExec: file_groups=<slt:ignore>, projection=[range_key, value], output_partitioning=Range([range_key@0 ASC], [(10), (20), (30)], 4), file_type=csv, has_header=false | ||
| 04)--RepartitionExec: partitioning=Hash([range_key@0], 4), input_partitions=4 | ||
| 05)----DataSourceExec: file_groups=<slt:ignore>, projection=[range_key, value], output_partitioning=Range([range_key@0 ASC], [(15), (20), (30)], 4), file_type=csv, has_header=false |
Contributor
There was a problem hiding this comment.
can we add query results
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
Partitioning::Rangeinputs for full hash joins #23454.Rationale for this change
#23184 let compatible range-partitioned inputs satisfy inner partitioned hash joins without repartitioning. Full partitioned equi joins still always went through the conservative hash-repartition path, even when both inputs were already co-partitioned by range on the join key(s). The per-partition unmatched-row tracking in
HashJoinExecis already partition-local underPartitionMode::Partitioned(not shared globally like inCollectLeft), so Full-join semantics generalize cleanly to range co-partitioning with no additional bookkeeping required.What changes are included in this PR?
HashJoinExec::input_distribution_requirements()to optJoinType::Fullin toallow_range_satisfaction_for_key_partitioning(), alongside the existingJoinType::Innercase. The underlyingco_partitioned/compatible_co_partitioning_layout/co_partitioning_satisfiedlogic indistribution_requirements.rswas already join-type-agnostic, so no changes were needed there.enforce_distribution.rscovering both the compatible-layout case (no repartition inserted) and the incompatible-split-points case (repartition still inserted) forJoinType::Full.range_partitioned_sparsesqllogictest fixture table with the same partition layout asrange_partitionedbut only partially overlapping keys, and add sqllogictest coverage inrange_partitioning.sltfor: a compatible Full join avoiding repartition, an incompatible Full join still repartitioning, and matched/left-only/right-only unmatched rows produced correctly by a co-partitioned Full join.Are these changes tested?
Yes:
datafusion/core/tests/physical_optimizer/enforce_distribution.rsassert on the physical plan shape (repartition inserted or not) for compatible and incompatible range layouts.datafusion/sqllogictest/test_files/range_partitioning.sltexercise the feature end-to-end against real data, including matched rows, left-only unmatched rows, and right-only unmatched rows for a Full outer join.enforce_distributiontests,range_partitioning.slt, and proto roundtrip tests all continue to pass.Are there any user-facing changes?
Yes:
EXPLAINoutput forFulljoins over compatible range-partitioned inputs will no longer show aRepartitionExec, and such queries will avoid the associated hash-shuffle cost at execution time. No public API changes.