HappenLee commented on code in PR #66477:
URL: https://github.com/apache/doris/pull/66477#discussion_r4143100704
##########
fe/fe-core/src/main/java/org/apache/doris/planner/PlanNode.java:
##########
@@ -1182,6 +1183,50 @@ protected Pair<PlanNode, LocalExchangeType>
enforceRequire(
return Pair.of(leNode, preferType);
}
+ /**
+ * Return the effective storage hash type when this subtree has one
unambiguous bucket layout.
+ * Unary nodes preserve their child's layout; multi-input nodes preserve
it only when every
+ * child reports the same layout.
+ */
+ public HashDistributionInfo.HashType getStorageDistributionHashType() {
+ HashDistributionInfo.HashType hashType = null;
+ for (PlanNode child : children) {
+ HashDistributionInfo.HashType childHashType =
child.getStorageDistributionHashType();
+ if (childHashType == null) {
+ return null;
+ }
+ if (hashType != null && hashType != childHashType) {
+ return null;
+ }
+ hashType = childHashType;
+ }
+ return hashType;
+ }
+
+ /**
+ * Collect every distinct storage hash type declared by nodes in this
subtree that have a
+ * definite layout opinion (OLAP scans, exchanges, local exchanges; nodes
without one, like
+ * schema scans or empty-set nodes, stay silent). Used to distinguish a
genuinely mixed
+ * subtree (both CRC32 and IDENTITY) from one that simply has no bucketed
storage at all.
+ */
+ public void collectStorageHashTypes(Set<HashDistributionInfo.HashType>
hashTypes) {
+ HashDistributionInfo.HashType own = getOwnStorageHashType();
+ if (own != null) {
+ hashTypes.add(own);
+ }
+ for (PlanNode child : children) {
Review Comment:
[P2] Keep the fragment layout check within the fragment and respect
broadcast join output distribution
`collectStorageHashTypes()` descends through `ExchangeNode.children`, which
retains the source fragment root. With `enable_local_shuffle_planner=false`,
consider this reduced plan:
```text
Fragment A: Broadcast HashJoin(r.id = i.id)
Scan r: DISTRIBUTED BY RANDOM -> storage hash type null
Broadcast Exchange: default hash tag CRC32
Fragment B: Scan i: distribution_hash_type=identity
```
`HashJoinNode.getStorageDistributionHashType()` correctly preserves the
probe's null layout. The new `PlanFragment.toThrift()` fallback then calls this
traversal, collects both the broadcast Exchange's default CRC32 tag and the
other fragment's IDENTITY scan, and throws `fragment mixes distribution hash
types`. Broadcast does not require the probe and build storage layouts to match.
A regression can create a RANDOM table `r` and an IDENTITY table `i`, then
force the shape with:
```sql
SET enable_local_shuffle_planner = false;
SELECT /*+ LEADING(r i) */ r.id
FROM r JOIN [broadcast] i ON r.id = i.id;
```
For `r.id={1,2}` and `i.id={2,3}`, the expected result is `2`, with
successful plan serialization. This is a code-derived scenario, not an executed
reproducer.
Please stop collection at Exchange boundaries and avoid treating an inactive
default hash tag on a broadcast Exchange as an actual storage bucket layout.
The validation should follow the operator's effective output distribution, so
the broadcast build cannot override the probe. Add a SQL-to-Thrift regression
covering this case and both FE local-shuffle settings, while retaining
rejection of genuinely incompatible bucket layouts.
--
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]
