englefly commented on code in PR #11825:
URL: https://github.com/apache/doris/pull/11825#discussion_r949857778
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/PhysicalPlanTranslator.java:
##########
@@ -360,39 +362,86 @@ public PlanFragment
visitAbstractPhysicalSort(AbstractPhysicalSort<Plan> sort, P
// 2. For ssb, there are only binary equal predicate, we shall
support more in the future.
@Override
public PlanFragment visitPhysicalHashJoin(PhysicalHashJoin<Plan, Plan>
hashJoin, PlanTranslatorContext context) {
+ if (JoinUtils.shouldNestedLoopJoin(hashJoin)) {
+ throw new RuntimeException("Physical hash join could not execute
without equal join condition.");
+ }
+
// NOTICE: We must visit from right to left, to ensure the last
fragment is root fragment
PlanFragment rightFragment = hashJoin.child(1).accept(this, context);
PlanFragment leftFragment = hashJoin.child(0).accept(this, context);
PlanNode leftFragmentPlanRoot = leftFragment.getPlanRoot();
PlanNode rightFragmentPlanRoot = rightFragment.getPlanRoot();
JoinType joinType = hashJoin.getJoinType();
- if (JoinUtils.shouldNestedLoopJoin(hashJoin)) {
- throw new RuntimeException("Physical hash join could not execute
without equal join condition.");
- } else {
- Expression eqJoinExpression = hashJoin.getCondition().get();
- List<Expr> execEqConjunctList =
ExpressionUtils.extractConjunction(eqJoinExpression).stream()
- .map(EqualTo.class::cast)
- .map(e -> swapEqualToForChildrenOrder(e,
hashJoin.left().getOutput()))
- .map(e -> ExpressionTranslator.translate(e, context))
- .collect(Collectors.toList());
+ Expression eqJoinExpression = hashJoin.getCondition().get();
+ List<Expr> execEqConjunctList =
ExpressionUtils.extractConjunction(eqJoinExpression).stream()
+ .map(EqualTo.class::cast)
+ .map(e -> swapEqualToForChildrenOrder(e,
hashJoin.left().getOutput()))
+ .map(e -> ExpressionTranslator.translate(e, context))
+ .collect(Collectors.toList());
- TupleDescriptor outputDescriptor = context.generateTupleDesc();
- List<Expr> srcToOutput = hashJoin.getOutput().stream()
- .map(SlotReference.class::cast)
- .peek(s -> context.createSlotDesc(outputDescriptor, s))
- .map(e -> ExpressionTranslator.translate(e, context))
- .collect(Collectors.toList());
+ TupleDescriptor outputDescriptor = context.generateTupleDesc();
+ List<Expr> srcToOutput = hashJoin.getOutput().stream()
+ .map(SlotReference.class::cast)
+ .peek(s -> context.createSlotDesc(outputDescriptor, s))
+ .map(e -> ExpressionTranslator.translate(e, context))
+ .collect(Collectors.toList());
- HashJoinNode hashJoinNode = new
HashJoinNode(context.nextPlanNodeId(), leftFragmentPlanRoot,
- rightFragmentPlanRoot, JoinType.toJoinOperator(joinType),
execEqConjunctList, Lists.newArrayList(),
- srcToOutput, outputDescriptor, outputDescriptor);
+ HashJoinNode hashJoinNode = new HashJoinNode(context.nextPlanNodeId(),
leftFragmentPlanRoot,
+ rightFragmentPlanRoot, JoinType.toJoinOperator(joinType),
execEqConjunctList, Lists.newArrayList(),
+ srcToOutput, outputDescriptor, outputDescriptor);
+ // TODO: use distribution info from physical properties.
+ boolean doBroadcast = !JoinUtils.onlyShuffle(hashJoin);
Review Comment:
This is a little bit aggressive. Shuffle Join is much safer than Broadcast
Join. Broadcast join cause many BE OOM.
And hence, it would be better to choose shuffle join, if both join type are
candidates, especially on TPC-H benchmark.
--
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]
