Repository: hive
Updated Branches:
  refs/heads/branch-3 bceee7af4 -> c8b167ff3


HIVE-18991: Drop database cascade doesn't work with materialized views (Jesus 
Camacho Rodriguez, reviewed by Ashutosh Chauhan)


Project: http://git-wip-us.apache.org/repos/asf/hive/repo
Commit: http://git-wip-us.apache.org/repos/asf/hive/commit/c8b167ff
Tree: http://git-wip-us.apache.org/repos/asf/hive/tree/c8b167ff
Diff: http://git-wip-us.apache.org/repos/asf/hive/diff/c8b167ff

Branch: refs/heads/branch-3
Commit: c8b167ff3b3be40f4ca9686b2d029478dedeb60d
Parents: bceee7a
Author: Jesus Camacho Rodriguez <jcama...@apache.org>
Authored: Sat Mar 24 09:03:46 2018 -0700
Committer: Jesus Camacho Rodriguez <jcama...@apache.org>
Committed: Wed Apr 11 00:09:29 2018 +0200

----------------------------------------------------------------------
 .../clientnegative/drop_table_used_by_mv.q      |  8 +++
 .../clientnegative/drop_table_used_by_mv.q.out  | 35 +++++++++++++
 .../hadoop/hive/metastore/HiveMetaStore.java    | 47 +++++++++++++++--
 .../hive/metastore/HiveMetaStoreClient.java     | 25 ++++++---
 .../hadoop/hive/metastore/ObjectStore.java      | 10 +++-
 .../metastore/client/builder/TableBuilder.java  | 13 ++++-
 .../hive/metastore/TestHiveMetaStore.java       | 53 ++++++++++++++++++++
 7 files changed, 176 insertions(+), 15 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hive/blob/c8b167ff/ql/src/test/queries/clientnegative/drop_table_used_by_mv.q
----------------------------------------------------------------------
diff --git a/ql/src/test/queries/clientnegative/drop_table_used_by_mv.q 
b/ql/src/test/queries/clientnegative/drop_table_used_by_mv.q
new file mode 100644
index 0000000..e5949a4
--- /dev/null
+++ b/ql/src/test/queries/clientnegative/drop_table_used_by_mv.q
@@ -0,0 +1,8 @@
+create table mytable (key int, value string);
+insert into mytable values (1, 'val1'), (2, 'val2');
+
+create materialized view mv1 as
+select key, value from mytable;
+
+drop table mytable;
+

http://git-wip-us.apache.org/repos/asf/hive/blob/c8b167ff/ql/src/test/results/clientnegative/drop_table_used_by_mv.q.out
----------------------------------------------------------------------
diff --git a/ql/src/test/results/clientnegative/drop_table_used_by_mv.q.out 
b/ql/src/test/results/clientnegative/drop_table_used_by_mv.q.out
new file mode 100644
index 0000000..5a9cabe
--- /dev/null
+++ b/ql/src/test/results/clientnegative/drop_table_used_by_mv.q.out
@@ -0,0 +1,35 @@
+PREHOOK: query: create table mytable (key int, value string)
+PREHOOK: type: CREATETABLE
+PREHOOK: Output: database:default
+PREHOOK: Output: default@mytable
+POSTHOOK: query: create table mytable (key int, value string)
+POSTHOOK: type: CREATETABLE
+POSTHOOK: Output: database:default
+POSTHOOK: Output: default@mytable
+PREHOOK: query: insert into mytable values (1, 'val1'), (2, 'val2')
+PREHOOK: type: QUERY
+PREHOOK: Input: _dummy_database@_dummy_table
+PREHOOK: Output: default@mytable
+POSTHOOK: query: insert into mytable values (1, 'val1'), (2, 'val2')
+POSTHOOK: type: QUERY
+POSTHOOK: Input: _dummy_database@_dummy_table
+POSTHOOK: Output: default@mytable
+POSTHOOK: Lineage: mytable.key SCRIPT []
+POSTHOOK: Lineage: mytable.value SCRIPT []
+PREHOOK: query: create materialized view mv1 as
+select key, value from mytable
+PREHOOK: type: CREATE_MATERIALIZED_VIEW
+PREHOOK: Input: default@mytable
+PREHOOK: Output: database:default
+PREHOOK: Output: default@mv1
+POSTHOOK: query: create materialized view mv1 as
+select key, value from mytable
+POSTHOOK: type: CREATE_MATERIALIZED_VIEW
+POSTHOOK: Input: default@mytable
+POSTHOOK: Output: database:default
+POSTHOOK: Output: default@mv1
+PREHOOK: query: drop table mytable
+PREHOOK: type: DROPTABLE
+PREHOOK: Input: default@mytable
+PREHOOK: Output: default@mytable
+FAILED: Execution Error, return code 1 from 
org.apache.hadoop.hive.ql.exec.DDLTask. MetaException(message:Exception thrown 
flushing changes to datastore)

http://git-wip-us.apache.org/repos/asf/hive/blob/c8b167ff/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
----------------------------------------------------------------------
diff --git 
a/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
 
b/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
index 30922ba..f94fffd 100644
--- 
a/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
+++ 
b/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
@@ -1382,11 +1382,11 @@ public class HiveMetaStore extends ThriftHiveMetastore {
         firePreEvent(new PreDropDatabaseEvent(db, this));
         String catPrependedName = 
MetaStoreUtils.prependCatalogToDbName(catName, name, conf);
 
-        List<String> allTables = get_all_tables(catPrependedName);
+        Set<String> uniqueTableNames = new 
HashSet<>(get_all_tables(catPrependedName));
         List<String> allFunctions = get_functions(catPrependedName, "*");
 
         if (!cascade) {
-          if (!allTables.isEmpty()) {
+          if (!uniqueTableNames.isEmpty()) {
             throw new InvalidOperationException(
                 "Database " + db.getName() + " is not empty. One or more 
tables exist.");
           }
@@ -1409,12 +1409,50 @@ public class HiveMetaStore extends ThriftHiveMetastore {
           drop_function(catPrependedName, funcName);
         }
 
-        // drop tables before dropping db
-        int tableBatchSize = MetastoreConf.getIntVar(conf,
+        final int tableBatchSize = MetastoreConf.getIntVar(conf,
             ConfVars.BATCH_RETRIEVE_MAX);
 
+        // First pass will drop the materialized views
+        List<String> materializedViewNames = get_tables_by_type(name, ".*", 
TableType.MATERIALIZED_VIEW.toString());
         int startIndex = 0;
         // retrieve the tables from the metastore in batches to alleviate 
memory constraints
+        while (startIndex < materializedViewNames.size()) {
+          int endIndex = Math.min(startIndex + tableBatchSize, 
materializedViewNames.size());
+
+          List<Table> materializedViews;
+          try {
+            materializedViews = ms.getTableObjectsByName(catName, name, 
materializedViewNames.subList(startIndex, endIndex));
+          } catch (UnknownDBException e) {
+            throw new MetaException(e.getMessage());
+          }
+
+          if (materializedViews != null && !materializedViews.isEmpty()) {
+            for (Table materializedView : materializedViews) {
+              if (materializedView.getSd().getLocation() != null) {
+                Path materializedViewPath = wh.getDnsPath(new 
Path(materializedView.getSd().getLocation()));
+                if (!wh.isWritable(materializedViewPath.getParent())) {
+                  throw new MetaException("Database metadata not deleted since 
table: " +
+                      materializedView.getTableName() + " has a parent 
location " + materializedViewPath.getParent() +
+                      " which is not writable by " + SecurityUtils.getUser());
+                }
+
+                if (!isSubdirectory(databasePath, materializedViewPath)) {
+                  tablePaths.add(materializedViewPath);
+                }
+              }
+              // Drop the materialized view but not its data
+              drop_table(name, materializedView.getTableName(), false);
+              // Remove from all tables
+              uniqueTableNames.remove(materializedView.getTableName());
+            }
+          }
+          startIndex = endIndex;
+        }
+
+        // drop tables before dropping db
+        List<String> allTables = new ArrayList<>(uniqueTableNames);
+        startIndex = 0;
+        // retrieve the tables from the metastore in batches to alleviate 
memory constraints
         while (startIndex < allTables.size()) {
           int endIndex = Math.min(startIndex + tableBatchSize, 
allTables.size());
 
@@ -1427,7 +1465,6 @@ public class HiveMetaStore extends ThriftHiveMetastore {
 
           if (tables != null && !tables.isEmpty()) {
             for (Table table : tables) {
-
               // If the table is not external and it might not be in a 
subdirectory of the database
               // add it's locations to the list of paths to delete
               Path tablePath = null;

http://git-wip-us.apache.org/repos/asf/hive/blob/c8b167ff/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
----------------------------------------------------------------------
diff --git 
a/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
 
b/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
index 95a3767..003dced 100644
--- 
a/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
+++ 
b/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
@@ -1000,15 +1000,24 @@ public class HiveMetaStoreClient implements 
IMetaStoreClient, AutoCloseable {
     }
 
     if (cascade) {
-       List<String> tableList = getAllTables(dbName);
-       for (String table : tableList) {
-         try {
-           // Subclasses can override this step (for example, for temporary 
tables)
-           dropTable(dbName, table, deleteData, true);
-         } catch (UnsupportedOperationException e) {
-           // Ignore Index tables, those will be dropped with parent tables
-         }
+      // Note that this logic may drop some of the tables of the database
+      // even if the drop database fail for any reason
+      // TODO: Fix this
+      List<String> materializedViews = getTables(dbName, ".*", 
TableType.MATERIALIZED_VIEW);
+      for (String table : materializedViews) {
+        // First we delete the materialized views
+        dropTable(dbName, table, deleteData, true);
+      }
+      List<String> tableList = getAllTables(dbName);
+      for (String table : tableList) {
+        // Now we delete the rest of tables
+        try {
+          // Subclasses can override this step (for example, for temporary 
tables)
+          dropTable(dbName, table, deleteData, true);
+        } catch (UnsupportedOperationException e) {
+          // Ignore Index tables, those will be dropped with parent tables
         }
+      }
     }
     client.drop_database(prependCatalogToDbName(catalogName, dbName, conf), 
deleteData, cascade);
   }

http://git-wip-us.apache.org/repos/asf/hive/blob/c8b167ff/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
----------------------------------------------------------------------
diff --git 
a/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
 
b/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
index 2056930..c5da7b5 100644
--- 
a/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
+++ 
b/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
@@ -1819,6 +1819,7 @@ public class ObjectStore implements RawStore, 
Configurable {
         lowered_tbl_names.add(normalizeIdentifier(t));
       }
       query = pm.newQuery(MTable.class);
+//<<<<<<< HEAD
       query.setFilter("database.name == db && database.catalogName == cat && 
tbl_names.contains(tableName)");
       query.declareParameters("java.lang.String db, java.lang.String cat, 
java.util.Collection tbl_names");
       Collection mtables = (Collection) query.execute(db, catName, 
lowered_tbl_names);
@@ -1835,7 +1836,14 @@ public class ObjectStore implements RawStore, 
Configurable {
         }
       } else {
         for (Iterator iter = mtables.iterator(); iter.hasNext(); ) {
-          tables.add(convertToTable((MTable) iter.next()));
+          Table tbl = convertToTable((MTable) iter.next());
+          // Retrieve creation metadata if needed
+          if 
(TableType.MATERIALIZED_VIEW.toString().equals(tbl.getTableType())) {
+            tbl.setCreationMetadata(
+                convertToCreationMetadata(
+                    getCreationMetadata(tbl.getCatName(), tbl.getDbName(), 
tbl.getTableName())));
+          }
+          tables.add(tbl);
         }
       }
       committed = commitTransaction();

http://git-wip-us.apache.org/repos/asf/hive/blob/c8b167ff/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/client/builder/TableBuilder.java
----------------------------------------------------------------------
diff --git 
a/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/client/builder/TableBuilder.java
 
b/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/client/builder/TableBuilder.java
index 79ef7de..055a46e 100644
--- 
a/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/client/builder/TableBuilder.java
+++ 
b/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/client/builder/TableBuilder.java
@@ -22,7 +22,7 @@ import org.apache.hadoop.hive.common.ValidTxnList;
 import org.apache.hadoop.hive.metastore.IMetaStoreClient;
 import org.apache.hadoop.hive.metastore.TableType;
 import org.apache.hadoop.hive.metastore.Warehouse;
-import org.apache.hadoop.hive.metastore.api.BasicTxnInfo;
+
 import org.apache.hadoop.hive.metastore.api.CreationMetadata;
 import org.apache.hadoop.hive.metastore.api.Database;
 import org.apache.hadoop.hive.metastore.api.FieldSchema;
@@ -47,6 +47,7 @@ import java.util.Set;
 public class TableBuilder extends StorageDescriptorBuilder<TableBuilder> {
   private String catName, dbName, tableName, owner, viewOriginalText, 
viewExpandedText, type,
       mvValidTxnList;
+  private CreationMetadata cm;
   private List<FieldSchema> partCols;
   private int createTime, lastAccessTime, retention;
   private Map<String, String> tableParams;
@@ -108,6 +109,11 @@ public class TableBuilder extends 
StorageDescriptorBuilder<TableBuilder> {
     return this;
   }
 
+  public TableBuilder setCreationMetadata(CreationMetadata cm) {
+    this.cm = cm;
+    return this;
+  }
+
   public TableBuilder setPartCols(List<FieldSchema> partCols) {
     this.partCols = partCols;
     return this;
@@ -165,6 +171,11 @@ public class TableBuilder extends 
StorageDescriptorBuilder<TableBuilder> {
     return this;
   }
 
+  public TableBuilder addMaterializedViewReferencedTables(Set<String> 
tableNames) {
+    mvReferencedTables.addAll(tableNames);
+    return this;
+  }
+
   public TableBuilder setMaterializedViewValidTxnList(ValidTxnList 
validTxnList) {
     mvValidTxnList = validTxnList.writeToString();
     return this;

http://git-wip-us.apache.org/repos/asf/hive/blob/c8b167ff/standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java
----------------------------------------------------------------------
diff --git 
a/standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java
 
b/standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java
index 5a56caf..6f52a52 100644
--- 
a/standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java
+++ 
b/standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java
@@ -39,6 +39,8 @@ import java.util.concurrent.Executors;
 import java.util.concurrent.Future;
 import java.util.concurrent.TimeUnit;
 
+import com.google.common.collect.Sets;
+import org.apache.hadoop.hive.metastore.api.CreationMetadata;
 import org.apache.hadoop.hive.metastore.client.builder.DatabaseBuilder;
 import org.apache.hadoop.hive.metastore.client.builder.TableBuilder;
 import org.apache.hadoop.hive.metastore.conf.MetastoreConf;
@@ -2708,6 +2710,18 @@ public abstract class TestHiveMetaStore {
         .create(client, conf);
   }
 
+  private void createMaterializedView(String dbName, String tableName, 
Set<String> tablesUsed)
+      throws TException {
+    Table t = new TableBuilder()
+        .setDbName(dbName)
+        .setTableName(tableName)
+        .setType(TableType.MATERIALIZED_VIEW.name())
+        .addMaterializedViewReferencedTables(tablesUsed)
+        .addCol("foo", "string")
+        .addCol("bar", "string")
+        .create(client, conf);
+  }
+
   private List<Partition> createPartitions(String dbName, Table tbl,
       List<List<String>> values)  throws Throwable {
     int i = 1;
@@ -2814,6 +2828,7 @@ public abstract class TestHiveMetaStore {
     for (String tableName : tableNames) {
       createTable(dbName, tableName);
     }
+    createMaterializedView(dbName, "mv1", Sets.newHashSet("db.table1", 
"db.table2"));
 
     // Test
     List<Table> tableObjs = client.getTableObjectsByName(dbName, tableNames);
@@ -2829,6 +2844,44 @@ public abstract class TestHiveMetaStore {
   }
 
   @Test
+  public void testDropDatabaseCascadeMVMultiDB() throws Exception {
+    String dbName1 = "db1";
+    String tableName1 = "table1";
+    String dbName2 = "db2";
+    String tableName2 = "table2";
+    String mvName = "mv1";
+
+    // Setup
+    silentDropDatabase(dbName1);
+    silentDropDatabase(dbName2);
+
+    Database db1 = new Database();
+    db1.setName(dbName1);
+    client.createDatabase(db1);
+    createTable(dbName1, tableName1);
+    Database db2 = new Database();
+    db2.setName(dbName2);
+    client.createDatabase(db2);
+    createTable(dbName2, tableName2);
+
+    createMaterializedView(dbName2, mvName, Sets.newHashSet("db1.table1", 
"db2.table2"));
+
+    boolean exceptionFound = false;
+    try {
+      // Cannot drop db1 because mv1 uses one of its tables
+      // TODO: Error message coming from metastore is currently not very 
concise
+      // (foreign key violation), we should make it easily understandable
+      client.dropDatabase(dbName1, true, true, true);
+    } catch (Exception e) {
+      exceptionFound = true;
+    }
+    assertTrue(exceptionFound);
+
+    client.dropDatabase(dbName2, true, true, true);
+    client.dropDatabase(dbName1, true, true, true);
+  }
+
+  @Test
   public void testDBLocationChange() throws IOException, TException {
     final String dbName = "alterDbLocation";
     String defaultUri = MetastoreConf.getVar(conf, ConfVars.WAREHOUSE) + 
"/default_location.db";

Reply via email to