Repository: hive Updated Branches: refs/heads/branch-3 90b442c1b -> 1998bfbda
HIVE-19569: alter table db1.t1 rename db2.t2 generates MetaStoreEventListener.onDropTable() (Mahesh Kumar Behera, reviewed by Sankar Hariappan) Project: http://git-wip-us.apache.org/repos/asf/hive/repo Commit: http://git-wip-us.apache.org/repos/asf/hive/commit/1998bfbd Tree: http://git-wip-us.apache.org/repos/asf/hive/tree/1998bfbd Diff: http://git-wip-us.apache.org/repos/asf/hive/diff/1998bfbd Branch: refs/heads/branch-3 Commit: 1998bfbdaff59b9e01f037f502376ad5d2d6e73a Parents: 90b442c Author: Sankar Hariappan <sank...@apache.org> Authored: Mon Jun 18 13:34:32 2018 -0700 Committer: Sankar Hariappan <sank...@apache.org> Committed: Mon Jun 18 13:34:32 2018 -0700 ---------------------------------------------------------------------- .../hadoop/hive/ql/TestTxnConcatenate.java | 24 +++-- .../hadoop/hive/metastore/HiveAlterHandler.java | 99 ++++---------------- .../hadoop/hive/metastore/HiveMetaStore.java | 31 +++--- .../TestTablesCreateDropAlterTruncate.java | 2 +- 4 files changed, 47 insertions(+), 109 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hive/blob/1998bfbd/ql/src/test/org/apache/hadoop/hive/ql/TestTxnConcatenate.java ---------------------------------------------------------------------- diff --git a/ql/src/test/org/apache/hadoop/hive/ql/TestTxnConcatenate.java b/ql/src/test/org/apache/hadoop/hive/ql/TestTxnConcatenate.java index 511198a..0e436e1 100644 --- a/ql/src/test/org/apache/hadoop/hive/ql/TestTxnConcatenate.java +++ b/ql/src/test/org/apache/hadoop/hive/ql/TestTxnConcatenate.java @@ -25,7 +25,6 @@ import org.apache.hadoop.hive.metastore.conf.MetastoreConf; import org.apache.hadoop.hive.metastore.txn.TxnDbUtil; import org.apache.hadoop.hive.metastore.txn.TxnStore; import org.apache.hadoop.hive.metastore.txn.TxnUtils; -import org.apache.hadoop.hive.ql.processors.CommandProcessorResponse; import org.junit.Assert; import org.junit.Rule; import org.junit.Test; @@ -225,14 +224,19 @@ public class TestTxnConcatenate extends TxnCommandsBaseForTests { Assert.assertEquals(1, TxnDbUtil.countQueryAgent(hiveConf, "select count(*) from NEXT_WRITE_ID where NWI_TABLE='s'")); - //this causes MetaStoreEvenListener.onDropTable()/onCreateTable() to execute and the data - //files are just moved under new table. This can't work since a drop table in Acid removes - //the relevant table metadata (like writeid, etc.), so writeIds in file names/ROW_IDs - //no longer make sense. (In fact 'select ...' returns nothing since there is no NEXT_WRITE_ID - //entry for the 'new' table and all existing data is 'above HWM'. see HIVE-19569 - CommandProcessorResponse cpr = - runStatementOnDriverNegative("alter table mydb1.S RENAME TO mydb2.bar"); - Assert.assertTrue(cpr.getErrorMessage() != null && cpr.getErrorMessage() - .contains("Changing database name of a transactional table mydb1.s is not supported.")); + runStatementOnDriver("alter table mydb1.S RENAME TO mydb2.bar"); + + Assert.assertEquals( + TxnDbUtil.queryToString(hiveConf, "select * from COMPLETED_TXN_COMPONENTS"), 2, + TxnDbUtil.countQueryAgent(hiveConf, + "select count(*) from COMPLETED_TXN_COMPONENTS where CTC_TABLE='bar'")); + Assert.assertEquals(1, TxnDbUtil.countQueryAgent(hiveConf, + "select count(*) from COMPACTION_QUEUE where CQ_TABLE='bar'")); + Assert.assertEquals(1, TxnDbUtil.countQueryAgent(hiveConf, + "select count(*) from WRITE_SET where WS_TABLE='bar'")); + Assert.assertEquals(2, TxnDbUtil.countQueryAgent(hiveConf, + "select count(*) from TXN_TO_WRITE_ID where T2W_TABLE='bar'")); + Assert.assertEquals(1, TxnDbUtil.countQueryAgent(hiveConf, + "select count(*) from NEXT_WRITE_ID where NWI_TABLE='bar'")); } } http://git-wip-us.apache.org/repos/asf/hive/blob/1998bfbd/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java ---------------------------------------------------------------------- diff --git a/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java b/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java index ed53c90..f328ad1 100644 --- a/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java +++ b/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java @@ -22,11 +22,8 @@ import com.google.common.collect.Lists; import org.apache.commons.lang.StringUtils; import org.apache.hadoop.hive.metastore.conf.MetastoreConf; -import org.apache.hadoop.hive.metastore.events.AddPartitionEvent; import org.apache.hadoop.hive.metastore.events.AlterPartitionEvent; import org.apache.hadoop.hive.metastore.events.AlterTableEvent; -import org.apache.hadoop.hive.metastore.events.CreateTableEvent; -import org.apache.hadoop.hive.metastore.events.DropTableEvent; import org.apache.hadoop.hive.metastore.messaging.EventMessage; import org.apache.hadoop.hive.metastore.utils.FileUtils; import org.apache.hadoop.hive.metastore.utils.MetaStoreUtils; @@ -102,7 +99,7 @@ public class HiveAlterHandler implements AlterHandler { && StatsSetupConst.TRUE.equals(environmentContext.getProperties().get( StatsSetupConst.CASCADE)); if (newt == null) { - throw new InvalidOperationException("New table is invalid: " + newt); + throw new InvalidOperationException("New table is null"); } String newTblName = newt.getTableName().toLowerCase(); @@ -125,19 +122,10 @@ public class HiveAlterHandler implements AlterHandler { boolean dataWasMoved = false; boolean isPartitionedTable = false; - Table oldt = null; - - List<TransactionalMetaStoreEventListener> transactionalListeners = null; - List<MetaStoreEventListener> listeners = null; + Table oldt; + List<TransactionalMetaStoreEventListener> transactionalListeners = handler.getTransactionalListeners(); + List<MetaStoreEventListener> listeners = handler.getListeners(); Map<String, String> txnAlterTableEventResponses = Collections.emptyMap(); - Map<String, String> txnDropTableEventResponses = Collections.emptyMap(); - Map<String, String> txnCreateTableEventResponses = Collections.emptyMap(); - Map<String, String> txnAddPartitionEventResponses = Collections.emptyMap(); - - if (handler != null) { - transactionalListeners = handler.getTransactionalListeners(); - listeners = handler.getListeners(); - } try { boolean rename = false; @@ -252,6 +240,15 @@ public class HiveAlterHandler implements AlterHandler { " failed to move data due to: '" + getSimpleMessage(e) + "' See hive log file for details."); } + + if (!HiveMetaStore.isRenameAllowed(olddb, db)) { + LOG.error("Alter Table operation for " + Warehouse.getCatalogQualifiedTableName(catName, dbname, name) + + "to new table = " + Warehouse.getCatalogQualifiedTableName(catName, newDbName, newTblName) + + " failed "); + throw new MetaException("Alter table not allowed for table " + + Warehouse.getCatalogQualifiedTableName(catName, dbname, name) + + "to new table = " + Warehouse.getCatalogQualifiedTableName(catName, newDbName, newTblName)); + } } if (isPartitionedTable) { @@ -348,29 +345,10 @@ public class HiveAlterHandler implements AlterHandler { } if (transactionalListeners != null && !transactionalListeners.isEmpty()) { - if (oldt.getDbName().equalsIgnoreCase(newt.getDbName())) { - txnAlterTableEventResponses = MetaStoreListenerNotifier.notifyEvent(transactionalListeners, + txnAlterTableEventResponses = MetaStoreListenerNotifier.notifyEvent(transactionalListeners, EventMessage.EventType.ALTER_TABLE, new AlterTableEvent(oldt, newt, false, true, handler), environmentContext); - } else { - txnDropTableEventResponses = MetaStoreListenerNotifier.notifyEvent(transactionalListeners, - EventMessage.EventType.DROP_TABLE, - new DropTableEvent(oldt, true, false, handler), - environmentContext); - txnCreateTableEventResponses = MetaStoreListenerNotifier.notifyEvent(transactionalListeners, - EventMessage.EventType.CREATE_TABLE, - new CreateTableEvent(newt, true, handler), - environmentContext); - if (isPartitionedTable) { - String cName = newt.isSetCatName() ? newt.getCatName() : DEFAULT_CATALOG_NAME; - parts = msdb.getPartitions(cName, newt.getDbName(), newt.getTableName(), -1); - txnAddPartitionEventResponses = MetaStoreListenerNotifier.notifyEvent(transactionalListeners, - EventMessage.EventType.ADD_PARTITION, - new AddPartitionEvent(newt, parts, true, handler), - environmentContext); - } - } } // commit the changes success = msdb.commitTransaction(); @@ -411,49 +389,12 @@ public class HiveAlterHandler implements AlterHandler { } if (!listeners.isEmpty()) { - // An ALTER_TABLE event will be created for any alter table operation happening inside the same - // database, otherwise a rename between databases is considered a DROP_TABLE from the old database - // and a CREATE_TABLE in the new database plus ADD_PARTITION operations if needed. - if (!success || dbname.equalsIgnoreCase(newDbName)) { - // I don't think event notifications in case of failures are necessary, but other HMS operations - // make this call whether the event failed or succeeded. To make this behavior consistent, then - // this call will be made also for failed events even for renaming the table between databases - // to avoid a large list of ADD_PARTITION unnecessary failed events. - MetaStoreListenerNotifier.notifyEvent(listeners, EventMessage.EventType.ALTER_TABLE, - new AlterTableEvent(oldt, newt, false, success, handler), - environmentContext, txnAlterTableEventResponses, msdb); - } else { - if(oldt.getParameters() != null && "true".equalsIgnoreCase( - oldt.getParameters().get(hive_metastoreConstants.TABLE_IS_TRANSACTIONAL))) { - /*Why does it split Alter into Drop + Create here????? This causes onDropTable logic - * to wipe out acid related metadata and writeIds from old table don't make sense - * in the new table.*/ - throw new IllegalStateException("Changing database name of a transactional table " + - Warehouse.getQualifiedName(oldt) + " is not supported. Please use create-table-as" + - " or create new table manually followed by Insert."); - } - MetaStoreListenerNotifier.notifyEvent(listeners, EventMessage.EventType.DROP_TABLE, - new DropTableEvent(oldt, true, false, handler), - environmentContext, txnDropTableEventResponses, msdb); - - MetaStoreListenerNotifier.notifyEvent(listeners, EventMessage.EventType.CREATE_TABLE, - new CreateTableEvent(newt, true, handler), - environmentContext, txnCreateTableEventResponses, msdb); - - if (isPartitionedTable) { - try { - List<Partition> parts = msdb.getPartitions(catName, newDbName, newTblName, -1); - MetaStoreListenerNotifier.notifyEvent(listeners, EventMessage.EventType.ADD_PARTITION, - new AddPartitionEvent(newt, parts, true, handler), - environmentContext, txnAddPartitionEventResponses, msdb); - } catch (NoSuchObjectException e) { - // Just log the error but not throw an exception as this post-commit event should - // not cause the HMS operation to fail. - LOG.error("ADD_PARTITION events for ALTER_TABLE rename operation cannot continue because the following " + - "table was not found on the metastore: " + newDbName + "." + newTblName, e); - } - } - } + // I don't think event notifications in case of failures are necessary, but other HMS operations + // make this call whether the event failed or succeeded. To make this behavior consistent, + // this call is made for failed events also. + MetaStoreListenerNotifier.notifyEvent(listeners, EventMessage.EventType.ALTER_TABLE, + new AlterTableEvent(oldt, newt, false, success, handler), + environmentContext, txnAlterTableEventResponses, msdb); } } http://git-wip-us.apache.org/repos/asf/hive/blob/1998bfbd/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 80bb1f0..f3ad723 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 @@ -241,6 +241,15 @@ public class HiveMetaStore extends ThriftHiveMetastore { } } + public static boolean isRenameAllowed(Database srcDB, Database destDB) { + if (!srcDB.getName().equalsIgnoreCase(destDB.getName())) { + if (ReplChangeManager.isSourceOfReplication(srcDB) || ReplChangeManager.isSourceOfReplication(destDB)) { + return false; + } + } + return true; + } + public static class HMSHandler extends FacebookBase implements IHMSHandler { public static final Logger LOG = HiveMetaStore.LOG; private final Configuration conf; // stores datastore (jpox) properties, @@ -3910,19 +3919,6 @@ public class HiveMetaStore extends ThriftHiveMetastore { return new Partition(); } - private boolean isRenameAllowed(String catalogName, String srcDBName, String destDBName) - throws MetaException, NoSuchObjectException { - RawStore ms = getMS(); - if (!srcDBName.equalsIgnoreCase(destDBName)) { - Database destDB = ms.getDatabase(catalogName, destDBName); - Database srcDB = ms.getDatabase(catalogName, srcDBName); - if (ReplChangeManager.isSourceOfReplication(srcDB) || ReplChangeManager.isSourceOfReplication(destDB)) { - return false; - } - } - return true; - } - @Override public List<Partition> exchange_partitions(Map<String, String> partitionSpecs, String sourceDbName, String sourceTableName, String destDbName, @@ -4011,7 +4007,9 @@ public class HiveMetaStore extends ThriftHiveMetastore { } } - if (!isRenameAllowed(parsedDestDbName[CAT_NAME], parsedSourceDbName[DB_NAME], parsedDestDbName[DB_NAME])) { + Database srcDb = ms.getDatabase(parsedSourceDbName[CAT_NAME], parsedSourceDbName[DB_NAME]); + Database destDb = ms.getDatabase(parsedDestDbName[CAT_NAME], parsedDestDbName[DB_NAME]); + if (!isRenameAllowed(srcDb, destDb)) { throw new MetaException("Exchange partition not allowed for " + getCatalogQualifiedTableName(parsedSourceDbName[CAT_NAME], parsedSourceDbName[DB_NAME], sourceTableName) + " Dest db : " + destDbName); @@ -5027,11 +5025,6 @@ public class HiveMetaStore extends ThriftHiveMetastore { Exception ex = null; try { Table oldt = get_table_core(catName, dbname, name); - if (!isRenameAllowed(catName, dbname, newTable.getDbName())) { - throw new MetaException("Alter table not allowed for table " + - getCatalogQualifiedTableName(catName, dbname, name) + - " new table = " + getCatalogQualifiedTableName(newTable)); - } firePreEvent(new PreAlterTableEvent(oldt, newTable, this)); alterHandler.alterTable(getMS(), wh, catName, dbname, name, newTable, envContext, this); http://git-wip-us.apache.org/repos/asf/hive/blob/1998bfbd/standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesCreateDropAlterTruncate.java ---------------------------------------------------------------------- diff --git a/standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesCreateDropAlterTruncate.java b/standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesCreateDropAlterTruncate.java index f824dbd..0281182 100644 --- a/standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesCreateDropAlterTruncate.java +++ b/standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesCreateDropAlterTruncate.java @@ -889,7 +889,7 @@ public class TestTablesCreateDropAlterTruncate extends MetaStoreClientTest { } } - @Test(expected = InvalidOperationException.class) + @Test(expected = MetaException.class) public void testAlterTableNullDatabaseInNew() throws Exception { Table originalTable = testTables[0]; Table newTable = originalTable.deepCopy();
