Repository: calcite Updated Branches: refs/heads/master 81691cef5 -> a147d1a21 (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/a147d1a2 Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/a147d1a2 Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/a147d1a2 Branch: refs/heads/master Commit: a147d1a21e68148ad591a690fc31ebe6de7656ea Parents: 77a3549 Author: Jesus Camacho Rodriguez <[email protected]> Authored: Wed Nov 8 20:35:20 2017 -0800 Committer: Jesus Camacho Rodriguez <[email protected]> Committed: Thu Nov 9 07:24:58 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/a147d1a2/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..29e98af 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.size() != 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.size() != 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/a147d1a2/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]),
