You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@drill.apache.org by ad...@apache.org on 2015/09/27 21:18:49 UTC

[3/3] drill git commit: DRILL-3596: Allow only () or (, 1) for LEAD and LAG window functions as input parameters

DRILL-3596: Allow only (<expression>) or (<expression>, 1) for LEAD and LAG window functions as input parameters

this closes #128


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

Branch: refs/heads/master
Commit: b9afcf8fa92e432d31bddf0c8c88b09a51b4102d
Parents: f7ce86d
Author: adeneche <ad...@gmail.com>
Authored: Mon Aug 24 13:37:25 2015 -0700
Committer: adeneche <ad...@gmail.com>
Committed: Sun Sep 27 12:17:55 2015 -0700

----------------------------------------------------------------------
 .../sql/parser/UnsupportedOperatorsVisitor.java | 35 ++++++++++++++++++++
 .../physical/impl/window/TestWindowFrame.java   | 23 +++++++++++++
 2 files changed, 58 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/drill/blob/b9afcf8f/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/UnsupportedOperatorsVisitor.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/UnsupportedOperatorsVisitor.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/UnsupportedOperatorsVisitor.java
index 67793bf..188095c 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/UnsupportedOperatorsVisitor.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/UnsupportedOperatorsVisitor.java
@@ -17,6 +17,7 @@
  */
 package org.apache.drill.exec.planner.sql.parser;
 
+import org.apache.calcite.sql.SqlNumericLiteral;
 import org.apache.drill.exec.ExecConstants;
 import org.apache.drill.exec.exception.UnsupportedOperatorCollector;
 import org.apache.drill.exec.ops.QueryContext;
@@ -118,6 +119,40 @@ public class UnsupportedOperatorsVisitor extends SqlShuttle {
                   "See Apache Drill JIRA: DRILL-3182");
               throw new UnsupportedOperationException();
             }
+
+            // DRILL-3596: we only allow (<column-name>) or (<column-name>, 1)
+            final String functionName = function.getOperator().getName().toUpperCase();
+            if ("LEAD".equals(functionName) || "LAG".equals(functionName)) {
+              boolean supported = true;
+              if (function.operandCount() > 2) {
+                // we don't support more than 2 arguments
+                supported = false;
+              } else if (function.operandCount() == 2) {
+                SqlNode operand = function.operand(1);
+                if (operand instanceof SqlNumericLiteral) {
+                  SqlNumericLiteral offsetLiteral = (SqlNumericLiteral) operand;
+                  try {
+                    if (offsetLiteral.intValue(true) != 1) {
+                      // we don't support offset != 1
+                      supported = false;
+                    }
+                  } catch (AssertionError e) {
+                    // we only support offset as an integer
+                    supported = false;
+                  }
+                } else {
+                  // we only support offset as a numeric literal
+                  supported = false;
+                }
+              }
+
+              if (!supported) {
+                unsupportedOperatorCollector.setException(SqlUnsupportedException.ExceptionType.FUNCTION,
+                  "Function " + functionName + " only supports (<value expression>) or (<value expression>, 1)\n" +
+                    "See Apache DRILL JIRA: DRILL-3596");
+                throw new UnsupportedOperationException();
+              }
+            }
           }
         }
       }

http://git-wip-us.apache.org/repos/asf/drill/blob/b9afcf8f/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/window/TestWindowFrame.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/window/TestWindowFrame.java b/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/window/TestWindowFrame.java
index ba66fce..92c27d7 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/window/TestWindowFrame.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/window/TestWindowFrame.java
@@ -306,6 +306,29 @@ public class TestWindowFrame extends BaseTestQuery {
   }
 
   @Test
+  public void testLeadParams() throws Exception {
+    // make sure we only support default arguments for LEAD/LAG functions
+    final String query = "SELECT %s OVER(PARTITION BY col7 ORDER BY col8) FROM dfs_test.`%s/window/3648.parquet`";
+
+    test(query, "LEAD(col8, 1)", TEST_RES_PATH);
+    test(query, "LAG(col8, 1)", TEST_RES_PATH);
+
+    try {
+      test(query, "LEAD(col8, 2)", TEST_RES_PATH);
+      fail("query should fail");
+    } catch (UserRemoteException e) {
+      assertEquals(ErrorType.UNSUPPORTED_OPERATION, e.getErrorType());
+    }
+
+    try {
+      test(query, "LAG(col8, 2)", TEST_RES_PATH);
+      fail("query should fail");
+    } catch (UserRemoteException e) {
+      assertEquals(ErrorType.UNSUPPORTED_OPERATION, e.getErrorType());
+    }
+  }
+
+  @Test
   public void testPartitionNtile() {
     Partition partition = new Partition(12);