You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by jh...@apache.org on 2018/02/25 05:29:32 UTC

calcite git commit: [CALCITE-2192] RelBuilder wrongly skips creating an Aggregate that prunes columns, if input is unique

Repository: calcite
Updated Branches:
  refs/heads/master 353a8be07 -> 3ce09bdb2


[CALCITE-2192] RelBuilder wrongly skips creating an Aggregate that prunes columns, if input is unique

Fix output in misc.iq (missed from [CALCITE-2183]).

Close apache/calcite#636


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

Branch: refs/heads/master
Commit: 3ce09bdb2adb01fb7b7abed0cc9f17a86ad448f5
Parents: 353a8be
Author: Jesus Camacho Rodriguez <jc...@apache.org>
Authored: Sat Feb 24 10:09:20 2018 -0800
Committer: Julian Hyde <jh...@apache.org>
Committed: Sat Feb 24 21:28:59 2018 -0800

----------------------------------------------------------------------
 .../org/apache/calcite/tools/RelBuilder.java    |  4 +-
 .../org/apache/calcite/test/RelBuilderTest.java | 56 ++++++++++++++++++++
 core/src/test/resources/sql/agg.iq              | 18 +++++++
 core/src/test/resources/sql/misc.iq             |  6 +++
 4 files changed, 82 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/calcite/blob/3ce09bdb/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/tools/RelBuilder.java b/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
index b4340f8..bdfcb59 100644
--- a/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
+++ b/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
@@ -1188,8 +1188,8 @@ public class RelBuilder {
       if (registrar.extraNodes.size() == fields().size()) {
         final Boolean unique = mq.areColumnsUnique(peek(), groupSet);
         if (unique != null && unique) {
-          // Rel is already unique. Nothing to do.
-          return this;
+          // Rel is already unique.
+          return project(fields(groupSet.asList()));
         }
       }
       final Double maxRowCount = mq.getMaxRowCount(peek());

http://git-wip-us.apache.org/repos/asf/calcite/blob/3ce09bdb/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java b/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java
index ab6ea1e..d04397f 100644
--- a/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java
+++ b/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java
@@ -659,6 +659,62 @@ public class RelBuilderTest {
             + "    LogicalTableScan(table=[[scott, EMP]])\n"));
   }
 
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-2192">[CALCITE-2192]
+   * RelBuilder wrongly skips creation of Aggregate that prunes columns if input
+   * is unique</a>. */
+  @Test public void testAggregate3() {
+    // Equivalent SQL:
+    //   SELECT DISTINCT deptno FROM (
+    //     SELECT deptno, COUNT(*)
+    //     FROM emp
+    //     GROUP BY deptno)
+    final RelBuilder builder = RelBuilder.create(config().build());
+    RelNode root =
+        builder.scan("EMP")
+            .aggregate(
+                builder.groupKey(builder.field(1)),
+                builder.aggregateCall(SqlStdOperatorTable.COUNT, false, false,
+                    null, "C"))
+            .aggregate(
+                builder.groupKey(builder.field(0)))
+            .build();
+    final String expected = ""
+        + "LogicalProject(ENAME=[$0])\n"
+        + "  LogicalAggregate(group=[{1}], C=[COUNT()])\n"
+        + "    LogicalTableScan(table=[[scott, EMP]])\n";
+    assertThat(str(root), is(expected));
+  }
+
+  /** As {@link #testAggregate3()} but with Filter. */
+  @Test public void testAggregate4() {
+    // Equivalent SQL:
+    //   SELECT DISTINCT deptno FROM (
+    //     SELECT deptno, COUNT(*)
+    //     FROM emp
+    //     GROUP BY deptno
+    //     HAVING COUNT(*) > 3)
+    final RelBuilder builder = RelBuilder.create(config().build());
+    RelNode root =
+        builder.scan("EMP")
+            .aggregate(
+                builder.groupKey(builder.field(1)),
+                builder.aggregateCall(SqlStdOperatorTable.COUNT, false, false,
+                    null, "C"))
+            .filter(
+                builder.call(SqlStdOperatorTable.GREATER_THAN, builder.field(1),
+                    builder.literal(3)))
+            .aggregate(
+                builder.groupKey(builder.field(0)))
+            .build();
+    final String expected = ""
+        + "LogicalProject(ENAME=[$0])\n"
+        + "  LogicalFilter(condition=[>($1, 3)])\n"
+        + "    LogicalAggregate(group=[{1}], C=[COUNT()])\n"
+        + "      LogicalTableScan(table=[[scott, EMP]])\n";
+    assertThat(str(root), is(expected));
+  }
+
   @Test public void testAggregateFilter() {
     // Equivalent SQL:
     //   SELECT deptno, COUNT(*) FILTER (WHERE empno > 100) AS c

http://git-wip-us.apache.org/repos/asf/calcite/blob/3ce09bdb/core/src/test/resources/sql/agg.iq
----------------------------------------------------------------------
diff --git a/core/src/test/resources/sql/agg.iq b/core/src/test/resources/sql/agg.iq
index a56ca09..b623ea1 100755
--- a/core/src/test/resources/sql/agg.iq
+++ b/core/src/test/resources/sql/agg.iq
@@ -129,6 +129,24 @@ select distinct count(*) as c from emp group by deptno order by 1 desc;
 
 !ok
 
+# [CALCITE-2192] RelBuilder wrongly skips creation of Aggregate that prunes
+# columns if input is unique
+select distinct deptno
+from (select deptno, count(*) from emp group by deptno);
++--------+
+| DEPTNO |
++--------+
+|     10 |
+|     20 |
+|     30 |
+|     50 |
+|     60 |
+|        |
++--------+
+(6 rows)
+
+!ok
+
 # [CALCITE-998] Exception when calling STDDEV_SAMP, STDDEV_POP
 # stddev_samp
 select stddev_samp(deptno) as s from emp;

http://git-wip-us.apache.org/repos/asf/calcite/blob/3ce09bdb/core/src/test/resources/sql/misc.iq
----------------------------------------------------------------------
diff --git a/core/src/test/resources/sql/misc.iq b/core/src/test/resources/sql/misc.iq
index 4e00ee7..d4c29e8 100644
--- a/core/src/test/resources/sql/misc.iq
+++ b/core/src/test/resources/sql/misc.iq
@@ -2131,6 +2131,12 @@ FROM (VALUES (1, 'X'),(2, 'Y'),(3, 'X'),(4, 'X')) AS T(A, B);
 select *
 from (values (1, 'a'), (2, 'b'), (1, 'b'), (2, 'c'), (2, 'c')) as t(x, y)
 where false;
++---+---+
+| X | Y |
++---+---+
++---+---+
+(0 rows)
+
 !ok
 
 # End misc.iq