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 0000000000..e5949a4f9a --- /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; + 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 0000000000..5a9cabe7a1 --- /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) 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 450da4f0f0..46e23db02a 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 @@ -1377,11 +1377,11 @@ private void drop_database_core(RawStore ms, String catName, firePreEvent(new PreDropDatabaseEvent(db, this)); String catPrependedName = MetaStoreUtils.prependCatalogToDbName(catName, name, conf); - List allTables = get_all_tables(catPrependedName); + Set uniqueTableNames = new HashSet<>(get_all_tables(catPrependedName)); List 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."); } @@ -1404,12 +1404,50 @@ private void drop_database_core(RawStore ms, String catName, 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 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 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 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()); @@ -1422,7 +1460,6 @@ private void drop_database_core(RawStore ms, String catName, 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; 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 ebbf465244..ed2ade7e70 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 void dropDatabase(String catalogName, String dbName, boolean deleteData, } if (cascade) { - List 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 materializedViews = getTables(dbName, ".*", TableType.MATERIALIZED_VIEW); + for (String table : materializedViews) { + // First we delete the materialized views + dropTable(dbName, table, deleteData, true); + } + List 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); } 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 20569303c4..c5da7b5d34 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 @@ private MTable getMTable(String catName, String db, String table) { 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 @@ private MTable getMTable(String catName, String db, String table) { } } 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(); 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 79ef7debcd..055a46e5e4 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.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 @@ public class TableBuilder extends StorageDescriptorBuilder { private String catName, dbName, tableName, owner, viewOriginalText, viewExpandedText, type, mvValidTxnList; + private CreationMetadata cm; private List partCols; private int createTime, lastAccessTime, retention; private Map tableParams; @@ -108,6 +109,11 @@ public TableBuilder setType(String type) { return this; } + public TableBuilder setCreationMetadata(CreationMetadata cm) { + this.cm = cm; + return this; + } + public TableBuilder setPartCols(List partCols) { this.partCols = partCols; return this; @@ -165,6 +171,11 @@ public TableBuilder addMaterializedViewReferencedTable(String tableName) { return this; } + public TableBuilder addMaterializedViewReferencedTables(Set tableNames) { + mvReferencedTables.addAll(tableNames); + return this; + } + public TableBuilder setMaterializedViewValidTxnList(ValidTxnList validTxnList) { mvValidTxnList = validTxnList.writeToString(); return this; 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 5a56cafcdc..34bb10478c 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.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,20 @@ private void createTable(String dbName, String tableName) throws TException { .create(client, conf); } + private void createMaterializedView(String dbName, String tableName, Set tablesUsed) + throws TException { + Table t = new TableBuilder() + .setCatName(Warehouse.DEFAULT_DATABASE_NAME) + .setDbName(dbName) + .setTableName(tableName) + .setType(TableType.MATERIALIZED_VIEW.name()) + .addMaterializedViewReferencedTables(tablesUsed) + .addCol("foo", "string") + .addCol("bar", "string") + .create(client, conf); + client.createTable(t); + } + private List createPartitions(String dbName, Table tbl, List> values) throws Throwable { int i = 1; @@ -2814,6 +2830,7 @@ public void testGetTableObjects() throws Exception { for (String tableName : tableNames) { createTable(dbName, tableName); } + createMaterializedView(dbName, "mv1", Sets.newHashSet("db.table1", "db.table2")); // Test List
tableObjs = client.getTableObjectsByName(dbName, tableNames); @@ -2828,6 +2845,44 @@ public void testGetTableObjects() throws Exception { client.dropDatabase(dbName, true, true, true); } + @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";