drill-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From vita...@apache.org
Subject [drill] 03/05: DRILL-6863: Drop table is not working if path within workspace starts with "/"
Date Thu, 29 Nov 2018 20:27:12 GMT
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 <bohdan.kazydub@gmail.com>
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);
+    }
+  }
 }


Mime
View raw message