Support co-partitioned range inner equi joins#23184
Conversation
|
Thank you for opening this pull request! Reviewer note: cargo-semver-checks reported the current version number is not SemVer-compatible with the changes in this pull request (compared against the base branch). Details |
| /// This is optimizer policy: partitioned joins require children that can be | ||
| /// paired by partition index. Inner hash joins can reuse compatible range | ||
| /// partitioning; otherwise the existing hash repartitioning policy applies. | ||
| fn partitioned_join_distribution( |
| /// left: Range(left.a ASC, [10, 20]), required KeyPartitioned(left.a) | ||
| /// right: Range(right.x ASC, [15, 20]), required KeyPartitioned(right.x) | ||
| /// ``` | ||
| pub fn co_partitioned_with( |
There was a problem hiding this comment.
I realized the concept was a bit off. I think this is ok if we haven't had a release... 😅
I linked the mirroring concepts that are used in trino in spark in the PR description
Lesson learned to have consumer of the public API first
|
cc: @gabotechs @stuhood |
8583303 to
45d598b
Compare
| # TEST 3b: Non-Range Join Repartitions | ||
| # The source Range partitioning does not satisfy a join on non_range_key, so | ||
| # planning still inserts Hash repartitioning. | ||
| ########## |
There was a problem hiding this comment.
I can propbably eliminate this, almost a sanity check / nice to see after teh positive case
|
Amazing timing. Will get you some feedback on this by early next week! Thank you! |
@stuhood great, thank you! I also have a huge line of follow up issues to support more join types that we can split up 👍 I am trying to make smaller tickets as more of the plumbing gets in to get more people involved |
45d598b to
2fca2bb
Compare
|
I have added a stacked PR that shows how I imagine Aggregations will consume the |
Which issue does this PR close?
Rationale for this change
DataFusion can represent source-declared range partitioning, but partitioned hash joins still required hash partitioned inputs. So an inner join on compatible range-partitioned keys would insert unnecessary hash repartitions, even when each left/right partition already covered the same key domain.
This PR adds a partitioning requirement that means "equal key values are co-located" . I was calling this "compatibility" but found we can satisfy the requirement with looser conditions. Other systems call this "co-location" or "co-partitioning" (trino, spark). Which they (and now I am proposing) define as when both sides of a join are already partitioned so matching key values appear in corresponding partitions, so we can join partition pairs directly without repartitioning the sides.
This lets "co-partitioned" range inputs satisfy inner partitioned hash joins. This will also be applicable to other join types and operators but kept the first PR thin to keep scope more reviewable.
What changes are included in this PR?
Adds
Distribution::KeyPartitioned(Vec<Arc<dyn PhysicalExpr>>)as a public distribution requirement.HashPartitioned([a])means rows must be partitioned by hash ona.KeyPartitioned([a])means rows with equalavalues must be co-located, but the partitioning algorithm may be hash, range, or another compatible scheme.Adds
Partitioning::co_partitioned_with(...)to validate that two independently satisfying partitionings also can be paired by partition index.Changes inner partitioned
HashJoinExecrequirements fromHashPartitionedtoKeyPartitioned.HashPartitionedfor now.Updates
EnforceDistributionso co-partitioned range inner joins avoid repartitioning.idoes not represent the same key domain on both sides.KeyPartitionedin this PR.Keeps partitioned dynamic filter pushdown restricted to hash-compatible routing.
Degrades range join output partitioning to
UnknownPartitioning(n)rather than erroring. Adding this behavior would need more tests and careful thought about, I think its safert o just degrade for first PR.Are these changes tested?
Yes.
KeyPartitionedsatisfaction for hash and range partitioning.co_partitioned_withfor compatible and incompatible range/hash partitioning.EnforceDistributionbehavior for:Are there any user-facing changes?
Yes.
This PR changes public physical planning APIs:
Distribution::KeyPartitioned.Partitioning::co_partitioned_with.Distribution.