You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by vl...@apache.org on 2015/07/22 22:42:13 UTC

incubator-calcite git commit: [CALCITE-806] ROW_NUMBER should emit distinct values

Repository: incubator-calcite
Updated Branches:
  refs/heads/master 4b60b9bf4 -> 096d2820b


[CALCITE-806] ROW_NUMBER should emit distinct values

Close apache/incubator-calcite#107


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

Branch: refs/heads/master
Commit: 096d2820b6a3c48e0fa9e5f7d5489a25aeff10a5
Parents: 4b60b9b
Author: Julian Hyde <jh...@apache.org>
Authored: Sun Jul 19 02:56:28 2015 +0300
Committer: Vladimir Sitnikov <si...@gmail.com>
Committed: Wed Jul 22 23:41:27 2015 +0300

----------------------------------------------------------------------
 .../apache/calcite/runtime/CalciteResource.java |  2 +-
 .../java/org/apache/calcite/sql/SqlWindow.java  |  4 ++
 .../calcite/sql2rel/SqlToRelConverter.java      |  6 +++
 .../calcite/runtime/CalciteResource.properties  |  2 +-
 .../java/org/apache/calcite/test/JdbcTest.java  | 22 +++++---
 .../apache/calcite/test/SqlValidatorTest.java   | 19 ++++---
 core/src/test/resources/sql/winagg.oq           | 54 ++++++++++++++++++--
 pom.xml                                         |  2 +-
 8 files changed, 91 insertions(+), 20 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-calcite/blob/096d2820/core/src/main/java/org/apache/calcite/runtime/CalciteResource.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/runtime/CalciteResource.java b/core/src/main/java/org/apache/calcite/runtime/CalciteResource.java
index 007a410..a511143 100644
--- a/core/src/main/java/org/apache/calcite/runtime/CalciteResource.java
+++ b/core/src/main/java/org/apache/calcite/runtime/CalciteResource.java
@@ -365,7 +365,7 @@ public interface CalciteResource {
   @BaseMessage("Duplicate window specification not allowed in the same window clause")
   ExInst<SqlValidatorException> dupWindowSpec();
 
-  @BaseMessage("ROW/RANGE not allowed with RANK or DENSE_RANK functions")
+  @BaseMessage("ROW/RANGE not allowed with RANK, DENSE_RANK or ROW_NUMBER functions")
   ExInst<SqlValidatorException> rankWithFrame();
 
   @BaseMessage("RANK or DENSE_RANK functions require ORDER BY clause in window specification")

http://git-wip-us.apache.org/repos/asf/incubator-calcite/blob/096d2820/core/src/main/java/org/apache/calcite/sql/SqlWindow.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/sql/SqlWindow.java b/core/src/main/java/org/apache/calcite/sql/SqlWindow.java
index 40ba4ee..b54b349 100644
--- a/core/src/main/java/org/apache/calcite/sql/SqlWindow.java
+++ b/core/src/main/java/org/apache/calcite/sql/SqlWindow.java
@@ -247,6 +247,10 @@ public class SqlWindow extends SqlCall {
     return false;
   }
 
+  public void setRows(SqlLiteral isRows) {
+    this.isRows = isRows;
+  }
+
   public boolean isRows() {
     return isRows.booleanValue();
   }

http://git-wip-us.apache.org/repos/asf/incubator-calcite/blob/096d2820/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 fe16f23..40e6ed2 100644
--- a/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java
+++ b/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java
@@ -1747,6 +1747,12 @@ public class SqlToRelConverter {
     SqlNode windowOrRef = call.operand(1);
     final SqlWindow window =
         validator.resolveWindow(windowOrRef, bb.scope, true);
+    // ROW_NUMBER() expects specific kind of framing.
+    if (aggCall.getOperator() == SqlStdOperatorTable.ROW_NUMBER) {
+      window.setLowerBound(SqlWindow.createUnboundedPreceding(SqlParserPos.ZERO));
+      window.setUpperBound(SqlWindow.createCurrentRow(SqlParserPos.ZERO));
+      window.setRows(SqlLiteral.createBoolean(true, SqlParserPos.ZERO));
+    }
     final SqlNodeList partitionList = window.getPartitionList();
     final ImmutableList.Builder<RexNode> partitionKeys =
         ImmutableList.builder();

http://git-wip-us.apache.org/repos/asf/incubator-calcite/blob/096d2820/core/src/main/resources/org/apache/calcite/runtime/CalciteResource.properties
----------------------------------------------------------------------
diff --git a/core/src/main/resources/org/apache/calcite/runtime/CalciteResource.properties b/core/src/main/resources/org/apache/calcite/runtime/CalciteResource.properties
index 3aec262..5ab24cc 100644
--- a/core/src/main/resources/org/apache/calcite/runtime/CalciteResource.properties
+++ b/core/src/main/resources/org/apache/calcite/runtime/CalciteResource.properties
@@ -124,7 +124,7 @@ WindowNameMustBeSimple=Window name must be a simple identifier
 DuplicateWindowName=Duplicate window names not allowed
 EmptyWindowSpec=Empty window specification not allowed
 DupWindowSpec=Duplicate window specification not allowed in the same window clause
-RankWithFrame=ROW/RANGE not allowed with RANK or DENSE_RANK functions
+RankWithFrame=ROW/RANGE not allowed with RANK, DENSE_RANK or ROW_NUMBER functions
 FuncNeedsOrderBy=RANK or DENSE_RANK functions require ORDER BY clause in window specification
 PartitionNotAllowed=PARTITION BY not allowed with existing window reference
 OrderByOverlap=ORDER BY not allowed in both base and referenced windows

http://git-wip-us.apache.org/repos/asf/incubator-calcite/blob/096d2820/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 a037a23..f365ab2 100644
--- a/core/src/test/java/org/apache/calcite/test/JdbcTest.java
+++ b/core/src/test/java/org/apache/calcite/test/JdbcTest.java
@@ -4022,18 +4022,19 @@ public class JdbcTest {
         .query("select \"deptno\",\n"
             + " \"empid\",\n"
             + " \"commission\",\n"
+            + " row_number() over (partition by \"deptno\") as r,\n"
             + " row_number() over (partition by \"deptno\" order by \"commission\" desc nulls first) as rcnf,\n"
             + " row_number() over (partition by \"deptno\" order by \"commission\" desc nulls last) as rcnl,\n"
             + " row_number() over (partition by \"deptno\" order by \"empid\") as r,\n"
             + " row_number() over (partition by \"deptno\" order by \"empid\" desc) as rd\n"
             + "from \"hr\".\"emps\"")
         .typeIs(
-            "[deptno INTEGER NOT NULL, empid INTEGER NOT NULL, commission INTEGER, RCNF INTEGER NOT NULL, RCNL INTEGER NOT NULL, R INTEGER NOT NULL, RD INTEGER NOT NULL]")
+            "[deptno INTEGER NOT NULL, empid INTEGER NOT NULL, commission INTEGER, R INTEGER NOT NULL, RCNF INTEGER NOT NULL, RCNL INTEGER NOT NULL, R INTEGER NOT NULL, RD INTEGER NOT NULL]")
         .returnsUnordered(
-            "deptno=10; empid=100; commission=1000; RCNF=2; RCNL=1; R=1; RD=3",
-            "deptno=10; empid=110; commission=250; RCNF=3; RCNL=2; R=2; RD=2",
-            "deptno=10; empid=150; commission=null; RCNF=1; RCNL=3; R=3; RD=1",
-            "deptno=20; empid=200; commission=500; RCNF=1; RCNL=1; R=1; RD=1");
+            "deptno=10; empid=100; commission=1000; R=1; RCNF=2; RCNL=1; R=1; RD=3",
+            "deptno=10; empid=110; commission=250; R=3; RCNF=3; RCNL=2; R=2; RD=2",
+            "deptno=10; empid=150; commission=null; R=2; RCNF=1; RCNL=3; R=3; RD=1",
+            "deptno=20; empid=200; commission=500; R=1; RCNF=1; RCNL=1; R=1; RD=1");
   }
 
   /** Tests UNBOUNDED PRECEDING clause. */
@@ -4595,7 +4596,16 @@ public class JdbcTest {
     final FileReader fileReader = new FileReader(inFile);
     final BufferedReader bufferedReader = new BufferedReader(fileReader);
     final FileWriter writer = new FileWriter(outFile);
-    final Quidem quidem = new Quidem(bufferedReader, writer);
+    final Function<String, Object> env =
+        new Function<String, Object>() {
+          public Object apply(String varName) {
+            if (varName.equals("jdk18")) {
+              return System.getProperty("java.version").startsWith("1.8");
+            }
+            return null;
+          }
+        };
+    final Quidem quidem = new Quidem(bufferedReader, writer, env);
     quidem.execute(
         new Quidem.ConnectionFactory() {
           public Connection connect(String name) throws Exception {

http://git-wip-us.apache.org/repos/asf/incubator-calcite/blob/096d2820/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
index 76413ec..106ad92 100644
--- a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
+++ b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
@@ -107,6 +107,9 @@ public class SqlValidatorTest extends SqlValidatorTestCase {
   private static final String STR_SET_OP_INCONSISTENT =
       "Set operator cannot combine streaming and non-streaming inputs";
 
+  private static final String ROW_RANGE_NOT_ALLOWED_WITH_RANK =
+      "ROW/RANGE not allowed with RANK, DENSE_RANK or ROW_NUMBER functions";
+
   //~ Constructors -----------------------------------------------------------
 
   public SqlValidatorTest() {
@@ -3891,9 +3894,9 @@ public class SqlValidatorTest extends SqlValidatorTestCase {
     winExp("row_number() over (partition by deptno)").ok();
     winExp("row_number() over ()").ok();
     winExp("row_number() over (order by deptno ^rows^ 2 preceding)")
-        .fails("ROW/RANGE not allowed with RANK or DENSE_RANK functions");
+        .fails(ROW_RANGE_NOT_ALLOWED_WITH_RANK);
     winExp("row_number() over (order by deptno ^range^ 2 preceding)")
-        .fails("ROW/RANGE not allowed with RANK or DENSE_RANK functions");
+        .fails(ROW_RANGE_NOT_ALLOWED_WITH_RANK);
 
     // rank function type
     if (defined.contains("DENSE_RANK")) {
@@ -3928,10 +3931,10 @@ public class SqlValidatorTest extends SqlValidatorTestCase {
     // window framing defined in window clause
     winSql(
         "select rank() over w from emp window w as (order by empno ^rows^ 2 preceding )")
-        .fails("ROW/RANGE not allowed with RANK or DENSE_RANK functions");
+        .fails(ROW_RANGE_NOT_ALLOWED_WITH_RANK);
     winSql(
         "select dense_rank() over w from emp window w as (order by empno ^rows^ 2 preceding)")
-        .fails("ROW/RANGE not allowed with RANK or DENSE_RANK functions");
+        .fails(ROW_RANGE_NOT_ALLOWED_WITH_RANK);
     if (defined.contains("PERCENT_RANK")) {
       winSql("select percent_rank() over w from emp\n"
           + "window w as (order by empno)")
@@ -3939,7 +3942,7 @@ public class SqlValidatorTest extends SqlValidatorTestCase {
       winSql(
           "select percent_rank() over w from emp\n"
           + "window w as (order by empno ^rows^ 2 preceding)")
-          .fails("ROW/RANGE not allowed with RANK or DENSE_RANK functions");
+          .fails(ROW_RANGE_NOT_ALLOWED_WITH_RANK);
       winSql(
           "select percent_rank() over w from emp\n"
           + "window w as ^(partition by empno)^")
@@ -3957,7 +3960,7 @@ public class SqlValidatorTest extends SqlValidatorTestCase {
               "RANK or DENSE_RANK functions require ORDER BY clause in window specification");
       winSql(
           "select cume_dist() over w from emp window w as (order by empno ^rows^ 2 preceding)")
-          .fails("ROW/RANGE not allowed with RANK or DENSE_RANK functions");
+          .fails(ROW_RANGE_NOT_ALLOWED_WITH_RANK);
       winSql(
           "select cume_dist() over w from emp window w as (order by empno)")
           .ok();
@@ -3969,10 +3972,10 @@ public class SqlValidatorTest extends SqlValidatorTestCase {
     }
     // window framing defined in in-line window
     winSql("select rank() over (order by empno ^range^ 2 preceding ) from emp ")
-        .fails("ROW/RANGE not allowed with RANK or DENSE_RANK functions");
+        .fails(ROW_RANGE_NOT_ALLOWED_WITH_RANK);
     winSql(
         "select dense_rank() over (order by empno ^rows^ 2 preceding ) from emp ")
-        .fails("ROW/RANGE not allowed with RANK or DENSE_RANK functions");
+        .fails(ROW_RANGE_NOT_ALLOWED_WITH_RANK);
     if (defined.contains("PERCENT_RANK")) {
       winSql("select percent_rank() over (order by empno) from emp").ok();
     }

http://git-wip-us.apache.org/repos/asf/incubator-calcite/blob/096d2820/core/src/test/resources/sql/winagg.oq
----------------------------------------------------------------------
diff --git a/core/src/test/resources/sql/winagg.oq b/core/src/test/resources/sql/winagg.oq
index d3c4b69..590af75 100644
--- a/core/src/test/resources/sql/winagg.oq
+++ b/core/src/test/resources/sql/winagg.oq
@@ -145,6 +145,7 @@ select *, rank() over (order by deptno) as c from emp;
 (9 rows)
 
 !ok
+
 # Calcite does not yet generate tied ranks
 select *, dense_rank() over (order by deptno) as c from emp;
  ENAME | DEPTNO | GENDER | C
@@ -161,6 +162,54 @@ select *, dense_rank() over (order by deptno) as c from emp;
 (9 rows)
 
 !ok
+
+# [CALCITE-806] ROW_NUMBER should emit distinct values
+#
+# We only run this test under JDK 1.8 because the results are
+# non-deterministic and are different (but still correct) on
+# JDK 1.7 and other platforms.
+!if (jdk18) {
+select *,
+  row_number() over (order by deptno) as r1,
+  row_number() over (partition by deptno order by gender desc) as r2,
+  row_number() over (partition by deptno order by gender) as r3,
+  row_number() over (partition by gender) as r4,
+  row_number() over () as r
+from emp;
+ ENAME | DEPTNO | GENDER | R1 | R2 | R3 | R4 | R
+-------+--------+--------+----+----+----+----+---
+ Wilma |        | F      |  9 |  1 |  1 |  1 | 1
+ Eve   |     50 | F      |  7 |  2 |  1 |  2 | 2
+ Jane  |     10 | F      |  1 |  2 |  1 |  3 | 3
+ Grace |     60 | F      |  8 |  1 |  1 |  4 | 4
+ Susan |     30 | F      |  4 |  1 |  1 |  5 | 5
+ Alice |     30 | F      |  5 |  2 |  2 |  6 | 6
+ Adam  |     50 | M      |  6 |  1 |  2 |  1 | 7
+ Eric  |     20 | M      |  3 |  1 |  1 |  2 | 8
+ Bob   |     10 | M      |  2 |  1 |  2 |  3 | 9
+(9 rows)
+
+!ok
+!}
+
+# As above, ROW_NUMBER without explicit ORDER BY
+select deptno,
+  ename,
+  row_number() over (partition by deptno) as r
+from emp
+where gender = 'F';
+ DEPTNO | ENAME | R
+--------+-------+---
+     10 | Jane  | 1
+     30 | Alice | 2
+     30 | Susan | 1
+     50 | Eve   | 1
+     60 | Grace | 1
+        | Wilma | 1
+(6 rows)
+
+!ok
+
 !if (false) {
 select *, count(*) over (order by deptno), first_value(ename) over (order by deptno rows 2 following) from emp;
  ERROR:  frame starting from following row cannot end with current row
@@ -184,6 +233,7 @@ select *, count(*) over (partition by deptno) as c from emp;
 (9 rows)
 
 !ok
+
 # No ORDER BY, windows defined in WINDOW clause.
 select deptno, gender, min(gender) over w1 as a, min(gender) over w2 as d
 from emp
@@ -224,13 +274,11 @@ window w1 as ();
 !ok
 
 # Window Aggregate and group-by.
-# (ORDER BY is necessary to work around [QUIDEM-7].)
 !set outputformat mysql
 select min(deptno) as x, rank() over (order by ename) as y,
   max(ename) over (partition by deptno) as z
 from emp
-group by deptno, ename
-order by ename;
+group by deptno, ename;
 
 +----+---+-------+
 | X  | Y | Z     |

http://git-wip-us.apache.org/repos/asf/incubator-calcite/blob/096d2820/pom.xml
----------------------------------------------------------------------
diff --git a/pom.xml b/pom.xml
index b44db5b..c48cc52 100644
--- a/pom.xml
+++ b/pom.xml
@@ -200,7 +200,7 @@ limitations under the License.
       <dependency>
         <groupId>net.hydromatic</groupId>
         <artifactId>quidem</artifactId>
-        <version>0.5</version>
+        <version>0.6</version>
       </dependency>
       <dependency>
         <groupId>net.hydromatic</groupId>