gene-bordegaray commented on code in PR #23184:
URL: https://github.com/apache/datafusion/pull/23184#discussion_r3486659012
##########
datafusion/physical-expr/src/partitioning.rs:
##########
@@ -406,37 +391,56 @@ impl Partitioning {
}
}
- /// Returns true when `self` and `other` describe compatible partition
maps.
+ /// Returns true when two partitionings both satisfy their own distribution
+ /// requirements and can be paired by partition index.
+ ///
+ /// Use this for multi-input operators, such as partitioned joins, where
+ /// each child has a different schema, required [`Distribution`], and
+ /// expression-equivalence context.
+ ///
+ /// ```text
+ /// # co-partitioned: each side satisfies its own requirement, and
boundaries match
+ /// left: Range(left.a ASC, [10, 20]), required KeyPartitioned(left.a)
+ /// right: Range(right.x ASC, [10, 20]), required KeyPartitioned(right.x)
///
- /// Compatible partition maps can be used for partition-local behavior: if
- /// this returns true, partition `i` from both partitionings can be treated
- /// as covering the same partition domain. This is stricter than
- /// [`Self::satisfaction`], which only answers whether this partitioning
can
- /// satisfy a required distribution.
- pub fn compatible_with(
+ /// # not compatible: right side does not satisfy a hash-specific
requirement
+ /// left: Range(left.a ASC, [10, 20]), required KeyPartitioned(left.a)
+ /// right: Range(right.x ASC, [10, 20]), required HashPartitioned(right.x)
+ ///
+ /// # not compatible: boundaries differ
+ /// 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(
Review Comment:
Ya I thought similarly but this matters for multi-children operators. For
joins, each side has a different may have a diff schema:
```
left: Partitioning::Range(left.a)
right: Partitioning::Range(right.x)
join: left.a = right.x
```
So the expressions `left.a` and `right.x` are not equal since they are from
different children. Because of this we need to check each sides req
`Distribution` to know what each partitioning has to satisfy:
```
left requirement: key partitioned on left.a
right requirement: key partitioned on right.x
```
So we need to check:
1. left partitioning satisfies left requirement
2. right partitioning satisfies right requirement
3. the two partition mappings can be paired
I could do something like this:
```rust
struct RequiredPartitioning<'a> {
partitioning: &'a Partitioning,
requirement: &'a Distribution,
eq_properties: &'a EquivalenceProperties,
}
impl RequiredPartitioning<'_> {
fn co_partitioned_with(&self, other:
&RequiredPartitioning<'_>) -> bool
}
```
or derive the `EquivalenceProperties` in the method if we pass the children
directly
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
