[CALCITE-800] Window function defined within another window function …

…should be invalid (Hsuan-Yi Chu) Close apache/calcite#106
ldming · Jul 17, 2015 · 798160d · 798160d
1 parent 79baaae
commit 798160d
Show file tree

Hide file tree

Showing 7 changed files with 84 additions and 8 deletions.
diff --git a/src/main/java/org/apache/calcite/runtime/CalciteResource.java b/src/main/java/org/apache/calcite/runtime/CalciteResource.java
@@ -332,6 +332,12 @@ ExInst<SqlValidatorException> intervalFieldExceedsPrecision(Number a0,
   @BaseMessage("Window specification must contain an ORDER BY clause")
   ExInst<SqlValidatorException> overMissingOrderBy();
 
+  @BaseMessage("PARTITION BY expression should not contain OVER clause")
+  ExInst<SqlValidatorException> partitionbyShouldNotContainOver();
+
+  @BaseMessage("ORDER BY expression should not contain OVER clause")
+  ExInst<SqlValidatorException> orderbyShouldNotContainOver();
+
   @BaseMessage("UNBOUNDED FOLLOWING cannot be specified for the lower frame boundary")
   ExInst<SqlValidatorException> badLowerBoundary();
 

diff --git a/src/main/java/org/apache/calcite/sql/SqlRankFunction.java b/src/main/java/org/apache/calcite/sql/SqlRankFunction.java
@@ -31,24 +31,26 @@
 public class SqlRankFunction extends SqlAggFunction {
   //~ Instance fields --------------------------------------------------------
 
+  private final boolean requiresOrder;
   private final RelDataType type = null;
 
   //~ Constructors -----------------------------------------------------------
 
-  public SqlRankFunction(String name) {
+  public SqlRankFunction(String name, boolean requiresOrder) {
     super(
         name,
         SqlKind.OTHER_FUNCTION,
         ReturnTypes.INTEGER,
         null,
         OperandTypes.NILADIC,
         SqlFunctionCategory.NUMERIC);
+    this.requiresOrder = requiresOrder;
   }
 
   //~ Methods ----------------------------------------------------------------
 
   @Override public boolean requiresOrder() {
-    return true;
+    return requiresOrder;
   }
 
   @Override public boolean allowsFraming() {

diff --git a/src/main/java/org/apache/calcite/sql/SqlWindow.java b/src/main/java/org/apache/calcite/sql/SqlWindow.java
@@ -27,6 +27,7 @@
 import org.apache.calcite.sql.validate.SqlValidator;
 import org.apache.calcite.sql.validate.SqlValidatorScope;
 import org.apache.calcite.sql.validate.SqlValidatorUtil;
+import org.apache.calcite.util.ControlFlowException;
 import org.apache.calcite.util.ImmutableNullableList;
 import org.apache.calcite.util.Util;
 
@@ -523,13 +524,27 @@ public boolean isAllowPartial() {
     }
 
     for (SqlNode partitionItem : partitionList) {
+      try {
+        partitionItem.accept(Util.OverFinder.INSTANCE);
+      } catch (ControlFlowException e) {
+        throw validator.newValidationError(this,
+            RESOURCE.partitionbyShouldNotContainOver());
+      }
+
       partitionItem.validateExpr(validator, operandScope);
     }
 
     for (SqlNode orderItem : orderList) {
       boolean savedColumnReferenceExpansion =
           validator.getColumnReferenceExpansion();
       validator.setColumnReferenceExpansion(false);
+      try {
+        orderItem.accept(Util.OverFinder.INSTANCE);
+      } catch (ControlFlowException e) {
+        throw validator.newValidationError(this,
+            RESOURCE.orderbyShouldNotContainOver());
+      }
+
       try {
         orderItem.validateExpr(validator, scope);
       } finally {

diff --git a/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java b/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java
@@ -850,30 +850,30 @@ public boolean argumentMustBeScalar(int ordinal) {
    * <code>CUME_DIST</code> Window function.
    */
   public static final SqlRankFunction CUME_DIST =
-      new SqlRankFunction("CUME_DIST");
+      new SqlRankFunction("CUME_DIST", true);
 
   /**
    * <code>DENSE_RANK</code> Window function.
    */
   public static final SqlRankFunction DENSE_RANK =
-      new SqlRankFunction("DENSE_RANK");
+      new SqlRankFunction("DENSE_RANK", true);
 
   /**
    * <code>PERCENT_RANK</code> Window function.
    */
   public static final SqlRankFunction PERCENT_RANK =
-      new SqlRankFunction("PERCENT_RANK");
+      new SqlRankFunction("PERCENT_RANK", true);
 
   /**
    * <code>RANK</code> Window function.
    */
-  public static final SqlRankFunction RANK = new SqlRankFunction("RANK");
+  public static final SqlRankFunction RANK = new SqlRankFunction("RANK", true);
 
   /**
    * <code>ROW_NUMBER</code> Window function.
    */
   public static final SqlRankFunction ROW_NUMBER =
-      new SqlRankFunction("ROW_NUMBER");
+      new SqlRankFunction("ROW_NUMBER", false);
 
   //-------------------------------------------------------------
   //                   SPECIAL OPERATORS

diff --git a/src/main/java/org/apache/calcite/util/Util.java b/src/main/java/org/apache/calcite/util/Util.java
@@ -21,10 +21,12 @@
 import org.apache.calcite.runtime.CalciteException;
 import org.apache.calcite.sql.SqlAggFunction;
 import org.apache.calcite.sql.SqlCall;
+import org.apache.calcite.sql.SqlKind;
 import org.apache.calcite.sql.SqlLiteral;
 import org.apache.calcite.sql.SqlNode;
 import org.apache.calcite.sql.SqlValuesOperator;
 import org.apache.calcite.sql.fun.SqlRowOperator;
+import org.apache.calcite.sql.util.SqlBasicVisitor;
 
 import com.google.common.base.Function;
 import com.google.common.base.Objects;
@@ -2175,6 +2177,21 @@ public Object getNode() {
       return node;
     }
   }
+
+  /**
+   * Visitor which looks for an OVER clause inside a tree of
+   * {@link SqlNode} objects.
+   */
+  public static class OverFinder extends SqlBasicVisitor<Void> {
+    public static final OverFinder INSTANCE = new Util.OverFinder();
+
+    @Override public Void visit(SqlCall call) {
+      if (call.getKind() == SqlKind.OVER) {
+        throw FoundOne.NULL;
+      }
+      return super.visit(call);
+    }
+  }
 }
 
 // End Util.java
diff --git a/src/main/resources/org/apache/calcite/runtime/CalciteResource.properties b/src/main/resources/org/apache/calcite/runtime/CalciteResource.properties
@@ -113,6 +113,8 @@ OrderByRangeMismatch=Data Type mismatch between ORDER BY and RANGE clause
 DateRequiresInterval=Window ORDER BY expression of type DATE requires range of type INTERVAL
 RowMustBeNonNegativeIntegral=ROWS value must be a non-negative integral constant
 OverMissingOrderBy=Window specification must contain an ORDER BY clause
+PartitionbyShouldNotContainOver=PARTITION BY expression should not contain OVER clause
+OrderbyShouldNotContainOver=ORDER BY expression should not contain OVER clause
 BadLowerBoundary=UNBOUNDED FOLLOWING cannot be specified for the lower frame boundary
 BadUpperBoundary=UNBOUNDED PRECEDING cannot be specified for the upper frame boundary
 CurrentRowPrecedingError=Upper frame boundary cannot be PRECEDING when lower boundary is CURRENT ROW

diff --git a/src/test/java/org/apache/calcite/test/SqlValidatorTest.java b/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
@@ -3801,6 +3801,32 @@ public void _testWinPartClause() {
     // Test specified collation, window clause syntax rule 4,5.
   }
 
+  @Test public void testOverInPartitionBy() {
+    winSql(
+        "select sum(deptno) over ^(partition by sum(deptno) \n"
+        + "over(order by deptno))^ from emp")
+        .fails("PARTITION BY expression should not contain OVER clause");
+
+    winSql(
+        "select sum(deptno) over w \n"
+        + "from emp \n"
+        + "window w as ^(partition by sum(deptno) over(order by deptno))^")
+        .fails("PARTITION BY expression should not contain OVER clause");
+  }
+
+  @Test public void testOverInOrderBy() {
+    winSql(
+        "select sum(deptno) over ^(order by sum(deptno) \n"
+        + "over(order by deptno))^ from emp")
+        .fails("ORDER BY expression should not contain OVER clause");
+
+    winSql(
+        "select sum(deptno) over w \n"
+        + "from emp \n"
+        + "window w as ^(order by sum(deptno) over(order by deptno))^")
+        .fails("ORDER BY expression should not contain OVER clause");
+  }
+
   @Test public void testWindowFunctions() {
     // SQL 03 Section 6.10
 
@@ -3859,8 +3885,15 @@ public void _testWinPartClause() {
             "RANK or DENSE_RANK functions require ORDER BY clause in window specification");
     winSql("select rank() over w2 from emp\n"
         + "window w as (partition by sal), w2 as (w order by deptno)").ok();
+
     // row_number function
     winExp("row_number() over (order by deptno)").ok();
+    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");
+    winExp("row_number() over (order by deptno ^range^ 2 preceding)")
+        .fails("ROW/RANGE not allowed with RANK or DENSE_RANK functions");
 
     // rank function type
     if (defined.contains("DENSE_RANK")) {
@@ -4187,7 +4220,8 @@ public void _testWinPartClause() {
         "Expression 'COMM' is not being grouped");
 
     // syntax rule 7
-    win("window w as (order by rank() over (order by sal))").ok();
+    win("window w as ^(order by rank() over (order by sal))^")
+        .fails("ORDER BY expression should not contain OVER clause");
 
     // ------------------------------------
     // ---- window frame between tests ----