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;");
+ }
+ }
}
|