morrySnow commented on code in PR #63366:
URL: https://github.com/apache/doris/pull/63366#discussion_r3412495881
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/PhysicalPlanTranslator.java:
##########
@@ -3314,7 +3349,7 @@ private boolean
findOlapScanNodesByPassExchangeNode(PlanNode root) {
return false;
}
- private List<List<Expr>> getDistributeExprs(Plan ... children) {
+ private List<List<Expr>> getChildrenDistributeExprs(Plan ... children) {
Review Comment:
this function's name is not very suitable. If you want to obtain the
DistributeExprs of children, the input should be the parent node rather than
the children.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/NereidsPlanner.java:
##########
@@ -634,7 +636,8 @@ protected void splitFragments(PhysicalPlan resultPlan) {
return;
}
- PlanTranslatorContext planTranslatorContext = new
PlanTranslatorContext(cascadesContext);
+ this.planTranslatorContext = new
PlanTranslatorContext(cascadesContext);
+ PlanTranslatorContext planTranslatorContext =
this.planTranslatorContext;
Review Comment:
useless local variable
##########
fe/fe-core/src/main/java/org/apache/doris/qe/runtime/ThriftPlansBuilder.java:
##########
@@ -277,7 +277,8 @@ private static Multiset<DistributedPlanWorker>
computeInstanceNumPerWorker(
return workerCounter;
}
- private static Map<Integer, Integer>
computeExchangeSenderNum(PipelineDistributedPlan distributedPlan) {
+ private static Map<Integer, Integer> computeExchangeSenderNum(
+ PipelineDistributedPlan distributedPlan) {
Review Comment:
why change this?
##########
fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java:
##########
@@ -1130,8 +1130,10 @@ public boolean isProfileSafeStmt() {
// 1. CreateTableCommand(mainly for create as select).
// 2. LoadCommand.
// 3. InsertOverwriteTableCommand.
+ // 4. MergeIntoCommand (merge into ... using ...).
if ((plan instanceof Command) && !(plan instanceof LoadCommand)
- && !(plan instanceof CreateTableCommand) && !(plan instanceof
InsertOverwriteTableCommand)) {
+ && !(plan instanceof CreateTableCommand) && !(plan instanceof
InsertOverwriteTableCommand)
+ && !(plan instanceof
org.apache.doris.nereids.trees.plans.commands.merge.MergeIntoCommand)) {
Review Comment:
use import, not use qualified name
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/NereidsPlanner.java:
##########
@@ -740,6 +744,18 @@ protected void distribute(PhysicalPlan physicalPlan,
ExplainLevel explainLevel)
splitFragments(physicalPlan);
doDistribute(canUseNereidsDistributePlanner, explainLevel);
+
+ SessionVariable sessionVariable =
cascadesContext.getConnectContext().getSessionVariable();
+ if (sessionVariable.isEnableLocalShufflePlanner() &&
sessionVariable.isEnableLocalShuffle()) {
+ addLocalExchangeAfterDistribute();
+ }
Review Comment:
the if jugdement should move into `addLocalExchangeAfterDistribute`
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/NereidsPlanner.java:
##########
@@ -657,7 +660,8 @@ protected void splitFragments(PhysicalPlan resultPlan) {
scanNodeList.addAll(planTranslatorContext.getScanNodes());
physicalRelations.addAll(planTranslatorContext.getPhysicalRelations());
descTable = planTranslatorContext.getDescTable();
- fragments = new ArrayList<>(planTranslatorContext.getPlanFragments());
+ List<PlanFragment> planFragments =
planTranslatorContext.getPlanFragments();
Review Comment:
useless local variable
##########
fe/fe-core/src/main/java/org/apache/doris/planner/PlanNode.java:
##########
@@ -142,7 +145,8 @@ public abstract class PlanNode extends TreeNode<PlanNode> {
protected int nereidsId = -1;
- private List<List<Expr>> childrenDistributeExprLists = new ArrayList<>();
+ protected List<List<Expr>> childrenDistributeExprLists = new ArrayList<>();
+ protected List<Expr> distributeExprLists = new ArrayList<>();
Review Comment:
add comment to these two variables
--
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]
