You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@drill.apache.org by am...@apache.org on 2018/01/18 23:17:32 UTC
[08/18] drill git commit: DRILL-3993: Fix failed tests after Calcite
update
DRILL-3993: Fix failed tests after Calcite update
- fix temporary table errors according to updated logic;
- fixed errors when we trying to make select from hbase table with schema name in query (example: "SELECT row_key FROM hbase.TestTableNullStr) from hbase schema (did "USE hbase" before). Added test for it;
- added fix for views which were created on Calcite 1.4 and test for it.
Project: http://git-wip-us.apache.org/repos/asf/drill/repo
Commit: http://git-wip-us.apache.org/repos/asf/drill/commit/9274cb92
Tree: http://git-wip-us.apache.org/repos/asf/drill/tree/9274cb92
Diff: http://git-wip-us.apache.org/repos/asf/drill/diff/9274cb92
Branch: refs/heads/master
Commit: 9274cb9204c6ebf5bd2d13fe4d02af5cebb48fa5
Parents: 0a525aa
Author: Roman Kulyk <ro...@gmail.com>
Authored: Thu Nov 2 18:22:36 2017 +0000
Committer: Volodymyr Vysotskyi <vv...@gmail.com>
Committed: Tue Jan 16 12:10:13 2018 +0200
----------------------------------------------------------------------
.../apache/drill/hbase/TestHBaseQueries.java | 7 ++
.../org/apache/drill/exec/dotdrill/View.java | 7 +-
.../drill/exec/planner/sql/SqlConverter.java | 99 ++++++++++++++------
.../apache/drill/exec/rpc/user/UserSession.java | 9 ++
.../org/apache/drill/exec/sql/TestCTTAS.java | 15 +++
.../apache/drill/exec/sql/TestViewSupport.java | 10 ++
.../view/view_from_calcite_1_4.view.drill | 10 ++
7 files changed, 127 insertions(+), 30 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/drill/blob/9274cb92/contrib/storage-hbase/src/test/java/org/apache/drill/hbase/TestHBaseQueries.java
----------------------------------------------------------------------
diff --git a/contrib/storage-hbase/src/test/java/org/apache/drill/hbase/TestHBaseQueries.java b/contrib/storage-hbase/src/test/java/org/apache/drill/hbase/TestHBaseQueries.java
index c3ee7d9..ee839c5 100644
--- a/contrib/storage-hbase/src/test/java/org/apache/drill/hbase/TestHBaseQueries.java
+++ b/contrib/storage-hbase/src/test/java/org/apache/drill/hbase/TestHBaseQueries.java
@@ -102,4 +102,11 @@ public class TestHBaseQueries extends BaseHBaseTest {
}
}
+ @Test
+ public void testSelectFromSchema() throws Exception {
+ setColumnWidths(new int[] {8, 15});
+ test("USE hbase");
+ runHBaseSQLVerifyCount("SELECT row_key\n"
+ + " FROM hbase.TestTableNullStr t WHERE row_key='a1'", 1);
+ }
}
http://git-wip-us.apache.org/repos/asf/drill/blob/9274cb92/exec/java-exec/src/main/java/org/apache/drill/exec/dotdrill/View.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/dotdrill/View.java b/exec/java-exec/src/main/java/org/apache/drill/exec/dotdrill/View.java
index 2b69f00..3524d73 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/dotdrill/View.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/dotdrill/View.java
@@ -71,8 +71,11 @@ public class View {
@JsonProperty("endUnit") TimeUnit endUnit,
@JsonProperty("fractionalSecondPrecision") Integer fractionalSecondPrecision,
@JsonProperty("isNullable") Boolean isNullable) {
- this.name = name;
- this.type = type;
+ // Fix for views which were created on Calcite 1.4.
+ // After Calcite upgrade star "*" was changed on dynamic star "**"
+ // and type of star was changed to SqlTypeName.DYNAMIC_STAR
+ this.name = "*".equals(name) ? "**" : name;
+ this.type = "*".equals(name) && type == SqlTypeName.ANY ? SqlTypeName.DYNAMIC_STAR : type;
this.precision = precision;
this.scale = scale;
this.intervalQualifier =
http://git-wip-us.apache.org/repos/asf/drill/blob/9274cb92/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/SqlConverter.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/SqlConverter.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/SqlConverter.java
index f900587..8224d97 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/SqlConverter.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/SqlConverter.java
@@ -26,8 +26,6 @@ import com.google.common.collect.ImmutableList;
import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
import org.apache.calcite.adapter.java.JavaTypeFactory;
-import org.apache.calcite.avatica.util.Casing;
-import org.apache.calcite.avatica.util.Quoting;
import org.apache.calcite.jdbc.CalciteSchema;
import org.apache.calcite.jdbc.JavaTypeFactoryImpl;
import org.apache.calcite.plan.ConventionTraitDef;
@@ -37,7 +35,6 @@ import org.apache.calcite.plan.RelOptTable;
import org.apache.calcite.plan.volcano.VolcanoPlanner;
import org.apache.calcite.prepare.CalciteCatalogReader;
import org.apache.calcite.prepare.Prepare;
-import org.apache.calcite.prepare.RelOptTableImpl;
import org.apache.calcite.rel.RelCollationTraitDef;
import org.apache.calcite.rel.RelRoot;
import org.apache.calcite.rel.type.RelDataType;
@@ -49,19 +46,20 @@ import org.apache.calcite.rex.RexLiteral;
import org.apache.calcite.rex.RexNode;
import org.apache.calcite.runtime.Hook;
import org.apache.calcite.schema.SchemaPlus;
+import org.apache.calcite.sql.SqlCall;
+import org.apache.calcite.sql.SqlIdentifier;
import org.apache.calcite.sql.SqlNode;
import org.apache.calcite.sql.SqlOperatorTable;
import org.apache.calcite.sql.parser.SqlParseException;
import org.apache.calcite.sql.parser.SqlParser;
-import org.apache.calcite.sql.parser.SqlParserImplFactory;
import org.apache.calcite.sql.parser.SqlParserPos;
import org.apache.calcite.sql.type.SqlTypeName;
import org.apache.calcite.sql.util.ChainedSqlOperatorTable;
import org.apache.calcite.sql.validate.SqlConformance;
-import org.apache.calcite.sql.validate.SqlConformanceEnum;
import org.apache.calcite.sql.validate.SqlValidatorCatalogReader;
import org.apache.calcite.sql.validate.SqlValidatorImpl;
import org.apache.calcite.sql.validate.SqlValidatorScope;
+import org.apache.calcite.sql.validate.SqlValidatorUtil;
import org.apache.calcite.sql2rel.RelDecorrelator;
import org.apache.calcite.sql2rel.SqlToRelConverter;
import org.apache.calcite.util.Util;
@@ -237,6 +235,51 @@ public class SqlConverter {
RelDataTypeFactory typeFactory, SqlConformance conformance) {
super(opTab, catalogReader, typeFactory, conformance);
}
+
+ @Override
+ protected void validateFrom(
+ SqlNode node,
+ RelDataType targetRowType,
+ SqlValidatorScope scope) {
+ switch (node.getKind()) {
+ case AS:
+ if (((SqlCall) node).operand(0) instanceof SqlIdentifier) {
+ SqlIdentifier tempNode = ((SqlCall) node).operand(0);
+ DrillCalciteCatalogReader catalogReader = (SqlConverter.DrillCalciteCatalogReader) getCatalogReader();
+
+ // Check the schema and throw a valid SchemaNotFound exception instead of TableNotFound exception.
+ if (catalogReader.getTable(Lists.newArrayList(tempNode.names)) == null) {
+ catalogReader.isValidSchema(tempNode.names);
+ }
+ changeNamesIfTableIsTemporary(tempNode);
+ }
+ default:
+ super.validateFrom(node, targetRowType, scope);
+ }
+ }
+
+ @Override
+ public String deriveAlias(
+ SqlNode node,
+ int ordinal) {
+ if (node instanceof SqlIdentifier) {
+ SqlIdentifier tempNode = ((SqlIdentifier) node);
+ changeNamesIfTableIsTemporary(tempNode);
+ }
+ return SqlValidatorUtil.getAlias(node, ordinal);
+ }
+
+ private void changeNamesIfTableIsTemporary(SqlIdentifier tempNode) {
+ List<String> temporaryTableNames = ((SqlConverter.DrillCalciteCatalogReader) getCatalogReader()).getTemporaryNames(tempNode.names);
+ if (temporaryTableNames != null) {
+ SqlParserPos pos = tempNode.getComponentParserPosition(0);
+ List<SqlParserPos> poses = Lists.newArrayList();
+ for (int i = 0; i < temporaryTableNames.size(); i++) {
+ poses.add(i, pos);
+ }
+ tempNode.setNames(temporaryTableNames, poses);
+ }
+ }
}
private static class DrillTypeSystem extends RelDataTypeSystemImpl {
@@ -508,6 +551,16 @@ public class SqlConverter {
this.allowTemporaryTables = false;
}
+ private List<String> getTemporaryNames(List<String> names) {
+ if (mightBeTemporaryTable(names, session.getDefaultSchemaPath(), drillConfig)) {
+ String temporaryTableName = session.resolveTemporaryTableName(names.get(names.size() - 1));
+ if (temporaryTableName != null) {
+ return Lists.newArrayList(temporarySchema, temporaryTableName);
+ }
+ }
+ return null;
+ }
+
/**
* If schema is not indicated (only one element in the list) or schema is default temporary workspace,
* we need to check among session temporary tables in default temporary workspace first.
@@ -520,33 +573,23 @@ public class SqlConverter {
*/
@Override
public Prepare.PreparingTable getTable(final List<String> names) {
- Prepare.PreparingTable temporaryTable = null;
-
- if (mightBeTemporaryTable(names, session.getDefaultSchemaPath(), drillConfig)) {
- String temporaryTableName = session.resolveTemporaryTableName(names.get(names.size() - 1));
- if (temporaryTableName != null) {
- List<String> temporaryNames = Lists.newArrayList(temporarySchema, temporaryTableName);
- temporaryTable = super.getTable(temporaryNames);
+ String originalTableName = session.getOriginalTableNameFromTemporaryTable(names.get(names.size() - 1));
+ if (originalTableName != null) {
+ if (!allowTemporaryTables) {
+ throw UserException
+ .validationError()
+ .message("Temporary tables usage is disallowed. Used temporary table name: [%s].", originalTableName)
+ .build(logger);
}
}
- if (temporaryTable != null) {
- if (allowTemporaryTables) {
- return temporaryTable;
+ // Fix for select from hbase table with schema name in query (example: "SELECT col FROM hbase.t)
+ // from hbase schema (did "USE hbase" before).
+ if (names.size() == getSchemaPaths().size() && getSchemaPaths().size() > 1) {
+ if (names.get(0).equals(getSchemaPaths().get(0).get(0))) {
+ useRootSchemaAsDefault(true);
}
- throw UserException
- .validationError()
- .message("Temporary tables usage is disallowed. Used temporary table name: %s.", names)
- .build(logger);
}
-
- Prepare.PreparingTable table = super.getTable(names);
-
- // Check the schema and throw a valid SchemaNotFound exception instead of TableNotFound exception.
- if (table == null) {
- isValidSchema(names);
- }
-
- return table;
+ return super.getTable(names);
}
@Override
http://git-wip-us.apache.org/repos/asf/drill/blob/9274cb92/exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserSession.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserSession.java b/exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserSession.java
index 2e3ce3e..4edeadc 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserSession.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserSession.java
@@ -287,6 +287,15 @@ public class UserSession implements AutoCloseable {
return temporaryTables.get(tableName.toLowerCase());
}
+ public String getOriginalTableNameFromTemporaryTable(String tableName) {
+ for (String originalTableName : temporaryTables.keySet()) {
+ if (temporaryTables.get(originalTableName).equals(tableName)) {
+ return originalTableName;
+ }
+ }
+ return null;
+ }
+
/**
* Checks if passed table is temporary, table name is case-insensitive.
* Before looking for table checks if passed schema is temporary and returns false if not
http://git-wip-us.apache.org/repos/asf/drill/blob/9274cb92/exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestCTTAS.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestCTTAS.java b/exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestCTTAS.java
index 1553227..b334049 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestCTTAS.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestCTTAS.java
@@ -282,6 +282,21 @@ public class TestCTTAS extends BaseTestQuery {
}
@Test
+ public void testSelectWithJoinOnTemporaryTables() throws Exception {
+ String temporaryLeftTableName = "temporary_left_table_for_Select_with_join";
+ String temporaryRightTableName = "temporary_right_table_for_Select_with_join";
+ test("create TEMPORARY table %s as select 'A' as c1, 'B' as c2 from (values(1))", temporaryLeftTableName);
+ test("create TEMPORARY table %s as select 'A' as c1, 'C' as c2 from (values(1))", temporaryRightTableName);
+
+ testBuilder()
+ .sqlQuery("select t1.c2 col1, t2.c2 col2 from %s t1 join %s t2 on t1.c1 = t2.c1", temporaryLeftTableName, temporaryRightTableName)
+ .unOrdered()
+ .baselineColumns("col1", "col2")
+ .baselineValues("B", "C")
+ .go();
+ }
+
+ @Test
public void testTemporaryAndPersistentTablesPriority() throws Exception {
String name = "temporary_and_persistent_table";
test("use %s", temp2_schema);
http://git-wip-us.apache.org/repos/asf/drill/blob/9274cb92/exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestViewSupport.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestViewSupport.java b/exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestViewSupport.java
index 6efa5ce..a3cb69a 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestViewSupport.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestViewSupport.java
@@ -741,4 +741,14 @@ public class TestViewSupport extends TestBaseViewSupport {
test("DROP TABLE IF EXISTS %s.%s ", DFS_TMP_SCHEMA, tableName);
}
}
+
+ @Test
+ public void selectFromViewCreatedOnCalcite1_4() throws Exception {
+ testBuilder()
+ .sqlQuery("select store_type from cp.`view/view_from_calcite_1_4.view.drill`")
+ .unOrdered()
+ .baselineColumns("store_type")
+ .baselineValues("HeadQuarters")
+ .go();
+ }
}
http://git-wip-us.apache.org/repos/asf/drill/blob/9274cb92/exec/java-exec/src/test/resources/view/view_from_calcite_1_4.view.drill
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/resources/view/view_from_calcite_1_4.view.drill b/exec/java-exec/src/test/resources/view/view_from_calcite_1_4.view.drill
new file mode 100755
index 0000000..6c86d62
--- /dev/null
+++ b/exec/java-exec/src/test/resources/view/view_from_calcite_1_4.view.drill
@@ -0,0 +1,10 @@
+{
+ "name" : "view_from_calcite_1_4",
+ "sql" : "SELECT *\nFROM `cp`.`store.json`\nWHERE `store_id` = 0",
+ "fields" : [ {
+ "name" : "*",
+ "type" : "ANY",
+ "isNullable" : true
+ } ],
+ "workspaceSchemaPath" : [ "dfs", "tmp" ]
+}
\ No newline at end of file