You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2021/08/13 06:57:07 UTC

[GitHub] [incubator-doris] EmmyMiao87 commented on a change in pull request #6380: [BUG] Fixed the materialized number of resultExprs/constExprs and output slot of Union Node is inconsistent.

EmmyMiao87 commented on a change in pull request #6380:
URL: https://github.com/apache/incubator-doris/pull/6380#discussion_r688285440



##########
File path: fe/fe-core/src/main/java/org/apache/doris/planner/SetOperationNode.java
##########
@@ -125,6 +126,62 @@ public void addChild(PlanNode node, List<Expr> resultExprs) {
         return materializedConstExprLists_;
     }
 
+    @Override
+    public void finalize(Analyzer analyzer) throws UserException {
+        // In Doris-6380, moved computePassthrough() and the materialized position of resultExprs/constExprs from this.init()
+        // to this.finalize(), and will not call SetOperationNode::init() again at the end of createSetOperationNodeFragment().
+        //
+        // Reasons for move computePassthrough():
+        //      Because the byteSize of the tuple corresponding to OlapScanNode is updated after
+        //      singleNodePlanner.createSingleNodePlan() and before singleNodePlan.finalize(),
+        //      calling computePassthrough() in SetOperationNode::init() may not be able to accurately determine whether
+        //      the child is pass through. In the previous logic , Will call SetOperationNode::init() again
+        //      at the end of createSetOperationNodeFragment().
+        //
+        // Reasons for move materialized position of resultExprs/constExprs:
+        //      Because the output slot is materialized at various positions in the planner stage, this is to ensure that
+        //      eventually the resultExprs/constExprs and the corresponding output slot have the same materialized state.
+        //      And the order of materialized resultExprs must be the same as the order of child adjusted by
+        //      computePassthrough(), so resultExprs materialized must be placed after computePassthrough().
+
+        // except Node must not reorder the child
+        if (!(this instanceof ExceptNode)) {
+            computePassthrough(analyzer);
+        }
+        // drop resultExprs/constExprs that aren't getting materialized (= where the
+        // corresponding output slot isn't being materialized)
+        materializedResultExprLists_.clear();
+        Preconditions.checkState(resultExprLists_.size() == children.size());
+        List<SlotDescriptor> slots = analyzer.getDescTbl().getTupleDesc(tupleId_).getSlots();
+        for (int i = 0; i < resultExprLists_.size(); ++i) {
+            List<Expr> exprList = resultExprLists_.get(i);
+            List<Expr> newExprList = Lists.newArrayList();
+            Preconditions.checkState(exprList.size() == slots.size());
+            for (int j = 0; j < exprList.size(); ++j) {
+                if (slots.get(j).isMaterialized()) {
+                    newExprList.add(exprList.get(j));
+                }
+            }
+            materializedResultExprLists_.add(
+                    Expr.substituteList(newExprList, getChild(i).getOutputSmap(), analyzer, true));
+        }
+        Preconditions.checkState(
+                materializedResultExprLists_.size() == getChildren().size());
+
+        materializedConstExprLists_.clear();
+        for (List<Expr> exprList : constExprLists_) {
+            Preconditions.checkState(exprList.size() == slots.size());
+            List<Expr> newExprList = Lists.newArrayList();
+            for (int i = 0; i < exprList.size(); ++i) {
+                if (slots.get(i).isMaterialized()) {
+                    newExprList.add(exprList.get(i));
+                }
+            }
+            materializedConstExprLists_.add(newExprList);
+        }
+        super.finalize(analyzer);

Review comment:
        The super function is generally placed on the first line




-- 
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: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org