drill-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From amansi...@apache.org
Subject [2/2] drill git commit: DRILL-2559: In case window functions are disabled, throw UnsupportedFunctionException
Date Wed, 01 Apr 2015 17:46:19 GMT
DRILL-2559: In case window functions are disabled, throw UnsupportedFunctionException


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

Branch: refs/heads/master
Commit: d05ff9d841b9a8f47ed522dfab5dd59fb1fdbfd1
Parents: 451bc5a
Author: Hsuan-Yi Chu <hsuanyi@usc.edu>
Authored: Wed Mar 25 09:45:47 2015 -0700
Committer: Aman Sinha <asinha@maprtech.com>
Committed: Wed Apr 1 10:33:06 2015 -0700

----------------------------------------------------------------------
 .../exec/physical/impl/window/OverFinder.java   | 53 --------------------
 .../planner/sql/handlers/DefaultSqlHandler.java | 11 +---
 .../sql/parser/UnsupportedOperatorsVisitor.java | 23 ++++++---
 .../apache/drill/TestDisabledFunctionality.java | 13 +++++
 4 files changed, 30 insertions(+), 70 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/drill/blob/d05ff9d8/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/OverFinder.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/OverFinder.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/OverFinder.java
deleted file mode 100644
index 40e5920..0000000
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/OverFinder.java
+++ /dev/null
@@ -1,53 +0,0 @@
-/**
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.apache.drill.exec.physical.impl.window;
-
-import org.eigenbase.sql.SqlCall;
-import org.eigenbase.sql.SqlKind;
-import org.eigenbase.sql.SqlNode;
-import org.eigenbase.sql.SqlOperator;
-import org.eigenbase.sql.util.SqlBasicVisitor;
-import org.eigenbase.util.Util;
-
-/**
- * Visitor which looks for an over clause inside a tree of {@link
- * SqlNode} objects.
- */
-public class OverFinder extends SqlBasicVisitor<Void> {
-
-  public boolean findOver(SqlNode node) {
-    try {
-      node.accept(this);
-      return false;
-    } catch (Util.FoundOne e) {
-      Util.swallow(e, null);
-      return true;
-    }
-  }
-
-  @Override
-  public Void visit(SqlCall call) {
-    final SqlOperator operator = call.getOperator();
-
-    if (operator.getKind().equals(SqlKind.OVER)) {
-      throw new Util.FoundOne(call);
-    }
-
-    return super.visit(call);
-  }
-}
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/drill/blob/d05ff9d8/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DefaultSqlHandler.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DefaultSqlHandler.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DefaultSqlHandler.java
index 37003f1..5ee502d 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DefaultSqlHandler.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DefaultSqlHandler.java
@@ -37,7 +37,6 @@ import org.apache.drill.exec.physical.PhysicalPlan;
 import org.apache.drill.exec.physical.base.AbstractPhysicalVisitor;
 import org.apache.drill.exec.physical.base.PhysicalOperator;
 import org.apache.drill.exec.physical.impl.join.JoinUtils;
-import org.apache.drill.exec.physical.impl.window.OverFinder;
 import org.apache.drill.exec.planner.logical.DrillRel;
 import org.apache.drill.exec.planner.logical.DrillScreenRel;
 import org.apache.drill.exec.planner.logical.DrillStoreRel;
@@ -146,18 +145,10 @@ public class DefaultSqlHandler extends AbstractSqlHandler {
   }
 
   protected SqlNode validateNode(SqlNode sqlNode) throws ValidationException, RelConversionException,
ForemanSetupException {
-    final boolean enableWindow = context.getOptions().getOption(ExecConstants.ENABLE_WINDOW_FUNCTIONS).bool_val;
-    if (!enableWindow) {
-      final OverFinder overFinder = new OverFinder();
-      if (overFinder.findOver(sqlNode)) {
-        throw new ValidationException("Window Functions have been disabled");
-      }
-    }
-
     SqlNode sqlNodeValidated = planner.validate(sqlNode);
 
     // Check if the unsupported functionality is used
-    UnsupportedOperatorsVisitor visitor = UnsupportedOperatorsVisitor.createVisitor();
+    UnsupportedOperatorsVisitor visitor = UnsupportedOperatorsVisitor.createVisitor(context);
     try {
       sqlNodeValidated.accept(visitor);
     } catch (UnsupportedOperationException ex) {

http://git-wip-us.apache.org/repos/asf/drill/blob/d05ff9d8/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 81734dc..4830fe1 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
@@ -19,15 +19,13 @@ package org.apache.drill.exec.planner.sql.parser;
 
 import org.apache.drill.exec.ExecConstants;
 import org.apache.drill.exec.exception.UnsupportedOperatorCollector;
-import org.apache.drill.exec.planner.StarColumnHelper;
+import org.apache.drill.exec.ops.QueryContext;
 import org.apache.drill.exec.work.foreman.SqlUnsupportedException;
 import org.eigenbase.sql.SqlCall;
 import org.eigenbase.sql.SqlKind;
 import org.eigenbase.sql.SqlJoin;
 import org.eigenbase.sql.JoinType;
 import org.eigenbase.sql.SqlNode;
-import org.eigenbase.sql.SqlNodeList;
-import org.eigenbase.sql.SqlSelect;
 import org.eigenbase.sql.type.SqlTypeName;
 import org.eigenbase.sql.util.SqlShuttle;
 import org.eigenbase.sql.SqlDataTypeSpec;
@@ -36,6 +34,7 @@ import java.util.List;
 import com.google.common.collect.Lists;
 
 public class UnsupportedOperatorsVisitor extends SqlShuttle {
+  private QueryContext context;
   private static List<String> disabledType = Lists.newArrayList();
   private static List<String> disabledOperators = Lists.newArrayList();
 
@@ -48,12 +47,13 @@ public class UnsupportedOperatorsVisitor extends SqlShuttle {
 
   private UnsupportedOperatorCollector unsupportedOperatorCollector;
 
-  private UnsupportedOperatorsVisitor() {
-    unsupportedOperatorCollector = new UnsupportedOperatorCollector();
+  private UnsupportedOperatorsVisitor(QueryContext context) {
+    this.context = context;
+    this.unsupportedOperatorCollector = new UnsupportedOperatorCollector();
   }
 
-  public static UnsupportedOperatorsVisitor createVisitor() {
-    return new UnsupportedOperatorsVisitor();
+  public static UnsupportedOperatorsVisitor createVisitor(QueryContext context) {
+    return new UnsupportedOperatorsVisitor(context);
   }
 
   public void convertException() throws SqlUnsupportedException {
@@ -116,6 +116,15 @@ public class UnsupportedOperatorsVisitor extends SqlShuttle {
       }
     }
 
+    // Throw exceptions if window functions are disabled
+    if(sqlCall.getOperator().getKind().equals(SqlKind.OVER)
+        && !context.getOptions().getOption(ExecConstants.ENABLE_WINDOW_FUNCTIONS).bool_val)
{
+      unsupportedOperatorCollector.setException(SqlUnsupportedException.ExceptionType.FUNCTION,
+          "Window functions are disabled\n" +
+          "See Apache Drill JIRA: DRILL-2559");
+      throw new UnsupportedOperationException();
+    }
+
     // Disable Function
     for(String strOperator : disabledOperators) {
       if(sqlCall.getOperator().isName(strOperator)) {

http://git-wip-us.apache.org/repos/asf/drill/blob/d05ff9d8/exec/java-exec/src/test/java/org/apache/drill/TestDisabledFunctionality.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/java/org/apache/drill/TestDisabledFunctionality.java
b/exec/java-exec/src/test/java/org/apache/drill/TestDisabledFunctionality.java
index f64d3e5..f62f060 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/TestDisabledFunctionality.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/TestDisabledFunctionality.java
@@ -312,4 +312,17 @@ public class TestDisabledFunctionality extends BaseTestQuery{
       throw ex;
     }
   }
+
+  @Test(expected = UnsupportedFunctionException.class) // see DRILL-2441
+  public void testDisabledWindowFunctions() throws Exception {
+    try {
+      test("SELECT employee_id,position_id, salary, avg(salary) " +
+          "OVER (PARTITION BY position_id order by position_id) " +
+          "FROM cp.`employee.json` " +
+          "order by employee_id;");
+    } catch(Exception ex) {
+      SqlUnsupportedException.errorMessageToException(ex.getMessage());
+      throw ex;
+    }
+  }
 }
\ No newline at end of file


Mime
View raw message