github-actions[bot] commented on code in PR #65129:
URL: https://github.com/apache/doris/pull/65129#discussion_r3548514818
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/PhysicalPlanTranslator.java:
##########
@@ -2496,14 +2496,38 @@ public PlanFragment visitPhysicalSetOperation(
setOperationNode.setColocate(true);
}
- // TODO: open comment when support `enable_local_shuffle_planner`
- // for (Plan child : setOperation.children()) {
- // PhysicalPlan childPhysicalPlan = (PhysicalPlan) child;
- // if
(JoinUtils.isStorageBucketed(childPhysicalPlan.getPhysicalProperties())) {
- //
setOperationNode.setDistributionMode(DistributionMode.BUCKET_SHUFFLE);
- // break;
- // }
- // }
+ // A storage-bucketed child means set-op bucket shuffle was chosen by
+ // ChildrenPropertiesRegulator, which only does so under the FE
local-shuffle planner;
+ // the gate here keeps the two sites explicitly consistent. Mark the
node BUCKET_SHUFFLE
+ // so the set sink/probe align by bucket instead of execution-bucketed
hash.
+ //
+ // Unlike hash join, BUCKET_SHUFFLE is not exclusive with isColocate
above: for a set
+ // operation isColocate describes the bucket-aligned scheduling of the
fragment (the
+ // basic child scans buckets directly), while BUCKET_SHUFFLE describes
how the other
+ // children arrive (bucket-shuffle exchanges). Both routes converge to
the same
+ // bucket-hash local exchange requirement in
SetOperationNode.enforceAndDeriveLocalExchange.
+ ConnectContext setOperationConnectContext =
context.getConnectContext();
+ if (setOperationConnectContext != null
+ &&
setOperationConnectContext.getSessionVariable().isEnableLocalShuffle()
+ &&
setOperationConnectContext.getSessionVariable().isEnableLocalShufflePlanner()
Review Comment:
`RequestPropertyDeriver.setOperationBucketShuffleAllowed()` now allows
set-operation bucket shuffle when local shuffle is disabled
(`!isEnableLocalShuffle()`), but this translator gate still requires
`isEnableLocalShuffle()` before it marks the legacy `SetOperationNode` as
`BUCKET_SHUFFLE`. In that session the Nereids plan can still have
bucket-shuffled set-operation children, while the translated node remains
`PARTITIONED`.
That marker is used later by the worker assignment path.
`UnassignedScanBucketOlapTableJob.shouldFillUpInstances()` only fills missing
bucket receiver instances for non-`INTERSECT` set operations when
`setOperationNode.isBucketShuffle()` is true. So for an `EXCEPT`/`UNION`
bucket-shuffle set op where buckets are pruned from the basic scan side, the
bucket-shuffled rows for those missing buckets can still have no receiver. The
new `enable_local_shuffle=false` regression only covers `INTERSECT`, and that
fill-up path explicitly skips `Intersect`, so it does not catch this.
Please use the same effective predicate as request derivation here, or
otherwise keep the `BUCKET_SHUFFLE` marker set for the local-shuffle-disabled
bucket-shuffle path.
--
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]
