You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by GitBox <gi...@apache.org> on 2021/02/20 16:38:35 UTC

[GitHub] [hive] kasakrisz commented on a change in pull request #1995: HIVE-24775: Incorrect null handling when rebuilding Materialized view incrementally

kasakrisz commented on a change in pull request #1995:
URL: https://github.com/apache/hive/pull/1995#discussion_r579678432



##########
File path: ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/HiveAggregateIncrementalRewritingRule.java
##########
@@ -99,31 +100,61 @@ public void onMatch(RelOptRuleCall call) {
     final RexBuilder rexBuilder =
         agg.getCluster().getRexBuilder();
     // 1) First branch is query, second branch is MV
-    final RelNode joinLeftInput = union.getInput(1);
+    RelNode joinLeftInput = union.getInput(1);
     final RelNode joinRightInput = union.getInput(0);
-    // 2) Build conditions for join and filter and start adding
+
+    // 2) Introduce a Project on top of MV scan having all columns from the view plus a boolean literal which indicates
+    // whether the row with the key values coming from the joinRightInput exists in the view:
+    // - true means exist
+    // - null means not exists
+    // Project also needed to encapsulate the view scan by a subquery -> this is required by
+    // CalcitePlanner.fixUpASTAggregateIncrementalRebuild
+    List<RexNode> mvCols = new ArrayList<>(joinLeftInput.getRowType().getFieldCount());
+    for (int i = 0; i < joinLeftInput.getRowType().getFieldCount(); ++i) {
+      mvCols.add(rexBuilder.makeInputRef(
+              joinLeftInput.getRowType().getFieldList().get(i).getType(), i));
+    }
+    mvCols.add(rexBuilder.makeLiteral(true));
+
+    joinLeftInput = call.builder()
+            .push(joinLeftInput)
+            .project(mvCols)
+            .build();
+
+    // 3) Build conditions for join and start adding
     // expressions for project operator
     List<RexNode> projExprs = new ArrayList<>();
     List<RexNode> joinConjs = new ArrayList<>();
-    List<RexNode> filterConjs = new ArrayList<>();
     int groupCount = agg.getGroupCount();
     int totalCount = agg.getGroupCount() + agg.getAggCallList().size();
-    for (int leftPos = 0, rightPos = totalCount;
+    for (int leftPos = 0, rightPos = totalCount + 1;
          leftPos < groupCount; leftPos++, rightPos++) {
       RexNode leftRef = rexBuilder.makeInputRef(
           joinLeftInput.getRowType().getFieldList().get(leftPos).getType(), leftPos);
       RexNode rightRef = rexBuilder.makeInputRef(
           joinRightInput.getRowType().getFieldList().get(leftPos).getType(), rightPos);
       projExprs.add(rightRef);
-      joinConjs.add(rexBuilder.makeCall(SqlStdOperatorTable.EQUALS,
-          ImmutableList.of(leftRef, rightRef)));
-      filterConjs.add(rexBuilder.makeCall(SqlStdOperatorTable.IS_NULL,
-          ImmutableList.of(leftRef)));
+
+      RexNode nsEqExpr = rexBuilder.makeCall(SqlStdOperatorTable.IS_NOT_DISTINCT_FROM,
+              ImmutableList.of(leftRef, rightRef));
+      joinConjs.add(nsEqExpr);
     }
-    // 3) Add the expressions that correspond to the aggregation
+
+    // 4) Create join node
+    RexNode joinCond = RexUtil.composeConjunction(rexBuilder, joinConjs);
+    RelNode join = call.builder()
+            .push(joinLeftInput)
+            .push(joinRightInput)
+            .join(JoinRelType.RIGHT, joinCond)
+            .build();
+
+    int flagIndex = joinLeftInput.getRowType().getFieldCount() - 1;
+    RexNode flagNode = rexBuilder.makeInputRef(
+            join.getRowType().getFieldList().get(flagIndex).getType(), flagIndex);
+
+    // 5) Add the expressions that correspond to the aggregation
     // functions
-    RexNode caseFilterCond = RexUtil.composeConjunction(rexBuilder, filterConjs);
-    for (int i = 0, leftPos = groupCount, rightPos = totalCount + groupCount;
+    for (int i = 0, leftPos = groupCount, rightPos = totalCount + 1 + groupCount;
          leftPos < totalCount; i++, leftPos++, rightPos++) {
       // case when mv2.deptno IS NULL AND mv2.deptname IS NULL then s else source.s + mv2.s end

Review comment:
       Unfortunately relying on the flag only is not enough.
   Aggregate functions eliminates null values:
   ```
   insert into t1(a,b) values 
   (1, NULL),
   (1, 10);
   select a, sum(b) from t1 group by a
   ```
   results
   ```
   1, 10
   ```
   However
   ```
   select 10 + NULL
   ```
   is `NULL`.
   
   The comment is not valid any more. I will update it.




----------------------------------------------------------------
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org