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 <rom.ku...@gmail.com> Authored: Thu Nov 2 18:22:36 2017 +0000 Committer: Volodymyr Vysotskyi <vvo...@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