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 2015/12/15 19:55:30 UTC

calcite git commit: [CALCITE-1015] OFFSET 0 causes AssertionError (Zhen Wang)

Repository: calcite
Updated Branches:
  refs/heads/master 5197a7147 -> f12aa72ec


[CALCITE-1015] OFFSET 0 causes AssertionError (Zhen Wang)

Close apache/calcite#181


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

Branch: refs/heads/master
Commit: f12aa72ec79f8300be6441c1afd83c908f9fe0be
Parents: 5197a71
Author: zhen wang <zi...@gmail.com>
Authored: Tue Dec 15 11:57:43 2015 +0800
Committer: Julian Hyde <jh...@apache.org>
Committed: Mon Dec 14 21:08:28 2015 -0800

----------------------------------------------------------------------
 .../calcite/sql2rel/SqlToRelConverter.java      |  7 +++++--
 .../org/apache/calcite/tools/RelBuilder.java    |  4 ++++
 .../java/org/apache/calcite/test/JdbcTest.java  | 21 ++++++++++++++++++++
 .../org/apache/calcite/test/RelBuilderTest.java | 17 ++++++++++++++++
 .../calcite/test/SqlToRelConverterTest.java     |  4 ++++
 .../calcite/test/SqlToRelConverterTest.xml      | 11 ++++++++++
 core/src/test/resources/sql/sort.iq             | 16 +++++++++++++++
 7 files changed, 78 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/calcite/blob/f12aa72e/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java b/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java
index b67fa72..2e87b53 100644
--- a/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java
+++ b/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java
@@ -814,9 +814,12 @@ public class SqlToRelConverter {
       List<SqlNode> orderExprList,
       SqlNode offset,
       SqlNode fetch) {
-    if (select.getOrderList() == null) {
+    if (select.getOrderList() == null
+        || select.getOrderList().getList().isEmpty()) {
       assert collation.getFieldCollations().isEmpty();
-      if (offset == null && fetch == null) {
+      if ((offset == null
+            || ((SqlLiteral) offset).bigDecimalValue().equals(BigDecimal.ZERO))
+          && fetch == null) {
         return;
       }
     }

http://git-wip-us.apache.org/repos/asf/calcite/blob/f12aa72e/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 753a0af..2cd786c 100644
--- a/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
+++ b/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
@@ -1223,6 +1223,10 @@ public class RelBuilder {
     }
     final RexNode offsetNode = offset <= 0 ? null : literal(offset);
     final RexNode fetchNode = fetch < 0 ? null : literal(fetch);
+    if (offsetNode == null && fetchNode == null && fieldCollations.isEmpty()) {
+      return this; // sort is trivial
+    }
+
     final boolean addedFields = extraNodes.size() > originalExtraNodes.size();
     if (fieldCollations.isEmpty()) {
       assert !addedFields;

http://git-wip-us.apache.org/repos/asf/calcite/blob/f12aa72e/core/src/test/java/org/apache/calcite/test/JdbcTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/test/JdbcTest.java b/core/src/test/java/org/apache/calcite/test/JdbcTest.java
index 0eccd4a..d5ac9ba 100644
--- a/core/src/test/java/org/apache/calcite/test/JdbcTest.java
+++ b/core/src/test/java/org/apache/calcite/test/JdbcTest.java
@@ -4330,6 +4330,27 @@ public class JdbcTest {
             "should fail with 'not a number' sql error while converting text to number");
   }
 
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-1015">[CALCITE-1015]
+   * OFFSET 0 causes AssertionError</a>. */
+  @Test public void testTrivialSort() {
+    final String sql = "select a.\"value\", b.\"value\"\n"
+        + "  from \"bools\" a\n"
+        + "     , \"bools\" b\n"
+        + " offset 0";
+    CalciteAssert.that()
+        .withSchema("s",
+            new ReflectiveSchema(
+                new ReflectiveSchemaTest.CatchallSchema()))
+        .query(sql)
+        .returnsUnordered("value=T; value=T",
+            "value=T; value=F",
+            "value=T; value=null",
+            "value=F; value=T",
+            "value=F; value=F",
+            "value=F; value=null");
+  }
+
   /** Tests the LIKE operator. */
   @Test public void testLike() {
     CalciteAssert.that()

http://git-wip-us.apache.org/repos/asf/calcite/blob/f12aa72e/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 84b42fd..82757ab 100644
--- a/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java
+++ b/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java
@@ -913,6 +913,23 @@ public class RelBuilderTest {
     assertThat(str(root2), is(expected));
   }
 
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-1015">[CALCITE-1015]
+   * OFFSET 0 causes AssertionError</a>. */
+  @Test public void testTrivialSort() {
+    // Equivalent SQL:
+    //   SELECT *
+    //   FROM emp
+    //   OFFSET 0
+    final RelBuilder builder = RelBuilder.create(config().build());
+    final RelNode root =
+        builder.scan("EMP")
+            .sortLimit(0, -1, ImmutableList.<RexNode>of())
+            .build();
+    final String expected = "LogicalTableScan(table=[[scott, EMP]])\n";
+    assertThat(str(root), is(expected));
+  }
+
   @Test public void testSortByExpression() {
     // Equivalent SQL:
     //   SELECT *

http://git-wip-us.apache.org/repos/asf/calcite/blob/f12aa72e/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java b/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
index 10cdb4e..314446e 100644
--- a/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
+++ b/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
@@ -1124,6 +1124,10 @@ public class SqlToRelConverterTest extends SqlToRelTestBase {
         true);
   }
 
+  @Test public void testOffset0() {
+    tester.assertConvertsTo("select * from emp offset 0", "${plan}");
+  }
+
   /**
    * Test group-by CASE expression involving a non-query IN
    */

http://git-wip-us.apache.org/repos/asf/calcite/blob/f12aa72e/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml
----------------------------------------------------------------------
diff --git a/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml b/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml
index 4500149..c46d493 100644
--- a/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml
+++ b/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml
@@ -2631,4 +2631,15 @@ LogicalTableModify(table=[[CATALOG, SALES, EMP]], operation=[UPDATE], updateColu
 ]]>
         </Resource>
     </TestCase>
+    <TestCase name="testOffset0">
+        <Resource name="sql">
+            <![CDATA[select * from emp offset 0]]>
+        </Resource>
+        <Resource name="plan">
+            <![CDATA[
+LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4], SAL=[$5], COMM=[$6], DEPTNO=[$7], SLACKER=[$8])
+  LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+]]>
+        </Resource>
+    </TestCase>
 </Root>

http://git-wip-us.apache.org/repos/asf/calcite/blob/f12aa72e/core/src/test/resources/sql/sort.iq
----------------------------------------------------------------------
diff --git a/core/src/test/resources/sql/sort.iq b/core/src/test/resources/sql/sort.iq
index 0be2add..417fc96 100644
--- a/core/src/test/resources/sql/sort.iq
+++ b/core/src/test/resources/sql/sort.iq
@@ -147,6 +147,22 @@ select * from e where empid > 100 limit 5;
 
 !ok
 
+# [CALCITE-1015] OFFSET 0 causes AssertionError
+select * from "hr"."emps" offset 0;
++-------+--------+-----------+---------+------------+
+| empid | deptno | name      | salary  | commission |
++-------+--------+-----------+---------+------------+
+|   100 |     10 | Bill      | 10000.0 |       1000 |
+|   110 |     10 | Theodore  | 11500.0 |        250 |
+|   150 |     10 | Sebastian |  7000.0 |            |
+|   200 |     20 | Eric      |  8000.0 |        500 |
++-------+--------+-----------+---------+------------+
+(4 rows)
+
+!ok
+EnumerableTableScan(table=[[hr, emps]])
+!plan
+
 # [CALCITE-634] Allow ORDER BY aggregate function in SELECT DISTINCT, provided
 # that it occurs in SELECT clause
 select distinct "deptno", count(*) as c