You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by mo...@apache.org on 2022/06/30 10:03:26 UTC

[doris] 11/13: [hotfix](dev-1.0.1) fix inline view bug for vec engine

This is an automated email from the ASF dual-hosted git repository.

morningman pushed a commit to branch dev-1.0.1
in repository https://gitbox.apache.org/repos/asf/doris.git

commit 108085330a7f0ff8083f501e53e12ae92d74905b
Author: EmmyMiao87 <52...@qq.com>
AuthorDate: Fri Jun 24 12:29:02 2022 +0800

    [hotfix](dev-1.0.1) fix inline view bug for vec engine
---
 be/src/vec/exec/join/vhash_join_node.cpp           |  6 ++--
 .../java/org/apache/doris/analysis/Analyzer.java   | 13 +++++--
 .../apache/doris/analysis/ExprSubstitutionMap.java |  4 +++
 .../java/org/apache/doris/analysis/FromClause.java | 23 ++++++++++++-
 .../org/apache/doris/analysis/InlineViewRef.java   |  1 +
 .../org/apache/doris/planner/HashJoinNode.java     | 40 ++++++++++++++++++++--
 .../apache/doris/planner/SingleNodePlanner.java    | 12 +++----
 7 files changed, 84 insertions(+), 15 deletions(-)

diff --git a/be/src/vec/exec/join/vhash_join_node.cpp b/be/src/vec/exec/join/vhash_join_node.cpp
index 37d31a0a13..c8d7f6e77c 100644
--- a/be/src/vec/exec/join/vhash_join_node.cpp
+++ b/be/src/vec/exec/join/vhash_join_node.cpp
@@ -1172,15 +1172,15 @@ Status HashJoinNode::_extract_probe_join_column(Block& block, NullMap& null_map,
 
 Status HashJoinNode::_process_build_block(RuntimeState* state, Block& block, uint8_t offset) {
     SCOPED_TIMER(_build_table_timer);
+    if (_join_op == TJoinOp::LEFT_OUTER_JOIN || _join_op == TJoinOp::FULL_OUTER_JOIN) {
+        _convert_block_to_null(block);
+    }
     size_t rows = block.rows();
     if (UNLIKELY(rows == 0)) {
         return Status::OK();
     }
     COUNTER_UPDATE(_build_rows_counter, rows);
 
-    if (_join_op == TJoinOp::LEFT_OUTER_JOIN || _join_op == TJoinOp::FULL_OUTER_JOIN) {
-        _convert_block_to_null(block);
-    }
     ColumnRawPtrs raw_ptrs(_build_expr_ctxs.size());
 
     NullMap null_map_val(rows);
diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/Analyzer.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/Analyzer.java
index d05cb2685d..fa40b57d2b 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/analysis/Analyzer.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/Analyzer.java
@@ -217,8 +217,9 @@ public class Analyzer {
         private final Map<TupleId, List<ExprId>> eqJoinConjuncts = Maps.newHashMap();
 
         // set of conjuncts that have been assigned to some PlanNode
-        private Set<ExprId> assignedConjuncts =
-                Collections.newSetFromMap(new IdentityHashMap<ExprId, Boolean>());
+        private Set<ExprId> assignedConjuncts = Collections.newSetFromMap(new IdentityHashMap<ExprId, Boolean>());
+
+        private Set<TupleId> inlineViewTupleIds = Sets.newHashSet();
 
         // map from outer-joined tuple id, ie, one that is nullable in this select block,
         // to the last Join clause (represented by its rhs table ref) that outer-joined it
@@ -841,6 +842,10 @@ public class Analyzer {
         return result;
     }
 
+    public void registerInlineViewTupleId(TupleId tupleId) {
+        globalState.inlineViewTupleIds.add(tupleId);
+    }
+
     /**
      * Register conjuncts that are outer joined by a full outer join. For a given
      * predicate, we record the last full outer join that outer-joined any of its
@@ -2068,6 +2073,10 @@ public class Analyzer {
         return globalState.outerJoinedTupleIds.containsKey(tid);
     }
 
+    public boolean isInlineView(TupleId tid) {
+        return globalState.inlineViewTupleIds.contains(tid);
+    }
+
     public boolean containSubquery() {
         return globalState.containsSubquery;
     }
diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/ExprSubstitutionMap.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/ExprSubstitutionMap.java
index 20e68cf5c3..062eef4df8 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/analysis/ExprSubstitutionMap.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/ExprSubstitutionMap.java
@@ -106,6 +106,10 @@ public final class ExprSubstitutionMap {
         }
     }
 
+    public void updateLhsExprs(List<Expr> lhsExprList) {
+        lhs_ = lhsExprList;
+    }
+
     /**
      * Return a map  which is equivalent to applying f followed by g,
      * i.e., g(f()).
diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/FromClause.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/FromClause.java
index e77fd6ac21..fa6242f01f 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/analysis/FromClause.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/FromClause.java
@@ -147,8 +147,11 @@ public class FromClause implements ParseNode, Iterable<TableRef> {
             tblRef.analyze(analyzer);
             leftTblRef = tblRef;
         }
-
         checkExternalTable(analyzer);
+
+        // Fix the problem of column nullable attribute error caused by inline view + outer join
+        changeTblRefToNullable(analyzer);
+
         // TODO: remove when query from hive table is supported
         checkFromHiveTable(analyzer);
 
@@ -184,6 +187,24 @@ public class FromClause implements ParseNode, Iterable<TableRef> {
         }
     }
 
+    // set null-side inlinve view column
+    // For example: select * from (select a as k1 from t) tmp right join b on tmp.k1=b.k1
+    // The columns from tmp should be nullable.
+    // The table ref tmp will be used by HashJoinNode.computeOutputTuple()
+    private void changeTblRefToNullable(Analyzer analyzer) {
+        for (TableRef tableRef : tableRefs_) {
+            if (!(tableRef instanceof InlineViewRef)) {
+                continue;
+            }
+            InlineViewRef inlineViewRef = (InlineViewRef) tableRef;
+            if (analyzer.isOuterJoined(inlineViewRef.getId())) {
+                for (SlotDescriptor slotDescriptor : inlineViewRef.getDesc().getSlots()) {
+                    slotDescriptor.setIsNullable(true);
+                }
+            }
+        }
+    }
+
     public FromClause clone() {
         ArrayList<TableRef> clone = Lists.newArrayList();
         for (TableRef tblRef : tableRefs_) {
diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/InlineViewRef.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/InlineViewRef.java
index 7bf43b78b6..01791ed17f 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/analysis/InlineViewRef.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/InlineViewRef.java
@@ -286,6 +286,7 @@ public class InlineViewRef extends TableRef {
         TupleDescriptor result = analyzer.getDescTbl().createTupleDescriptor();
         result.setIsMaterialized(false);
         result.setTable(inlineView);
+        analyzer.registerInlineViewTupleId(result.getId());
         return result;
     }
 
diff --git a/fe/fe-core/src/main/java/org/apache/doris/planner/HashJoinNode.java b/fe/fe-core/src/main/java/org/apache/doris/planner/HashJoinNode.java
index 0534209275..52c623f3e8 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/planner/HashJoinNode.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/planner/HashJoinNode.java
@@ -28,10 +28,10 @@ import org.apache.doris.analysis.SlotRef;
 import org.apache.doris.analysis.TableRef;
 import org.apache.doris.analysis.TupleDescriptor;
 import org.apache.doris.analysis.TupleId;
+import org.apache.doris.analysis.TupleIsNullPredicate;
 import org.apache.doris.catalog.ColumnStats;
 import org.apache.doris.catalog.OlapTable;
 import org.apache.doris.catalog.Table;
-import org.apache.doris.common.AnalysisException;
 import org.apache.doris.common.CheckedMath;
 import org.apache.doris.common.NotImplementedException;
 import org.apache.doris.common.Pair;
@@ -320,7 +320,7 @@ public class HashJoinNode extends PlanNode {
         }
     }
 
-    private void computeOutputTuple(Analyzer analyzer) throws AnalysisException {
+    private void computeOutputTuple(Analyzer analyzer) throws UserException {
         // 1. create new tuple
         vOutputTupleDesc = analyzer.getDescTbl().createTupleDescriptor();
         boolean copyLeft = false;
@@ -362,6 +362,8 @@ public class HashJoinNode extends PlanNode {
                 break;
         }
         ExprSubstitutionMap srcTblRefToOutputTupleSmap = new ExprSubstitutionMap();
+        int leftNullableNumber = 0;
+        int rightNullableNumber = 0;
         if (copyLeft) {
             for (TupleDescriptor leftTupleDesc : analyzer.getDescTbl().getTupleDesc(getChild(0).getOutputTblRefIds())) {
                 for (SlotDescriptor leftSlotDesc : leftTupleDesc.getSlots()) {
@@ -372,6 +374,7 @@ public class HashJoinNode extends PlanNode {
                             analyzer.getDescTbl().copySlotDescriptor(vOutputTupleDesc, leftSlotDesc);
                     if (leftNullable) {
                         outputSlotDesc.setIsNullable(true);
+                        leftNullableNumber++;
                     }
                     srcTblRefToOutputTupleSmap.put(new SlotRef(leftSlotDesc), new SlotRef(outputSlotDesc));
                 }
@@ -388,6 +391,7 @@ public class HashJoinNode extends PlanNode {
                             analyzer.getDescTbl().copySlotDescriptor(vOutputTupleDesc, rightSlotDesc);
                     if (rightNullable) {
                         outputSlotDesc.setIsNullable(true);
+                        rightNullableNumber++;
                     }
                     srcTblRefToOutputTupleSmap.put(new SlotRef(rightSlotDesc), new SlotRef(outputSlotDesc));
                 }
@@ -406,7 +410,37 @@ public class HashJoinNode extends PlanNode {
             }
         }
         vOutputTupleDesc.computeStatAndMemLayout();
-        // 3. change the outputSmap
+        // 3. add tupleisnull in null-side
+        Preconditions.checkState(srcTblRefToOutputTupleSmap.getLhs().size() == vSrcToOutputSMap.getLhs().size());
+        // Condition1: the left child is null-side
+        // Condition2: the left child is a inline view
+        // Then: add tuple is null in left child columns
+        if (leftNullable && getChild(0).tblRefIds.size() == 1 && analyzer.isInlineView(getChild(0).tblRefIds.get(0))) {
+            List<Expr> tupleIsNullLhs = TupleIsNullPredicate
+                    .wrapExprs(vSrcToOutputSMap.getLhs().subList(0, leftNullableNumber), getChild(0).getTupleIds(),
+                            analyzer);
+            tupleIsNullLhs
+                    .addAll(vSrcToOutputSMap.getLhs().subList(leftNullableNumber, vSrcToOutputSMap.getLhs().size()));
+            vSrcToOutputSMap.updateLhsExprs(tupleIsNullLhs);
+        }
+        // Condition1: the right child is null-side
+        // Condition2: the right child is a inline view
+        // Then: add tuple is null in right child columns
+        if (rightNullable && getChild(1).tblRefIds.size() == 1 && analyzer.isInlineView(getChild(1).tblRefIds.get(0))) {
+            if (rightNullableNumber != 0) {
+                int rightBeginIndex = vSrcToOutputSMap.size() - rightNullableNumber;
+                List<Expr> tupleIsNullLhs = TupleIsNullPredicate
+                        .wrapExprs(vSrcToOutputSMap.getLhs().subList(rightBeginIndex, vSrcToOutputSMap.size()),
+                                getChild(1).getTupleIds(), analyzer);
+                List<Expr> newLhsList = Lists.newArrayList();
+                if (rightBeginIndex > 0) {
+                    newLhsList.addAll(vSrcToOutputSMap.getLhs().subList(0, rightBeginIndex));
+                }
+                newLhsList.addAll(tupleIsNullLhs);
+                vSrcToOutputSMap.updateLhsExprs(newLhsList);
+            }
+        }
+        // 4. change the outputSmap
         outputSmap = ExprSubstitutionMap.combineAndReplace(outputSmap, srcTblRefToOutputTupleSmap);
     }
 
diff --git a/fe/fe-core/src/main/java/org/apache/doris/planner/SingleNodePlanner.java b/fe/fe-core/src/main/java/org/apache/doris/planner/SingleNodePlanner.java
index 1f4a90a3ee..c804ed624f 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/planner/SingleNodePlanner.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/planner/SingleNodePlanner.java
@@ -59,6 +59,7 @@ import org.apache.doris.common.FeConstants;
 import org.apache.doris.common.Pair;
 import org.apache.doris.common.Reference;
 import org.apache.doris.common.UserException;
+import org.apache.doris.common.util.VectorizedUtil;
 
 import com.google.common.base.Preconditions;
 import com.google.common.base.Predicate;
@@ -66,7 +67,6 @@ import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
 import com.google.common.collect.Sets;
-
 import org.apache.commons.collections.CollectionUtils;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
@@ -1347,10 +1347,10 @@ public class SingleNodePlanner {
                 unionNode.addConstExprList(selectStmt.getBaseTblResultExprs());
                 unionNode.init(analyzer);
                 //set outputSmap to substitute literal in outputExpr
-                unionNode.setWithoutTupleIsNullOutputSmap(inlineViewRef.getSmap());
-                if (analyzer.isOuterJoined(inlineViewRef.getId())) {
-                    List<Expr> nullableRhs = TupleIsNullPredicate.wrapExprs(
-                            inlineViewRef.getSmap().getRhs(), unionNode.getTupleIds(), analyzer);
+                if (analyzer.isOuterJoined(inlineViewRef.getId()) && !VectorizedUtil.isVectorized()) {
+                    unionNode.setWithoutTupleIsNullOutputSmap(inlineViewRef.getSmap());
+                    List<Expr> nullableRhs = TupleIsNullPredicate
+                            .wrapExprs(inlineViewRef.getSmap().getRhs(), unionNode.getTupleIds(), analyzer);
                     unionNode.setOutputSmap(new ExprSubstitutionMap(inlineViewRef.getSmap().getLhs(), nullableRhs));
                 }
                 return unionNode;
@@ -1370,7 +1370,7 @@ public class SingleNodePlanner {
         ExprSubstitutionMap outputSmap = ExprSubstitutionMap.compose(
                 inlineViewRef.getSmap(), rootNode.getOutputSmap(), analyzer);
 
-        if (analyzer.isOuterJoined(inlineViewRef.getId())) {
+        if (analyzer.isOuterJoined(inlineViewRef.getId()) && !VectorizedUtil.isVectorized()) {
             rootNode.setWithoutTupleIsNullOutputSmap(outputSmap);
             // Exprs against non-matched rows of an outer join should always return NULL.
             // Make the rhs exprs of the output smap nullable, if necessary. This expr wrapping


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