This is an automated email from the ASF dual-hosted git repository.
morrysnow pushed a commit to branch branch-2.0
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-2.0 by this push:
new 3a3d0107189 [fix](Nereids) physical property deriver on some node is
not right (#30819) (#30838)
3a3d0107189 is described below
commit 3a3d010718940da298c6e2c5dc1db91065e0fb12
Author: morrySnow <[email protected]>
AuthorDate: Mon Feb 5 16:55:48 2024 +0800
[fix](Nereids) physical property deriver on some node is not right (#30819)
(#30838)
pick from master #30819
commit id d6bff23f4805f9d6b58c5450479ffa6ecc7a633c
---
.../properties/ChildOutputPropertyDeriver.java | 37 ++++++++++++++++-----
.../nereids/properties/PhysicalProperties.java | 8 +++++
.../properties/ChildOutputPropertyDeriverTest.java | 38 ++++++++++++++++++++--
3 files changed, 73 insertions(+), 10 deletions(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/properties/ChildOutputPropertyDeriver.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/properties/ChildOutputPropertyDeriver.java
index 4bcf6cafb84..41cdf78cf89 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/properties/ChildOutputPropertyDeriver.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/properties/ChildOutputPropertyDeriver.java
@@ -96,7 +96,16 @@ public class ChildOutputPropertyDeriver extends
PlanVisitor<PhysicalProperties,
@Override
public PhysicalProperties visit(Plan plan, PlanContext context) {
- return PhysicalProperties.ANY;
+ if (childrenOutputProperties.isEmpty()) {
+ return PhysicalProperties.ANY;
+ } else {
+ DistributionSpec firstChildSpec =
childrenOutputProperties.get(0).getDistributionSpec();
+ if (firstChildSpec instanceof DistributionSpecHash) {
+ return
PhysicalProperties.createAnyFromHash((DistributionSpecHash) firstChildSpec);
+ } else {
+ return PhysicalProperties.ANY;
+ }
+ }
}
/*
********************************************************************************************
@@ -115,7 +124,7 @@ public class ChildOutputPropertyDeriver extends
PlanVisitor<PhysicalProperties,
@Override
public PhysicalProperties visitPhysicalCTEConsumer(
PhysicalCTEConsumer cteConsumer, PlanContext context) {
- Preconditions.checkState(childrenOutputProperties.size() == 0);
+ Preconditions.checkState(childrenOutputProperties.isEmpty(), "cte
consumer should be leaf node");
return PhysicalProperties.MUST_SHUFFLE;
}
@@ -278,7 +287,7 @@ public class ChildOutputPropertyDeriver extends
PlanVisitor<PhysicalProperties,
leftHashSpec.getShuffleType()));
}
case FULL_OUTER_JOIN:
- return PhysicalProperties.ANY;
+ return PhysicalProperties.createAnyFromHash(leftHashSpec);
default:
throw new AnalysisException("unknown join type " +
hashJoin.getJoinType());
}
@@ -347,7 +356,12 @@ public class ChildOutputPropertyDeriver extends
PlanVisitor<PhysicalProperties,
@Override
public PhysicalProperties visitPhysicalRepeat(PhysicalRepeat<? extends
Plan> repeat, PlanContext context) {
Preconditions.checkState(childrenOutputProperties.size() == 1);
- return
PhysicalProperties.ANY.withOrderSpec(childrenOutputProperties.get(0).getOrderSpec());
+ DistributionSpec childDistributionSpec =
childrenOutputProperties.get(0).getDistributionSpec();
+ PhysicalProperties output = childrenOutputProperties.get(0);
+ if (childDistributionSpec instanceof DistributionSpecHash) {
+ output =
PhysicalProperties.createAnyFromHash((DistributionSpecHash)
childDistributionSpec);
+ }
+ return
output.withOrderSpec(childrenOutputProperties.get(0).getOrderSpec());
}
@Override
@@ -375,7 +389,12 @@ public class ChildOutputPropertyDeriver extends
PlanVisitor<PhysicalProperties,
for (int i = 0; i < childrenDistribution.size(); i++) {
DistributionSpec childDistribution = childrenDistribution.get(i);
if (!(childDistribution instanceof DistributionSpecHash)) {
- return PhysicalProperties.ANY;
+ if (i != 0) {
+ // NOTICE: if come here, the first child output must be
DistributionSpecHash
+ return
PhysicalProperties.createAnyFromHash((DistributionSpecHash)
childrenDistribution.get(0));
+ } else {
+ return new PhysicalProperties(childDistribution);
+ }
}
DistributionSpecHash distributionSpecHash = (DistributionSpecHash)
childDistribution;
int[] offsetsOfCurrentChild = new
int[distributionSpecHash.getOrderedShuffledColumns().size()];
@@ -385,7 +404,8 @@ public class ChildOutputPropertyDeriver extends
PlanVisitor<PhysicalProperties,
if (offset >= 0) {
offsetsOfCurrentChild[offset] = j;
} else {
- return PhysicalProperties.ANY;
+ // NOTICE: if come here, the first child output must be
DistributionSpecHash
+ return
PhysicalProperties.createAnyFromHash((DistributionSpecHash)
childrenDistribution.get(0));
}
}
if (offsetsOfFirstChild == null) {
@@ -393,7 +413,8 @@ public class ChildOutputPropertyDeriver extends
PlanVisitor<PhysicalProperties,
offsetsOfFirstChild = offsetsOfCurrentChild;
} else if (!Arrays.equals(offsetsOfFirstChild,
offsetsOfCurrentChild)
|| firstType != ((DistributionSpecHash)
childDistribution).getShuffleType()) {
- return PhysicalProperties.ANY;
+ // NOTICE: if come here, the first child output must be
DistributionSpecHash
+ return
PhysicalProperties.createAnyFromHash((DistributionSpecHash)
childrenDistribution.get(0));
}
}
// bucket
@@ -411,7 +432,7 @@ public class ChildOutputPropertyDeriver extends
PlanVisitor<PhysicalProperties,
} else {
// current be could not run const expr on appropriate node,
// so if we have constant exprs on union, the output of union
always any
- return PhysicalProperties.ANY;
+ return visit(union, context);
}
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/properties/PhysicalProperties.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/properties/PhysicalProperties.java
index e345b462b52..c9cca233e57 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/properties/PhysicalProperties.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/properties/PhysicalProperties.java
@@ -93,6 +93,14 @@ public class PhysicalProperties {
return new PhysicalProperties(distributionSpecHash);
}
+ public static PhysicalProperties createAnyFromHash(DistributionSpecHash
childSpec) {
+ if (childSpec.getShuffleType() == ShuffleType.NATURAL) {
+ return PhysicalProperties.STORAGE_ANY;
+ } else {
+ return PhysicalProperties.ANY;
+ }
+ }
+
public PhysicalProperties withOrderSpec(OrderSpec orderSpec) {
return new PhysicalProperties(distributionSpec, orderSpec);
}
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/nereids/properties/ChildOutputPropertyDeriverTest.java
b/fe/fe-core/src/test/java/org/apache/doris/nereids/properties/ChildOutputPropertyDeriverTest.java
index 9c9634e44d4..727a1694489 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/nereids/properties/ChildOutputPropertyDeriverTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/nereids/properties/ChildOutputPropertyDeriverTest.java
@@ -449,7 +449,7 @@ public class ChildOutputPropertyDeriverTest {
}
@Test
- public void testFullOuterJoin() {
+ void testFullOuterJoinWithNatural() {
PhysicalHashJoin<GroupPlan, GroupPlan> join = new
PhysicalHashJoin<>(JoinType.FULL_OUTER_JOIN,
ExpressionUtils.EMPTY_CONDITION,
ExpressionUtils.EMPTY_CONDITION, JoinHint.NONE, Optional.empty(),
logicalProperties,
groupPlan, groupPlan);
@@ -479,7 +479,41 @@ public class ChildOutputPropertyDeriverTest {
PhysicalProperties result =
deriver.getOutputProperties(groupExpression);
Assertions.assertTrue(result.getOrderSpec().getOrderKeys().isEmpty());
- Assertions.assertTrue(result.getDistributionSpec() instanceof
DistributionSpecAny);
+ Assertions.assertInstanceOf(DistributionSpecStorageAny.class,
result.getDistributionSpec());
+ }
+
+ @Test
+ void testFullOuterJoinWithOther() {
+ PhysicalHashJoin<GroupPlan, GroupPlan> join = new
PhysicalHashJoin<>(JoinType.FULL_OUTER_JOIN,
+ ExpressionUtils.EMPTY_CONDITION,
ExpressionUtils.EMPTY_CONDITION, JoinHint.NONE, Optional.empty(),
logicalProperties,
+ groupPlan, groupPlan);
+ GroupExpression groupExpression = new GroupExpression(join);
+
+ PhysicalProperties left = new PhysicalProperties(
+ new DistributionSpecHash(
+ Lists.newArrayList(new ExprId(0)),
+ ShuffleType.EXECUTION_BUCKETED,
+ 0,
+ Sets.newHashSet(0L)
+ ),
+ new OrderSpec(
+ Lists.newArrayList(new OrderKey(new
SlotReference("ignored", IntegerType.INSTANCE),
+ true, true)))
+ );
+
+ PhysicalProperties right = new PhysicalProperties(new
DistributionSpecHash(
+ Lists.newArrayList(new ExprId(1)),
+ ShuffleType.EXECUTION_BUCKETED,
+ 1,
+ Sets.newHashSet(1L)
+ ));
+
+ List<PhysicalProperties> childrenOutputProperties =
Lists.newArrayList(left, right);
+ ChildOutputPropertyDeriver deriver = new
ChildOutputPropertyDeriver(childrenOutputProperties);
+
+ PhysicalProperties result =
deriver.getOutputProperties(groupExpression);
+ Assertions.assertTrue(result.getOrderSpec().getOrderKeys().isEmpty());
+ Assertions.assertInstanceOf(DistributionSpecAny.class,
result.getDistributionSpec());
}
@Test
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
