924060929 commented on code in PR #65129:
URL: https://github.com/apache/doris/pull/65129#discussion_r3550569401
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/properties/ChildOutputPropertyDeriver.java:
##########
@@ -440,53 +443,89 @@ public PhysicalProperties
visitPhysicalSetOperation(PhysicalSetOperation setOper
return PhysicalProperties.GATHER;
}
- // TODO: open comment when support `enable_local_shuffle_planner`
- // int distributeToChildIndex
- // =
setOperation.<Integer>getMutableState(PhysicalSetOperation.DISTRIBUTE_TO_CHILD_INDEX).orElse(-1);
- // if (distributeToChildIndex >= 0
- // && childrenDistribution.get(distributeToChildIndex)
instanceof DistributionSpecHash) {
- // DistributionSpecHash childDistribution
- // = (DistributionSpecHash)
childrenDistribution.get(distributeToChildIndex);
- // List<SlotReference> childToIndex =
setOperation.getRegularChildrenOutputs().get(distributeToChildIndex);
- // Map<ExprId, Integer> idToOutputIndex = new LinkedHashMap<>();
- // for (int j = 0; j < childToIndex.size(); j++) {
- // idToOutputIndex.put(childToIndex.get(j).getExprId(), j);
- // }
- //
- // List<ExprId> orderedShuffledColumns =
childDistribution.getOrderedShuffledColumns();
- // List<ExprId> setOperationDistributeColumnIds = new
ArrayList<>();
- // for (ExprId tableDistributeColumnId : orderedShuffledColumns) {
- // Integer index =
idToOutputIndex.get(tableDistributeColumnId);
- // if (index == null) {
- // break;
- // }
- //
setOperationDistributeColumnIds.add(setOperation.getOutput().get(index).getExprId());
- // }
- // // check whether the set operation output all distribution
columns of the child
- // if (setOperationDistributeColumnIds.size() ==
orderedShuffledColumns.size()) {
- // boolean isUnion = setOperation instanceof Union;
- // boolean shuffleToRight = distributeToChildIndex > 0;
- // if (!isUnion && shuffleToRight) {
- // return new PhysicalProperties(
- // new DistributionSpecHash(
- // setOperationDistributeColumnIds,
- // ShuffleType.EXECUTION_BUCKETED
- // )
- // );
- // } else {
- // // keep the distribution as the child
- // return new PhysicalProperties(
- // new DistributionSpecHash(
- // setOperationDistributeColumnIds,
- // childDistribution.getShuffleType(),
- // childDistribution.getTableId(),
- // childDistribution.getSelectedIndexId(),
- // childDistribution.getPartitionIds()
- // )
- // );
- // }
- // }
- // }
+ // After ChildrenPropertiesRegulator the children distributions are
already legal, so
+ // the output is derived by describing what the children provide:
+ // 1. one or more NATURAL children: output the first NATURAL child's
distribution
+ // (several NATURAL children behave like colocate);
+ // 2. no NATURAL but some STORAGE_BUCKETED child: output its
STORAGE_BUCKETED
+ // distribution;
+ // 3. all EXECUTION_BUCKETED children: output the execution hash (the
generic loop
+ // below);
+ // 4. anything else (e.g. random): output a non-specific property
(also below).
+ // Rules 1 and 2 only apply when every child is hash-distributed: with
a mixed shape
+ // such as one execution-any child plus one storage-bucketed child
(reachable through
+ // the ANY child request of union), the set operation output is not
distributed by the
+ // storage layout, so it falls through to the generic derivation below.
+ // The basic child is recomputed from the children distributions
instead of being
+ // carried as mutable planner state, because mutable state does not
survive the
+ // with-copies in chooseBestPlan() and the
RecomputePhysicalPropertiesPostProcessor
+ // re-derivation, while this recomputation is deterministic on any
copy of the plan.
+ int firstNaturalIndex = -1;
+ int firstStorageBucketedIndex = -1;
+ boolean allChildrenHash = true;
+ for (int i = 0; i < childrenDistribution.size(); i++) {
+ if (!(childrenDistribution.get(i) instanceof
DistributionSpecHash)) {
+ allChildrenHash = false;
+ break;
+ }
+ ShuffleType childShuffleType
+ = ((DistributionSpecHash)
childrenDistribution.get(i)).getShuffleType();
+ if (childShuffleType == ShuffleType.NATURAL) {
+ if (firstNaturalIndex < 0) {
+ firstNaturalIndex = i;
+ }
+ } else if (childShuffleType == ShuffleType.STORAGE_BUCKETED
+ && firstStorageBucketedIndex < 0) {
+ firstStorageBucketedIndex = i;
+ }
+ }
+ int distributeToChildIndex = -1;
+ if (allChildrenHash) {
+ distributeToChildIndex = firstNaturalIndex >= 0 ?
firstNaturalIndex : firstStorageBucketedIndex;
+ }
+ if (distributeToChildIndex >= 0) {
+ DistributionSpecHash childDistribution
Review Comment:
You're right — the simplification held for children aligned by the
regulator's bucket-shuffle branch, but the union's cheap ANY child request does
not align its children, so two children can carry different storage layouts /
positions there. Restored the cross-child validation: when any child is
NATURAL/STORAGE_BUCKETED, the set operation only claims that bucket
distribution when EVERY child is hash-distributed by the same storage layout
(table/index/partitions) and maps its shuffle columns to the same set-op output
positions; otherwise it returns `createAnyFromHash` so a parent operator
inserts a real exchange. The all-execution-bucketed path is unchanged (generic
loop). Verified on a 4-BE cluster: the existing shape .out is unchanged (the
guard never fires for aligned plans), and your reduced shape (two nested
bucket-shuffle intersects of different layouts under a union feeding a
group-by) already inserts the realignment exchanges and returns correct results.
--
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]
