This is an automated email from the ASF dual-hosted git repository. vitalii pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/drill.git
commit 6e3f0f7df69228bd1a3e95b83cfd8399b4722d51 Author: Bohdan Kazydub <[email protected]> AuthorDate: Wed Nov 28 16:51:18 2018 +0200 DRILL-6863: Drop table is not working if path within workspace starts with "/" - Made workspace to be honored when table/view name starts with "/" for DROP TABLE, DROP VIEW, CREATE VIEW and SELECT from view queries; - Made "/{name}" and "{name}" to be equivalent names (the leading "/" is removed) when creating temporary tables so that SELECT ... FROM "/{name}" ... and SELECT ... FROM "{name}" ... produce the same results and behave as regular tables in the context. closes #1557 --- .../drill/exec/planner/sql/SqlConverter.java | 4 +- .../planner/sql/handlers/CreateTableHandler.java | 3 +- .../planner/sql/handlers/DropTableHandler.java | 3 +- .../exec/planner/sql/handlers/ViewHandler.java | 7 +-- .../exec/store/dfs/WorkspaceSchemaFactory.java | 3 +- .../test/java/org/apache/drill/TestDropTable.java | 47 +++++++++++------ .../java/org/apache/drill/exec/sql/TestCTAS.java | 32 ++++++++++++ .../java/org/apache/drill/exec/sql/TestCTTAS.java | 61 ++++++++++++++++++++++ .../org/apache/drill/exec/sql/TestViewSupport.java | 49 +++++++++++++++++ 9 files changed, 185 insertions(+), 24 deletions(-) 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 51aad40..154bf8c 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 @@ -82,6 +82,7 @@ import org.apache.drill.exec.planner.logical.DrillTable; import org.apache.drill.exec.planner.physical.DrillDistributionTraitDef; import org.apache.drill.exec.planner.physical.PlannerSettings; import org.apache.drill.exec.rpc.user.UserSession; +import org.apache.drill.exec.store.dfs.FileSelection; import static org.apache.calcite.util.Static.RESOURCE; import org.apache.drill.shaded.guava.com.google.common.base.Joiner; @@ -624,7 +625,8 @@ public class SqlConverter { private List<String> getTemporaryNames(List<String> names) { if (mightBeTemporaryTable(names, session.getDefaultSchemaPath(), drillConfig)) { - String temporaryTableName = session.resolveTemporaryTableName(names.get(names.size() - 1)); + String tableName = FileSelection.removeLeadingSlash(names.get(names.size() - 1)); + String temporaryTableName = session.resolveTemporaryTableName(tableName); if (temporaryTableName != null) { List<String> temporaryNames = new ArrayList<>(SchemaUtilites.getSchemaPathAsList(temporarySchema)); temporaryNames.add(temporaryTableName); diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java index 928a849..82a111a 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java @@ -55,6 +55,7 @@ import org.apache.drill.exec.planner.sql.DrillSqlOperator; import org.apache.drill.exec.planner.sql.SchemaUtilites; import org.apache.drill.exec.planner.sql.parser.SqlCreateTable; import org.apache.drill.exec.store.AbstractSchema; +import org.apache.drill.exec.store.dfs.FileSelection; import org.apache.drill.exec.util.Pointer; import org.apache.drill.exec.work.foreman.ForemanSetupException; import org.apache.drill.exec.work.foreman.SqlUnsupportedException; @@ -72,7 +73,7 @@ public class CreateTableHandler extends DefaultSqlHandler { @Override public PhysicalPlan getPlan(SqlNode sqlNode) throws ValidationException, RelConversionException, IOException, ForemanSetupException { final SqlCreateTable sqlCreateTable = unwrap(sqlNode, SqlCreateTable.class); - final String originalTableName = sqlCreateTable.getName(); + final String originalTableName = FileSelection.removeLeadingSlash(sqlCreateTable.getName()); final ConvertedRelNode convertedRelNode = validateAndConvert(sqlCreateTable.getQuery()); final RelDataType validatedRowType = convertedRelNode.getValidatedRowType(); diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DropTableHandler.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DropTableHandler.java index c17ac20..4f1a759 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DropTableHandler.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DropTableHandler.java @@ -34,6 +34,7 @@ import org.apache.drill.exec.planner.sql.SchemaUtilites; import org.apache.drill.exec.planner.sql.parser.SqlDropTable; import org.apache.drill.exec.rpc.user.UserSession; import org.apache.drill.exec.store.AbstractSchema; +import org.apache.drill.exec.store.dfs.FileSelection; // SqlHandler for dropping a table. public class DropTableHandler extends DefaultSqlHandler { @@ -56,7 +57,7 @@ public class DropTableHandler extends DefaultSqlHandler { @Override public PhysicalPlan getPlan(SqlNode sqlNode) throws ValidationException, RelConversionException, IOException { SqlDropTable dropTableNode = ((SqlDropTable) sqlNode); - String originalTableName = dropTableNode.getName(); + String originalTableName = FileSelection.removeLeadingSlash(dropTableNode.getName()); SchemaPlus defaultSchema = config.getConverter().getDefaultSchema(); List<String> tableSchema = dropTableNode.getSchema(); DrillConfig drillConfig = context.getConfig(); diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ViewHandler.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ViewHandler.java index e86b90a..012315f 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ViewHandler.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ViewHandler.java @@ -35,6 +35,7 @@ import org.apache.drill.exec.planner.sql.SchemaUtilites; import org.apache.drill.exec.planner.sql.parser.SqlCreateView; import org.apache.drill.exec.planner.sql.parser.SqlDropView; import org.apache.drill.exec.store.AbstractSchema; +import org.apache.drill.exec.store.dfs.FileSelection; import org.apache.drill.exec.work.foreman.ForemanSetupException; import org.apache.calcite.rel.RelNode; import org.apache.calcite.sql.SqlNode; @@ -60,7 +61,7 @@ public abstract class ViewHandler extends DefaultSqlHandler { public PhysicalPlan getPlan(SqlNode sqlNode) throws ValidationException, RelConversionException, IOException, ForemanSetupException { SqlCreateView createView = unwrap(sqlNode, SqlCreateView.class); - final String newViewName = createView.getName(); + final String newViewName = FileSelection.removeLeadingSlash(createView.getName()); // Disallow temporary tables usage in view definition config.getConverter().disallowTemporaryTables(); @@ -87,7 +88,7 @@ public abstract class ViewHandler extends DefaultSqlHandler { final boolean replaced = drillSchema.createView(view); final String summary = String.format("View '%s' %s successfully in '%s' schema", - createView.getName(), replaced ? "replaced" : "created", drillSchema.getFullSchemaName()); + newViewName, replaced ? "replaced" : "created", drillSchema.getFullSchemaName()); return DirectPlan.createDirectPlan(context, true, summary); } @@ -155,7 +156,7 @@ public abstract class ViewHandler extends DefaultSqlHandler { @Override public PhysicalPlan getPlan(SqlNode sqlNode) throws ValidationException, RelConversionException, IOException, ForemanSetupException { SqlDropView dropView = unwrap(sqlNode, SqlDropView.class); - final String viewName = dropView.getName(); + final String viewName = FileSelection.removeLeadingSlash(dropView.getName()); final AbstractSchema drillSchema = SchemaUtilites.resolveToMutableDrillSchema(context.getNewDefaultSchema(), dropView.getSchemaPath()); diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/WorkspaceSchemaFactory.java b/exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/WorkspaceSchemaFactory.java index 63b5497..493278c 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/WorkspaceSchemaFactory.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/WorkspaceSchemaFactory.java @@ -518,7 +518,8 @@ public class WorkspaceSchemaFactory { List<DotDrillFile> files = Collections.emptyList(); try { try { - files = DotDrillUtil.getDotDrills(getFS(), new Path(config.getLocation()), tableName, DotDrillType.VIEW); + files = DotDrillUtil.getDotDrills(getFS(), new Path(config.getLocation()), + FileSelection.removeLeadingSlash(tableName), DotDrillType.VIEW); } catch (AccessControlException e) { if (!schemaConfig.getIgnoreAuthErrors()) { logger.debug(e.getMessage()); diff --git a/exec/java-exec/src/test/java/org/apache/drill/TestDropTable.java b/exec/java-exec/src/test/java/org/apache/drill/TestDropTable.java index 6313d74..0c9067c 100644 --- a/exec/java-exec/src/test/java/org/apache/drill/TestDropTable.java +++ b/exec/java-exec/src/test/java/org/apache/drill/TestDropTable.java @@ -21,6 +21,7 @@ import org.apache.drill.categories.SqlTest; import org.apache.drill.categories.UnlikelyTest; import org.apache.drill.common.exceptions.UserException; import org.apache.hadoop.fs.Path; +import org.junit.BeforeClass; import org.junit.Test; import org.junit.Assert; import org.junit.experimental.categories.Category; @@ -28,16 +29,19 @@ import org.junit.experimental.categories.Category; @Category(SqlTest.class) public class TestDropTable extends PlanTestBase { - private static final String CREATE_SIMPLE_TABLE = "create table %s as select 1 from cp.`employee.json`"; - private static final String CREATE_SIMPLE_VIEW = "create view %s as select 1 from cp.`employee.json`"; - private static final String DROP_TABLE = "drop table %s"; - private static final String DROP_TABLE_IF_EXISTS = "drop table if exists %s"; - private static final String DROP_VIEW_IF_EXISTS = "drop view if exists %s"; - private static final String BACK_TICK = "`"; + private static final String CREATE_SIMPLE_TABLE = "create table `%s` as select 1 from cp.`employee.json`"; + private static final String CREATE_SIMPLE_VIEW = "create view `%s` as select 1 from cp.`employee.json`"; + private static final String DROP_TABLE = "drop table `%s`"; + private static final String DROP_TABLE_IF_EXISTS = "drop table if exists `%s`"; + private static final String DROP_VIEW_IF_EXISTS = "drop view if exists `%s`"; + + @BeforeClass + public static void setup() throws Exception { + test("use dfs.tmp"); + } @Test public void testDropJsonTable() throws Exception { - test("use dfs.tmp"); test("alter session set `store.format` = 'json'"); final String tableName = "simple_json"; @@ -55,7 +59,6 @@ public class TestDropTable extends PlanTestBase { @Test public void testDropParquetTable() throws Exception { - test("use dfs.tmp"); final String tableName = "simple_json"; // create a parquet table @@ -72,7 +75,6 @@ public class TestDropTable extends PlanTestBase { @Test public void testDropTextTable() throws Exception { - test("use dfs.tmp"); test("alter session set `store.format` = 'csv'"); final String csvTable = "simple_csv"; @@ -118,7 +120,6 @@ public class TestDropTable extends PlanTestBase { @Test public void testNonHomogenousDrop() throws Exception { - test("use dfs.tmp"); final String tableName = "homogenous_table"; // create a parquet table @@ -127,7 +128,7 @@ public class TestDropTable extends PlanTestBase { // create a json table within the same directory test("alter session set `store.format` = 'json'"); final String nestedJsonTable = tableName + Path.SEPARATOR + "json_table"; - test(CREATE_SIMPLE_TABLE, BACK_TICK + nestedJsonTable + BACK_TICK); + test(CREATE_SIMPLE_TABLE, nestedJsonTable); boolean dropFailed = false; // this should fail, because the directory contains non-homogenous files @@ -142,7 +143,7 @@ public class TestDropTable extends PlanTestBase { // drop the individual json table testBuilder() - .sqlQuery(DROP_TABLE, BACK_TICK + nestedJsonTable + BACK_TICK) + .sqlQuery(DROP_TABLE, nestedJsonTable) .unOrdered() .baselineColumns("ok", "summary") .baselineValues(true, String.format("Table [%s] dropped", nestedJsonTable)) @@ -174,7 +175,6 @@ public class TestDropTable extends PlanTestBase { @Category(UnlikelyTest.class) public void testDropTableIfExistsWhileTableExists() throws Exception { final String existentTableName = "test_table_exists"; - test("use dfs.tmp"); // successful dropping of existent table test(CREATE_SIMPLE_TABLE, existentTableName); @@ -190,7 +190,6 @@ public class TestDropTable extends PlanTestBase { @Category(UnlikelyTest.class) public void testDropTableIfExistsWhileTableDoesNotExist() throws Exception { final String nonExistentTableName = "test_table_not_exists"; - test("use dfs.tmp"); // dropping of non existent table without error testBuilder() @@ -205,9 +204,7 @@ public class TestDropTable extends PlanTestBase { @Category(UnlikelyTest.class) public void testDropTableIfExistsWhileItIsAView() throws Exception { final String viewName = "test_view"; - try{ - test("use dfs.tmp"); - + try { // dropping of non existent table without error if the view with such name is existed test(CREATE_SIMPLE_VIEW, viewName); testBuilder() @@ -220,4 +217,20 @@ public class TestDropTable extends PlanTestBase { test(DROP_VIEW_IF_EXISTS, viewName); } } + + @Test + public void testDropTableNameStartsWithSlash() throws Exception { + String tableName = "test_table_starts_with_slash_drop"; + try { + test(CREATE_SIMPLE_TABLE, tableName); + testBuilder() + .sqlQuery(DROP_TABLE, "/" + tableName) + .unOrdered() + .baselineColumns("ok", "summary") + .baselineValues(true, String.format("Table [%s] dropped", tableName)) + .go(); + } finally { + test(DROP_TABLE_IF_EXISTS, tableName); + } + } } diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestCTAS.java b/exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestCTAS.java index 49ec6d8..6e9f456 100644 --- a/exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestCTAS.java +++ b/exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestCTAS.java @@ -325,6 +325,38 @@ public class TestCTAS extends BaseTestQuery { } } + @Test + public void testTableIsCreatedWithinWorkspace() throws Exception { + String tableName = "table_created_within_workspace"; + try { + test("CREATE TABLE `%s`.`%s` AS SELECT * FROM cp.`region.json`", DFS_TMP_SCHEMA, "/" + tableName); + testBuilder() + .sqlQuery("SELECT region_id FROM `%s`.`%s` LIMIT 1", DFS_TMP_SCHEMA, tableName) + .unOrdered() + .baselineColumns("region_id") + .baselineValues(0L) + .go(); + } finally { + test("DROP TABLE IF EXISTS `%s`.`%s`", DFS_TMP_SCHEMA, tableName); + } + } + + @Test + public void testTableIsFoundWithinWorkspaceWhenNameStartsWithSlash() throws Exception { + String tableName = "table_found_within_workspace"; + try { + test("CREATE TABLE `%s`.`%s` AS SELECT * FROM cp.`region.json`", DFS_TMP_SCHEMA, tableName); + testBuilder() + .sqlQuery("SELECT region_id FROM `%s`.`%s` LIMIT 1", DFS_TMP_SCHEMA, "/" + tableName) + .unOrdered() + .baselineColumns("region_id") + .baselineValues(0L) + .go(); + } finally { + test("DROP TABLE IF EXISTS `%s`.`%s`", DFS_TMP_SCHEMA, tableName); + } + } + private static void ctasErrorTestHelper(final String ctasSql, final String expErrorMsg) throws Exception { final String createTableSql = String.format(ctasSql, "testTableName"); errorMsgTestHelper(createTableSql, expErrorMsg); 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 55e0b59..ba8bece 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 @@ -446,6 +446,67 @@ public class TestCTTAS extends BaseTestQuery { .go(); } + @Test + public void testTemporaryTableWithAndWithoutLeadingSlashAreTheSame() throws Exception { + String tablename = "table_with_and_without_slash_create"; + + try { + test("CREATE TEMPORARY TABLE %s AS SELECT * FROM cp.`region.json`", tablename); + + expectUserRemoteExceptionWithMessage( + String.format("VALIDATION ERROR: A table or view with given name [%s] already exists in schema [%s]", + tablename, DFS_TMP_SCHEMA)); + + test(String.format("CREATE TEMPORARY TABLE `%s` AS SELECT * FROM cp.`employee.json`", "/" + tablename)); + } finally { + test("DROP TABLE IF EXISTS %s", tablename); + } + } + + @Test + public void testSelectFromTemporaryTableWithAndWithoutLeadingSlash() throws Exception { + String tablename = "select_from_table_with_and_without_slash"; + + try { + test("CREATE TEMPORARY TABLE %s AS SELECT * FROM cp.`region.json`", tablename); + + String query = "SELECT region_id FROM `%s` LIMIT 1"; + + testBuilder() + .sqlQuery(query, tablename) + .unOrdered() + .baselineColumns("region_id") + .baselineValues(0L) + .go(); + + testBuilder() + .sqlQuery(query, "/" + tablename) + .unOrdered() + .baselineColumns("region_id") + .baselineValues(0L) + .go(); + } finally { + test("DROP TABLE IF EXISTS %s", tablename); + } + } + + @Test + public void testDropTemporaryTableNameStartsWithSlash() throws Exception { + String tableName = "table_starts_with_slash_drop"; + + try { + test("CREATE TEMPORARY TABLE `%s` AS SELECT 1 FROM cp.`employee.json`", tableName); + testBuilder() + .sqlQuery("DROP TABLE `%s`", "/" + tableName) + .unOrdered() + .baselineColumns("ok", "summary") + .baselineValues(true, String.format("Temporary table [%s] dropped", tableName)) + .go(); + } finally { + test("DROP TABLE IF EXISTS %s", tableName); + } + } + private void expectUserRemoteExceptionWithMessage(String message) { thrown.expect(UserRemoteException.class); thrown.expectMessage(containsString(message)); 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 e7de7ab..a0773bc 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 @@ -751,4 +751,53 @@ public class TestViewSupport extends TestBaseViewSupport { .baselineValues("HeadQuarters") .go(); } + + @Test + public void testDropViewNameStartsWithSlash() throws Exception { + String viewName = "view_name_starts_with_slash_drop"; + try { + test("CREATE VIEW `%s`.`%s` AS SELECT * FROM cp.`region.json`", DFS_TMP_SCHEMA, viewName); + testBuilder() + .sqlQuery("DROP VIEW `%s`.`%s`", DFS_TMP_SCHEMA, "/" + viewName) + .unOrdered() + .baselineColumns("ok", "summary") + .baselineValues(true, + String.format("View [%s] deleted successfully from schema [%s].", viewName, DFS_TMP_SCHEMA)) + .go(); + } finally { + test("DROP VIEW IF EXISTS `%s`.`%s`", DFS_TMP_SCHEMA, viewName); + } + } + + @Test + public void testViewIsCreatedWithinWorkspace() throws Exception { + String viewName = "view_created_within_workspace"; + try { + test("CREATE VIEW `%s`.`%s` AS SELECT * FROM cp.`region.json`", DFS_TMP_SCHEMA, "/" + viewName); + testBuilder() + .sqlQuery("SELECT region_id FROM `%s`.`%s` LIMIT 1", DFS_TMP_SCHEMA, viewName) + .unOrdered() + .baselineColumns("region_id") + .baselineValues(0L) + .go(); + } finally { + test("DROP VIEW IF EXISTS `%s`.`%s`", DFS_TMP_SCHEMA, viewName); + } + } + + @Test + public void testViewIsFoundWithinWorkspaceWhenNameStartsWithSlash() throws Exception { + String viewName = "view_found_within_workspace"; + try { + test("CREATE VIEW `%s`.`%s` AS SELECT * FROM cp.`region.json`", DFS_TMP_SCHEMA, viewName); + testBuilder() + .sqlQuery("SELECT region_id FROM `%s`.`%s` LIMIT 1", DFS_TMP_SCHEMA, "/" + viewName) + .unOrdered() + .baselineColumns("region_id") + .baselineValues(0L) + .go(); + } finally { + test("DROP VIEW IF EXISTS `%s`.`%s`", DFS_TMP_SCHEMA, viewName); + } + } }
