calcite-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jcama...@apache.org
Subject calcite git commit: [CALCITE-1984] Incorrect rewriting with materialized views using DISTINCT in aggregate functions [Forced Update!]
Date Thu, 09 Nov 2017 15:34:10 GMT
Repository: calcite
Updated Branches:
  refs/heads/master a147d1a21 -> 20eee6313 (forced update)


[CALCITE-1984] Incorrect rewriting with materialized views using DISTINCT in aggregate functions


Project: http://git-wip-us.apache.org/repos/asf/calcite/repo
Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/20eee631
Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/20eee631
Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/20eee631

Branch: refs/heads/master
Commit: 20eee6313c87a149fb36392a8edf7ede2ec17705
Parents: 77a3549
Author: Jesus Camacho Rodriguez <jcamacho@apache.org>
Authored: Wed Nov 8 20:35:20 2017 -0800
Committer: Jesus Camacho Rodriguez <jcamacho@apache.org>
Committed: Thu Nov 9 07:33:56 2017 -0800

----------------------------------------------------------------------
 .../rel/rules/AbstractMaterializedViewRule.java | 16 ++++
 .../calcite/test/MaterializationTest.java       | 89 +++++++++++++++++++-
 2 files changed, 104 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/calcite/blob/20eee631/core/src/main/java/org/apache/calcite/rel/rules/AbstractMaterializedViewRule.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/rel/rules/AbstractMaterializedViewRule.java
b/core/src/main/java/org/apache/calcite/rel/rules/AbstractMaterializedViewRule.java
index e2ea81d..3d997cb 100644
--- a/core/src/main/java/org/apache/calcite/rel/rules/AbstractMaterializedViewRule.java
+++ b/core/src/main/java/org/apache/calcite/rel/rules/AbstractMaterializedViewRule.java
@@ -1053,9 +1053,17 @@ public abstract class AbstractMaterializedViewRule extends RelOptRule
{
                 rexBuilder.makeInputRef(relBuilder.peek(),
                     aggregate.getGroupCount() + i)));
       }
+      RelNode prevNode = relBuilder.peek();
       RelNode result = relBuilder
           .aggregate(relBuilder.groupKey(groupSet, null), aggregateCalls)
           .build();
+      if (prevNode == result && groupSet.cardinality() != result.getRowType().getFieldCount())
{
+        // Aggregate was not inserted but we need to prune columns
+        result = relBuilder
+            .push(result)
+            .project(relBuilder.fields(groupSet.asList()))
+            .build();
+      }
       if (topProject != null) {
         // Top project
         return topProject.copy(topProject.getTraitSet(), ImmutableList.of(result));
@@ -1271,10 +1279,18 @@ public abstract class AbstractMaterializedViewRule extends RelOptRule
{
             rewritingMapping.set(targetIdx, sourceIdx);
           }
         }
+        RelNode prevNode = result;
         result = relBuilder
             .push(result)
             .aggregate(relBuilder.groupKey(groupSet, null), aggregateCalls)
             .build();
+        if (prevNode == result && groupSet.cardinality() != result.getRowType().getFieldCount())
{
+          // Aggregate was not inserted but we need to prune columns
+          result = relBuilder
+              .push(result)
+              .project(relBuilder.fields(groupSet.asList()))
+              .build();
+        }
         // We introduce a project on top, as group by columns order is lost
         List<RexNode> projects = new ArrayList<>();
         Mapping inverseMapping = rewritingMapping.inverse();

http://git-wip-us.apache.org/repos/asf/calcite/blob/20eee631/core/src/test/java/org/apache/calcite/test/MaterializationTest.java
----------------------------------------------------------------------
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 20093f1..04cbe62 100644
--- a/core/src/test/java/org/apache/calcite/test/MaterializationTest.java
+++ b/core/src/test/java/org/apache/calcite/test/MaterializationTest.java
@@ -1582,6 +1582,19 @@ public class MaterializationTest {
         HR_FKUK_MODEL);
   }
 
+  @Test public void testJoinAggregateMaterializationAggregateFuncs13() {
+    checkNoMaterialize(
+        "select \"dependents\".\"empid\", \"emps\".\"deptno\", count(distinct \"salary\")
as s\n"
+            + "from \"emps\"\n"
+            + "join \"dependents\" on (\"emps\".\"empid\" = \"dependents\".\"empid\")\n"
+            + "group by \"dependents\".\"empid\", \"emps\".\"deptno\"",
+        "select \"emps\".\"deptno\", count(\"salary\") as s\n"
+            + "from \"emps\"\n"
+            + "join \"dependents\" on (\"emps\".\"empid\" = \"dependents\".\"empid\")\n"
+            + "group by \"dependents\".\"empid\", \"emps\".\"deptno\"",
+        HR_FKUK_MODEL);
+  }
+
   @Test public void testJoinMaterialization4() {
     checkMaterialize(
         "select \"empid\" \"deptno\" from \"emps\"\n"
@@ -1995,6 +2008,80 @@ public class MaterializationTest {
     }
   }
 
+  @Test public void testAggregateMaterializationOnCountDistinctQuery1() {
+    // The column empid is already unique, thus DISTINCT is not
+    // in the COUNT of the resulting rewriting
+    checkMaterialize(
+        "select \"deptno\", \"empid\", \"salary\"\n"
+            + "from \"emps\"\n"
+            + "group by \"deptno\", \"empid\", \"salary\"",
+        "select \"deptno\", count(distinct \"empid\") as c from (\n"
+            + "select \"deptno\", \"empid\"\n"
+            + "from \"emps\"\n"
+            + "group by \"deptno\", \"empid\")\n"
+            + "group by \"deptno\"",
+        HR_FKUK_MODEL,
+        CalciteAssert.checkResultContains(
+            "EnumerableAggregate(group=[{0}], C=[COUNT($1)])\n"
+                + "  EnumerableTableScan(table=[[hr, m0]]"));
+  }
+
+  @Test public void testAggregateMaterializationOnCountDistinctQuery2() {
+    // The column empid is already unique, thus DISTINCT is not
+    // in the COUNT of the resulting rewriting
+    checkMaterialize(
+        "select \"deptno\", \"salary\", \"empid\"\n"
+            + "from \"emps\"\n"
+            + "group by \"deptno\", \"salary\", \"empid\"",
+        "select \"deptno\", count(distinct \"empid\") as c from (\n"
+            + "select \"deptno\", \"empid\"\n"
+            + "from \"emps\"\n"
+            + "group by \"deptno\", \"empid\")\n"
+            + "group by \"deptno\"",
+        HR_FKUK_MODEL,
+        CalciteAssert.checkResultContains(
+            "EnumerableAggregate(group=[{0}], C=[COUNT($2)])\n"
+                + "  EnumerableTableScan(table=[[hr, m0]]"));
+  }
+
+  @Test public void testAggregateMaterializationOnCountDistinctQuery3() {
+    // The column salary is not unique, thus we end up with
+    // a different rewriting
+    checkMaterialize(
+        "select \"deptno\", \"empid\", \"salary\"\n"
+            + "from \"emps\"\n"
+            + "group by \"deptno\", \"empid\", \"salary\"",
+        "select \"deptno\", count(distinct \"salary\") from (\n"
+            + "select \"deptno\", \"salary\"\n"
+            + "from \"emps\"\n"
+            + "group by \"deptno\", \"salary\")\n"
+            + "group by \"deptno\"",
+        HR_FKUK_MODEL,
+        CalciteAssert.checkResultContains(
+            "EnumerableAggregate(group=[{0}], EXPR$1=[COUNT($1)])\n"
+                + "  EnumerableAggregate(group=[{0, 2}])\n"
+                + "    EnumerableTableScan(table=[[hr, m0]]"));
+  }
+
+  @Test public void testAggregateMaterializationOnCountDistinctQuery4() {
+    // Although there is no DISTINCT in the COUNT, this is
+    // equivalent to previous query
+    checkMaterialize(
+        "select \"deptno\", \"salary\", \"empid\"\n"
+            + "from \"emps\"\n"
+            + "group by \"deptno\", \"salary\", \"empid\"",
+        "select \"deptno\", count(\"salary\") from (\n"
+            + "select \"deptno\", \"salary\"\n"
+            + "from \"emps\"\n"
+            + "group by \"deptno\", \"salary\")\n"
+            + "group by \"deptno\"",
+        HR_FKUK_MODEL,
+        CalciteAssert.checkResultContains(
+            "EnumerableAggregate(group=[{0}], EXPR$1=[COUNT()])\n"
+                + "  EnumerableAggregate(group=[{0, 1}])\n"
+                + "    EnumerableTableScan(table=[[hr, m0]]"));
+  }
+
   @Test public void testMaterializationSubstitution() {
     String q = "select *\n"
         + "from (select * from \"emps\" where \"empid\" < 300)\n"
@@ -2127,7 +2214,7 @@ public class MaterializationTest {
         new Employee(100, 10, "Bill", 10000, 1000),
         new Employee(200, 20, "Eric", 8000, 500),
         new Employee(150, 10, "Sebastian", 7000, null),
-        new Employee(110, 10, "Theodore", 11500, 250),
+        new Employee(110, 10, "Theodore", 10000, 250),
     };
     public final Department[] depts = {
         new Department(10, "Sales", Arrays.asList(emps[0], emps[2], emps[3]),


Mime
View raw message