924060929 commented on code in PR #65129:
URL: https://github.com/apache/doris/pull/65129#discussion_r3550385398
##########
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:
Good catch — that gate was stale after the request-derivation predicate
changed. Removed the session-variable check from the translator entirely: a
storage-bucketed child only appears when `ChildrenPropertiesRegulator` chose
the bucket shuffle alternative (which it only does when set-op bucket shuffle
is allowed and `canUseNereidsDistributePlanner` is true), so the marker now
follows the regulator's decision directly and cannot drift out of sync with
`setOperationBucketShuffleAllowed()`. Verified on a 4-BE cluster that
intersect/except/join-as-basic under `enable_local_shuffle=false` are now
marked BUCKET_SHUFFLE and return correct results, while the
`enable_nereids_distribute_planner=false` path produces no storage-bucketed
children and is correctly left unmarked.
--
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]
