morrySnow commented on code in PR #39148:
URL: https://github.com/apache/doris/pull/39148#discussion_r1713321881
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/properties/ChildOutputPropertyDeriver.java:
##########
@@ -255,11 +255,28 @@ public PhysicalProperties visitPhysicalHashJoin(
Preconditions.checkState(childrenOutputProperties.size() == 2);
PhysicalProperties leftOutputProperty =
childrenOutputProperties.get(0);
PhysicalProperties rightOutputProperty =
childrenOutputProperties.get(1);
+ Plan leftChild = hashJoin.child(0);
// broadcast
if (rightOutputProperty.getDistributionSpec() instanceof
DistributionSpecReplicated) {
- DistributionSpec parentDistributionSpec =
leftOutputProperty.getDistributionSpec();
- return new PhysicalProperties(parentDistributionSpec);
+ DistributionSpec leftDistributionSpec =
leftOutputProperty.getDistributionSpec();
+ List<ExprId> leftExprIds =
hashJoin.getHashConjunctsExprIds().first;
+ List<ExprId> rightExprIds =
hashJoin.getHashConjunctsExprIds().second;
+ if (leftChild.getOutputSet().containsAll(hashJoin.getOutputSet()))
{
+ return new PhysicalProperties(leftDistributionSpec);
+ } else if (leftDistributionSpec instanceof DistributionSpecHash
+ && !leftExprIds.isEmpty() && !rightExprIds.isEmpty() &&
leftExprIds.size() == rightExprIds.size()) {
+ DistributionSpecHash tmpRightHashSpec = new
DistributionSpecHash(rightExprIds, ShuffleType.REQUIRE);
+ if (SessionVariable.canUseNereidsDistributePlanner()) {
+ return computeShuffleJoinOutputProperties(hashJoin,
+ (DistributionSpecHash) leftDistributionSpec,
tmpRightHashSpec);
+ } else {
+ return legacyComputeShuffleJoinOutputProperties(hashJoin,
+ (DistributionSpecHash) leftDistributionSpec,
tmpRightHashSpec);
+ }
Review Comment:
could not reuse these two help functions directly. because they assume LEFT
and RIGHT have same distribute with same keys and same order. But in this case,
left not always distribute as join keys. so we should check left distribute
first. ensure it is a hash distribute and its keys is a subset of join keys.
and then construct a hash distibute for right side with same keys (not all join
keys, but only key used in left hash dist).
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/properties/ChildOutputPropertyDeriver.java:
##########
@@ -255,11 +255,28 @@ public PhysicalProperties visitPhysicalHashJoin(
Preconditions.checkState(childrenOutputProperties.size() == 2);
PhysicalProperties leftOutputProperty =
childrenOutputProperties.get(0);
PhysicalProperties rightOutputProperty =
childrenOutputProperties.get(1);
+ Plan leftChild = hashJoin.child(0);
// broadcast
if (rightOutputProperty.getDistributionSpec() instanceof
DistributionSpecReplicated) {
- DistributionSpec parentDistributionSpec =
leftOutputProperty.getDistributionSpec();
- return new PhysicalProperties(parentDistributionSpec);
+ DistributionSpec leftDistributionSpec =
leftOutputProperty.getDistributionSpec();
+ List<ExprId> leftExprIds =
hashJoin.getHashConjunctsExprIds().first;
+ List<ExprId> rightExprIds =
hashJoin.getHashConjunctsExprIds().second;
+ if (leftChild.getOutputSet().containsAll(hashJoin.getOutputSet()))
{
+ return new PhysicalProperties(leftDistributionSpec);
+ } else if (leftDistributionSpec instanceof DistributionSpecHash
+ && !leftExprIds.isEmpty() && !rightExprIds.isEmpty() &&
leftExprIds.size() == rightExprIds.size()) {
+ DistributionSpecHash tmpRightHashSpec = new
DistributionSpecHash(rightExprIds, ShuffleType.REQUIRE);
+ if (SessionVariable.canUseNereidsDistributePlanner()) {
+ return computeShuffleJoinOutputProperties(hashJoin,
+ (DistributionSpecHash) leftDistributionSpec,
tmpRightHashSpec);
+ } else {
+ return legacyComputeShuffleJoinOutputProperties(hashJoin,
+ (DistributionSpecHash) leftDistributionSpec,
tmpRightHashSpec);
+ }
+ } else {
+ throw new RuntimeException("Could not find hash join's
conjuncts: " + hashJoin);
+ }
Review Comment:
should not throw exception. should `return new
PhysicalProperties(leftDistributionSpec);`
--
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]
