[calcite] branch master updated: [CALCITE-3113] Equivalent MutableAggregates with different row types should match with each other(Jin Xing)

[da...@apache.org]

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

danny0405 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/calcite.git


The following commit(s) were added to refs/heads/master by this push:
     new 0a87daf  [CALCITE-3113] Equivalent MutableAggregates with different row types should match with each other(Jin Xing)
0a87daf is described below

commit 0a87daf30d635277b8f1c94355e04ef907fc4002
Author: jinxing <ji...@gmail.com>
AuthorDate: Tue Jul 9 22:34:58 2019 +0800

    [CALCITE-3113] Equivalent MutableAggregates with different row types should match with each other(Jin Xing)
    
    The MaterializedView now can match with eath other if the row fields
    types equals but with different alias.
    
    close apache/calcite#1310
---
 .../apache/calcite/plan/SubstitutionVisitor.java   | 25 ++++++++++++++++++----
 .../apache/calcite/test/MaterializationTest.java   | 17 ++++++++++++++-
 2 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java b/core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java
index c832d55..4bbf8e9 100644
--- a/core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java
+++ b/core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java
@@ -495,7 +495,8 @@ public class SubstitutionVisitor {
     for (MutableRel targetDescendant : targetDescendants) {
       MutableRel queryDescendant = map.get(targetDescendant);
       if (queryDescendant != null) {
-        assert queryDescendant.rowType.equals(targetDescendant.rowType);
+        assert rowTypesAreEquivalent(
+            queryDescendant, targetDescendant, Litmus.THROW);
         equivalents.put(queryDescendant, targetDescendant);
       }
     }
@@ -532,7 +533,7 @@ public class SubstitutionVisitor {
               if (result != null) {
                 ++count;
                 attempted.add(new Replacement(result.call.query, result.result));
-                MutableRel parent = result.call.query.replaceInParent(result.result);
+                result.call.query.replaceInParent(result.result);
 
                 // Replace previous equivalents with new equivalents, higher up
                 // the tree.
@@ -542,8 +543,7 @@ public class SubstitutionVisitor {
                     equivalents.remove(slots[i], equi.iterator().next());
                   }
                 }
-                assert result.result.rowType.equals(result.call.query.rowType)
-                    : Pair.of(result.result, result.call.query);
+                assert rowTypesAreEquivalent(result.result, result.call.query, Litmus.THROW);
                 equivalents.put(result.result, result.call.query);
                 if (targetDescendant == target) {
                   // A real substitution happens. We purge the attempted
@@ -594,6 +594,23 @@ public class SubstitutionVisitor {
   }
 
   /**
+   * Equivalence checking for row types, but except for the field names.
+   */
+  private boolean rowTypesAreEquivalent(
+      MutableRel rel0, MutableRel rel1, Litmus litmus) {
+    if (rel0.rowType.getFieldCount() != rel1.rowType.getFieldCount()) {
+      return litmus.fail("Mismatch for column count: [{}]", Pair.of(rel0, rel1));
+    }
+    for (Pair<RelDataTypeField, RelDataTypeField> pair
+        : Pair.zip(rel0.rowType.getFieldList(), rel0.rowType.getFieldList())) {
+      if (!pair.left.getType().equals(pair.right.getType())) {
+        return litmus.fail("Mismatch for column type: [{}]", Pair.of(rel0, rel1));
+      }
+    }
+    return litmus.succeed();
+  }
+
+  /**
    * Represents a replacement action: before &rarr; after.
    */
   static class Replacement {
diff --git a/core/src/test/java/org/apache/calcite/test/MaterializationTest.java b/core/src/test/java/org/apache/calcite/test/MaterializationTest.java
index ad87c8d..405d3a1 100644
--- a/core/src/test/java/org/apache/calcite/test/MaterializationTest.java
+++ b/core/src/test/java/org/apache/calcite/test/MaterializationTest.java
@@ -529,7 +529,22 @@ public class MaterializationTest {
 
   /** Aggregation query at same level of aggregation as aggregation
    * materialization. */
-  @Test public void testAggregate() {
+  @Test public void testAggregate0() {
+    checkMaterialize(
+        "select count(*) as c from \"emps\" group by \"empid\"",
+        "select count(*) + 1 as c from \"emps\" group by \"empid\"");
+  }
+
+  /**
+   * Aggregation query at same level of aggregation as aggregation
+   * materialization but with different row types. */
+  @Test public void testAggregate1() {
+    checkMaterialize(
+        "select count(*) as c0 from \"emps\" group by \"empid\"",
+        "select count(*) as c1 from \"emps\" group by \"empid\"");
+  }
+
+  @Test public void testAggregate2() {
     checkMaterialize(
         "select \"deptno\", count(*) as c, sum(\"empid\") as s from \"emps\" group by \"deptno\"",
         "select count(*) + 1 as c, \"deptno\" from \"emps\" group by \"deptno\"");