drill-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From par...@apache.org
Subject [1/2] drill git commit: DRILL-3944: Drill MAXDIR Unknown variable or type FILE_SEPARATOR. This closes #391
Date Fri, 26 Feb 2016 21:30:49 GMT
Repository: drill
Updated Branches:
  refs/heads/master 3e55fe1db -> b9960f89e


DRILL-3944: Drill MAXDIR Unknown variable or type FILE_SEPARATOR. This
closes #391


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

Branch: refs/heads/master
Commit: cca17cb8f9891bb0b49560acad7f129a6cb7edc0
Parents: 3e55fe1
Author: Arina Ielchiieva <arina.yelchiyeva@gmail.com>
Authored: Tue Feb 2 18:48:29 2016 +0200
Committer: Parth Chandra <parthc@apache.org>
Committed: Fri Feb 26 09:52:38 2016 -0800

----------------------------------------------------------------------
 .../codegen/templates/DirectoryExplorers.java   |  3 +-
 .../sql/parser/UnsupportedOperatorsVisitor.java | 73 +++++++++++++++-
 .../exec/planner/TestDirectoryExplorerUDFs.java | 89 ++++++++++++++++----
 3 files changed, 147 insertions(+), 18 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/drill/blob/cca17cb8/exec/java-exec/src/main/codegen/templates/DirectoryExplorers.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/codegen/templates/DirectoryExplorers.java b/exec/java-exec/src/main/codegen/templates/DirectoryExplorers.java
index c97e34b..655ff81 100644
--- a/exec/java-exec/src/main/codegen/templates/DirectoryExplorers.java
+++ b/exec/java-exec/src/main/codegen/templates/DirectoryExplorers.java
@@ -38,7 +38,6 @@ import javax.inject.Inject;
  */
 public class DirectoryExplorers {
   static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(DirectoryExplorers.class);
-  private static final String FILE_SEPARATOR = "/";
 
   <#list [ { "name" : "\"maxdir\"", "functionClassName" : "MaxDir", "comparison" : "compareTo(curr)
< 0", "goal" : "maximum", "comparisonType" : "case-sensitive"},
            { "name" : "\"imaxdir\"", "functionClassName" : "IMaxDir", "comparison" : "compareToIgnoreCase(curr)
< 0", "goal" : "maximum", "comparisonType" : "case-insensitive"},
@@ -94,7 +93,7 @@ public class DirectoryExplorers {
           subPartitionStr = curr;
         }
       }
-      String[] subPartitionParts = subPartitionStr.split(FILE_SEPARATOR);
+      String[] subPartitionParts = subPartitionStr.split("/");
       subPartitionStr = subPartitionParts[subPartitionParts.length - 1];
       byte[] result = subPartitionStr.getBytes();
       out.buffer = buffer = buffer.reallocIfNeeded(result.length);

http://git-wip-us.apache.org/repos/asf/drill/blob/cca17cb8/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 17bc339..e90aa3b 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
@@ -24,6 +24,7 @@ import org.apache.calcite.sql.util.SqlBasicVisitor;
 import org.apache.drill.exec.ExecConstants;
 import org.apache.drill.exec.exception.UnsupportedOperatorCollector;
 import org.apache.drill.exec.ops.QueryContext;
+import org.apache.drill.exec.planner.physical.PlannerSettings;
 import org.apache.drill.exec.work.foreman.SqlUnsupportedException;
 
 import org.apache.calcite.sql.SqlSelectKeyword;
@@ -48,12 +49,17 @@ public class UnsupportedOperatorsVisitor extends SqlShuttle {
   private QueryContext context;
   private static List<String> disabledType = Lists.newArrayList();
   private static List<String> disabledOperators = Lists.newArrayList();
+  private static List<String> dirExplorers = Lists.newArrayList();
 
   static {
     disabledType.add(SqlTypeName.TINYINT.name());
     disabledType.add(SqlTypeName.SMALLINT.name());
     disabledType.add(SqlTypeName.REAL.name());
     disabledOperators.add("CARDINALITY");
+    dirExplorers.add("MAXDIR");
+    dirExplorers.add("IMAXDIR");
+    dirExplorers.add("MINDIR");
+    dirExplorers.add("IMINDIR");
   }
 
   private UnsupportedOperatorCollector unsupportedOperatorCollector;
@@ -269,9 +275,29 @@ public class UnsupportedOperatorsVisitor extends SqlShuttle {
       }
     }
 
-    // Disable complex functions being present in any place other than Select-Clause
+    // Disable complex functions incorrect placement
     if(sqlCall instanceof SqlSelect) {
       SqlSelect sqlSelect = (SqlSelect) sqlCall;
+
+      for (SqlNode nodeInSelectList : sqlSelect.getSelectList()) {
+        if (checkDirExplorers(nodeInSelectList)) {
+          unsupportedOperatorCollector.setException(SqlUnsupportedException.ExceptionType.FUNCTION,
+              "Directory explorers " + dirExplorers + " functions are not supported in Select
List\n" +
+                  "See Apache Drill JIRA: DRILL-3944");
+          throw new UnsupportedOperationException();
+        }
+      }
+
+      if (sqlSelect.hasWhere()) {
+        if (checkDirExplorers(sqlSelect.getWhere()) && !context.getPlannerSettings().isConstantFoldingEnabled())
{
+          unsupportedOperatorCollector.setException(SqlUnsupportedException.ExceptionType.FUNCTION,
+              "Directory explorers " + dirExplorers + " functions can not be used " +
+                  "when " + PlannerSettings.CONSTANT_FOLDING.getOptionName() + " option is
set to false\n" +
+                  "See Apache Drill JIRA: DRILL-3944");
+          throw new UnsupportedOperationException();
+        }
+      }
+
       if(sqlSelect.hasOrderBy()) {
         for (SqlNode sqlNode : sqlSelect.getOrderList()) {
           if(containsFlatten(sqlNode)) {
@@ -279,6 +305,11 @@ public class UnsupportedOperatorsVisitor extends SqlShuttle {
                 "Flatten function is not supported in Order By\n" +
                 "See Apache Drill JIRA: DRILL-2181");
             throw new UnsupportedOperationException();
+          } else if (checkDirExplorers(sqlNode)) {
+            unsupportedOperatorCollector.setException(SqlUnsupportedException.ExceptionType.FUNCTION,
+                "Directory explorers " + dirExplorers + " functions are not supported in
Order By\n" +
+                "See Apache Drill JIRA: DRILL-3944");
+            throw new UnsupportedOperationException();
           }
         }
       }
@@ -290,6 +321,11 @@ public class UnsupportedOperatorsVisitor extends SqlShuttle {
                 "Flatten function is not supported in Group By\n" +
                 "See Apache Drill JIRA: DRILL-2181");
             throw new UnsupportedOperationException();
+          } else if (checkDirExplorers(sqlNode)) {
+                unsupportedOperatorCollector.setException(SqlUnsupportedException.ExceptionType.FUNCTION,
+                "Directory explorers " + dirExplorers + " functions are not supported in
Group By\n" +
+                "See Apache Drill JIRA: DRILL-3944");
+            throw new UnsupportedOperationException();
           }
         }
       }
@@ -351,6 +387,12 @@ public class UnsupportedOperatorsVisitor extends SqlShuttle {
     }
   }
 
+  private boolean checkDirExplorers(SqlNode sqlNode) {
+    final ExprFinder dirExplorersFinder = new ExprFinder(DirExplorersCondition);
+    sqlNode.accept(dirExplorersFinder);
+    return dirExplorersFinder.find();
+  }
+
   /**
    * A function that replies true or false for a given expression.
    *
@@ -403,6 +445,35 @@ public class UnsupportedOperatorsVisitor extends SqlShuttle {
   };
 
   /**
+   * A condition that returns true if SqlNode has Directory Explorers.
+   */
+  private final SqlNodeCondition DirExplorersCondition = new SqlNodeCondition() {
+    @Override
+    public boolean test(SqlNode sqlNode) {
+      return sqlNode instanceof SqlCall && checkOperator((SqlCall) sqlNode, dirExplorers,
true);
+    }
+
+    /**
+     * Checks recursively if operator and its operands are present in provided list of operators
+     */
+    private boolean checkOperator(SqlCall sqlCall, List<String> operators, boolean
checkOperator) {
+      if (checkOperator) {
+        return operators.contains(sqlCall.getOperator().getName().toUpperCase()) || checkOperator(sqlCall,
operators, false);
+      }
+      for (SqlNode sqlNode : sqlCall.getOperandList()) {
+        if (!(sqlNode instanceof SqlCall)) {
+          continue;
+        }
+        if (checkOperator((SqlCall) sqlNode, operators, true)) {
+          return true;
+        }
+      }
+      return false;
+    }
+
+  };
+
+  /**
    * A visitor to check if the given SqlNodeCondition is tested as true or not.
    * If the condition is true, mark flag 'find' as true.
    */

http://git-wip-us.apache.org/repos/asf/drill/blob/cca17cb8/exec/java-exec/src/test/java/org/apache/drill/exec/planner/TestDirectoryExplorerUDFs.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/planner/TestDirectoryExplorerUDFs.java
b/exec/java-exec/src/test/java/org/apache/drill/exec/planner/TestDirectoryExplorerUDFs.java
index 516f975..b830f48 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/exec/planner/TestDirectoryExplorerUDFs.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/exec/planner/TestDirectoryExplorerUDFs.java
@@ -18,11 +18,16 @@
 package org.apache.drill.exec.planner;
 
 import java.util.List;
+import java.util.Map;
 
+import com.google.common.collect.ImmutableMap;
 import org.apache.drill.PlanTestBase;
+import org.apache.drill.common.exceptions.UserRemoteException;
 import org.apache.drill.exec.fn.interp.TestConstantFolding;
 import org.apache.drill.exec.util.JsonStringArrayList;
 import org.apache.drill.exec.util.Text;
+import org.junit.Before;
+import org.junit.BeforeClass;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
@@ -30,12 +35,12 @@ import org.junit.rules.TemporaryFolder;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Lists;
 
-public class TestDirectoryExplorerUDFs extends PlanTestBase {
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.junit.Assert.assertThat;
 
-  @Rule
-  public TemporaryFolder folder = new TemporaryFolder();
+public class TestDirectoryExplorerUDFs extends PlanTestBase {
 
-  private class ConstantFoldingTestConfig {
+  private static class ConstantFoldingTestConfig {
     String funcName;
     String expectedFolderName;
     public ConstantFoldingTestConfig(String funcName, String expectedFolderName) {
@@ -44,14 +49,14 @@ public class TestDirectoryExplorerUDFs extends PlanTestBase {
     }
   }
 
-  @Test
-  public void testConstExprFolding_maxDir0() throws Exception {
-
-    new TestConstantFolding.SmallFileCreator(folder).createFiles(1, 1000);
-    String path = folder.getRoot().toPath().toString();
+  private static List<ConstantFoldingTestConfig> tests;
+  private String path;
 
-    test("use dfs.root");
+  @Rule
+  public TemporaryFolder folder = new TemporaryFolder();
 
+  @BeforeClass
+  public static void init() {
     // Need the suffixes to make the names unique in the directory.
     // The capitalized name is on the opposite function (imaxdir and mindir)
     // because they are looking on opposite ends of the list.
@@ -60,12 +65,25 @@ public class TestDirectoryExplorerUDFs extends PlanTestBase {
     // first in the case-sensitive ordering.
     // SMALLFILE_2 comes last in a case-insensitive ordering because it has
     // a suffix not found on smallfile.
-    List<ConstantFoldingTestConfig> tests = ImmutableList.<ConstantFoldingTestConfig>builder()
-        .add(new ConstantFoldingTestConfig("maxdir", "smallfile"))
-        .add(new ConstantFoldingTestConfig("imaxdir", "SMALLFILE_2"))
-        .add(new ConstantFoldingTestConfig("mindir", "BIGFILE_2"))
-        .add(new ConstantFoldingTestConfig("imindir", "bigfile"))
+    tests = ImmutableList.<ConstantFoldingTestConfig>builder()
+        .add(new ConstantFoldingTestConfig("MAXDIR", "smallfile"))
+        .add(new ConstantFoldingTestConfig("IMAXDIR", "SMALLFILE_2"))
+        .add(new ConstantFoldingTestConfig("MINDIR", "BIGFILE_2"))
+        .add(new ConstantFoldingTestConfig("IMINDIR", "bigfile"))
         .build();
+  }
+
+  @Before
+  public void setup() throws Exception {
+    new TestConstantFolding.SmallFileCreator(folder).createFiles(1, 1000);
+    path = folder.getRoot().toPath().toString();
+  }
+
+
+  @Test
+  public void testConstExprFolding_maxDir0() throws Exception {
+
+    test("use dfs.root");
 
     List<String> allFiles = ImmutableList.<String>builder()
         .add("smallfile")
@@ -104,4 +122,45 @@ public class TestDirectoryExplorerUDFs extends PlanTestBase {
         .go();
   }
 
+  @Test
+  public void testIncorrectFunctionPlacement() throws Exception {
+
+    Map<String, String> configMap = ImmutableMap.<String, String>builder()
+        .put("select %s('dfs.root','" + path + "') from dfs.`" + path + "/*/*.csv`", "Select
List")
+        .put("select dir0 from dfs.`" + path + "/*/*.csv` order by %s('dfs.root','" + path
+ "')", "Order By")
+        .put("select max(dir0) from dfs.`" + path + "/*/*.csv` group by %s('dfs.root','"
+ path + "')", "Group By")
+        .put("select concat(concat(%s('dfs.root','" + path + "'),'someName'),'someName')
from dfs.`" + path + "/*/*.csv`", "Select List")
+        .put("select dir0 from dfs.`" + path + "/*/*.csv` order by concat(%s('dfs.root','"
+ path + "'),'someName')", "Order By")
+        .put("select max(dir0) from dfs.`" + path + "/*/*.csv` group by concat(%s('dfs.root','"
+ path + "'),'someName')", "Group By")
+        .build();
+
+    for (Map.Entry<String, String> configEntry : configMap.entrySet()) {
+      for (ConstantFoldingTestConfig functionConfig : tests) {
+        try {
+          test(String.format(configEntry.getKey(), functionConfig.funcName));
+        } catch (UserRemoteException e) {
+          assertThat(e.getMessage(), containsString(
+              String.format("Directory explorers [MAXDIR, IMAXDIR, MINDIR, IMINDIR] functions
are not supported in %s", configEntry.getValue())));
+        }
+      }
+    }
+  }
+
+  @Test
+  public void testConstantFoldingOff() throws Exception {
+    try {
+      test("set `planner.enable_constant_folding` = false;");
+      String query = "select * from dfs.`" + path + "/*/*.csv` where dir0 = %s('dfs.root','"
+ path + "')";
+      for (ConstantFoldingTestConfig config : tests) {
+        try {
+          test(String.format(query, config.funcName));
+        } catch (UserRemoteException e) {
+          assertThat(e.getMessage(), containsString("Directory explorers [MAXDIR, IMAXDIR,
MINDIR, IMINDIR] functions can not be used " +
+              "when planner.enable_constant_folding option is set to false"));
+        }
+      }
+    } finally {
+      test("set `planner.enable_constant_folding` = true;");
+    }
+  }
 }


Mime
View raw message