924060929 commented on code in PR #65129:
URL: https://github.com/apache/doris/pull/65129#discussion_r3568255090
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/properties/ChildOutputPropertyDeriver.java:
##########
@@ -528,6 +527,121 @@ public PhysicalProperties
visitPhysicalSetOperation(PhysicalSetOperation setOper
return PhysicalProperties.createHash(request, firstType);
}
+ /**
+ * The basic (bucket-shuffle anchor) child candidate of a set operation:
the first NATURAL
+ * child, or else the first STORAGE_BUCKETED child, or -1 when no child is
bucket-distributed.
+ * The largest natural / storage-bucketed child keeps its buckets and the
others are
+ * bucket-shuffled to it, so this only picks the candidate; {@link
#isSetOperationBucketAligned}
+ * decides whether the children actually agree on that anchor's buckets.
+ */
+ public static int setOperationBucketBasicIndex(List<DistributionSpec>
childrenDistribution) {
+ int firstNaturalIndex = -1;
+ int firstStorageBucketedIndex = -1;
+ for (int i = 0; i < childrenDistribution.size(); i++) {
+ if (!(childrenDistribution.get(i) instanceof
DistributionSpecHash)) {
+ continue;
+ }
+ ShuffleType childShuffleType = ((DistributionSpecHash)
childrenDistribution.get(i)).getShuffleType();
+ if (childShuffleType == ShuffleType.NATURAL && firstNaturalIndex <
0) {
+ firstNaturalIndex = i;
+ } else if (childShuffleType == ShuffleType.STORAGE_BUCKETED &&
firstStorageBucketedIndex < 0) {
+ firstStorageBucketedIndex = i;
+ }
+ }
+ return firstNaturalIndex >= 0 ? firstNaturalIndex :
firstStorageBucketedIndex;
+ }
+
+ /**
+ * Whether every child of the set operation is genuinely bucket-aligned to
a single basic
+ * child, i.e. all children are hash NATURAL / STORAGE_BUCKETED on the
same storage layout
+ * (table / index / partitions) AND map their shuffle columns to the same
set-operation output
+ * positions. This is the single proof shared by {@link
#visitPhysicalSetOperation} (to decide
+ * the output distribution) and {@code PhysicalPlanTranslator} (to set the
legacy
+ * {@code BUCKET_SHUFFLE} marker), so the derived property and the marker
can never disagree: a
+ * same-layout shape whose bucket key feeds different set-operation output
positions in
+ * different children is not aligned and must not be treated as bucketed
by either side.
+ */
+ public static boolean isSetOperationBucketAligned(
+ PhysicalSetOperation setOperation, List<DistributionSpec>
childrenDistribution) {
+ int distributeToChildIndex =
setOperationBucketBasicIndex(childrenDistribution);
+ if (distributeToChildIndex < 0) {
+ return false;
+ }
+ DistributionSpecHash basicChildHash = (DistributionSpecHash)
childrenDistribution.get(distributeToChildIndex);
+ List<Integer> basicOutputPositions
+ = mapShuffleColumnsToOutputPositions(setOperation,
basicChildHash, distributeToChildIndex);
+ if (basicOutputPositions == null || basicOutputPositions.isEmpty() ||
basicChildHash.getTableId() < 0) {
+ return false;
+ }
+ for (int i = 0; i < childrenDistribution.size(); i++) {
+ if (!(childrenDistribution.get(i) instanceof
DistributionSpecHash)) {
+ return false;
+ }
+ DistributionSpecHash childHash = (DistributionSpecHash)
childrenDistribution.get(i);
+ ShuffleType childShuffleType = childHash.getShuffleType();
+ if (childShuffleType != ShuffleType.NATURAL && childShuffleType !=
ShuffleType.STORAGE_BUCKETED) {
+ return false;
+ }
+ // same storage layout (a NATURAL basic child and its enforced
STORAGE_BUCKETED siblings
+ // all carry the basic child's layout, so they share the bucket
function)
+ if (childHash.getTableId() != basicChildHash.getTableId()
+ || childHash.getSelectedIndexId() !=
basicChildHash.getSelectedIndexId()
+ ||
!childHash.getPartitionIds().equals(basicChildHash.getPartitionIds())) {
+ return false;
+ }
+ // same set-operation output positions (the bucket key must feed
the same output columns
+ // in every child, otherwise per-bucket set operation would
compare mismatched columns)
+ List<Integer> positions =
mapShuffleColumnsToOutputPositions(setOperation, childHash, i);
+ if (positions == null || !positions.equals(basicOutputPositions)) {
+ return false;
+ }
+ }
+ return true;
+ }
+
+ /**
+ * Map a child's ordered shuffle columns to the set operation output
positions. The lookup is
+ * equivalence-set aware, mirroring the regulator's {@code
canMapBucketKeysToRequire} /
+ * {@code calAnotherSideRequiredShuffleIds}: a bucket key can reach the
set operation through an
+ * equivalent slot (for example a join key {@code a.id} exposed only as
the projected
+ * {@code b.id}), so the ordered shuffle ExprId itself may not appear in
the child output while
+ * an equivalent one does. Each bucket key is therefore resolved through
its hash equivalence
+ * set to the output position of a visible member of that set. Returns
null when any bucket key
+ * has no visible member in the child output (the set operation does not
carry that distribution
+ * column, so the children cannot be treated as bucket-aligned).
+ */
+ static List<Integer> mapShuffleColumnsToOutputPositions(
+ PhysicalSetOperation setOperation, DistributionSpecHash childHash,
int childIndex) {
+ List<SlotReference> childOutput =
setOperation.getRegularChildrenOutputs().get(childIndex);
+ Map<ExprId, Integer> exprIdToEquivalenceSet =
childHash.getExprIdToEquivalenceSet();
+ Map<ExprId, Integer> idToOutputIndex = new LinkedHashMap<>();
+ // equivalence set index -> first output position of a slot in that set
+ Map<Integer, Integer> equivalenceSetToOutputIndex = new
LinkedHashMap<>();
+ for (int j = 0; j < childOutput.size(); j++) {
+ ExprId outputExprId = childOutput.get(j).getExprId();
+ idToOutputIndex.putIfAbsent(outputExprId, j);
+ Integer equivalenceSet = exprIdToEquivalenceSet.get(outputExprId);
+ if (equivalenceSet != null) {
+ equivalenceSetToOutputIndex.putIfAbsent(equivalenceSet, j);
+ }
Review Comment:
Fixed by making the resolution deterministic across children instead of each
child picking its own first visible equivalent.
`mapShuffleColumnsToOutputPositionSets` now returns, per bucket key, the full
set of output positions holding a visible member of that key's equivalence set,
and `resolveSetOperationBucketOutputPositions` intersects those sets across all
children (choosing the smallest common position). When a hidden bucket key has
several visible equivalents (`x`, `y`) and the parent hashes on the later one,
the enforced sibling is shuffled by `y` and so exposes only that single
position, so the intersection collapses to `y` for the basic child too —
matching the regulator's resolution. Both the deriver's output property and the
translator marker go through this one function, so they stay consistent.
Added `bucket_shuffle_two_equivalent_keys`: the join chain `a.id = b.id =
c.id` makes `fill_anchor.id` equivalent to both projected columns `x` (`c.id`)
and `y` (`b.id`), the anchor is pruned to 4/10 buckets, and the parent hashes
on `y`. It asserts `PhysicalUnion[bucketShuffle]` and an ordered result (21
rows) matching the execution-shuffle path.
In the interest of full disclosure: I could not get the pre-fix
first-visible code to actually drop rows for this shape on a 4-BE cluster — the
optimizer's column pruning and distribution propagation kept the basic child's
bucket key resolved to a directly-visible output column in every variant I
built, so the hidden-key-with-two-visible-equivalents precondition did not
arise for a fill-up-dependent union (the result was already 21 both ways). The
intersection removes the arbitrary choice regardless, and the whole existing
suite `.out` is unchanged, so this is a behavior-preserving hardening. If you
have a concrete shape that reaches the hidden-key case, I'm happy to add it as
a stronger regression.
--
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]
