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 511198adc5..28f8f8dc5f 100644 --- a/ql/src/test/org/apache/hadoop/hive/ql/TestTxnConcatenate.java +++ b/ql/src/test/org/apache/hadoop/hive/ql/TestTxnConcatenate.java @@ -225,14 +225,19 @@ public void testRenameTable() throws Exception { 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'")); } } 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 ed53c90eb6..5c7de9bd52 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 @@ -63,6 +63,7 @@ import java.util.Map.Entry; import static org.apache.hadoop.hive.metastore.Warehouse.DEFAULT_CATALOG_NAME; +import static org.apache.hadoop.hive.metastore.Warehouse.getCatalogQualifiedTableName; import static org.apache.hadoop.hive.metastore.utils.MetaStoreUtils.getDefaultCatalog; import static org.apache.hadoop.hive.metastore.utils.StringUtils.normalizeIdentifier; @@ -252,6 +253,13 @@ public void alterTable(RawStore msdb, Warehouse wh, String catName, String dbnam " 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 " + dbname + "." + name + " failed"); + throw new MetaException("Alter table not allowed for table " + + getCatalogQualifiedTableName(catName, dbname, name) + + "to new table = " + getCatalogQualifiedTableName(catName, newDbName, name)); + } } if (isPartitionedTable) { @@ -348,29 +356,10 @@ public void alterTable(RawStore msdb, Warehouse wh, String catName, String dbnam } 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 +400,12 @@ public void alterTable(RawStore msdb, Warehouse wh, String catName, String dbnam } 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 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); } } 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 b9f5fb874d..0e204403e9 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 @@ -240,6 +240,15 @@ public TTransport getTransport(TTransport trans) { } } + 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, @@ -3843,19 +3852,6 @@ public Partition exchange_partition(Map partitionSpecs, 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 exchange_partitions(Map partitionSpecs, String sourceDbName, String sourceTableName, String destDbName, @@ -3944,7 +3940,9 @@ private boolean isRenameAllowed(String catalogName, String srcDBName, String des } } - 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); @@ -4953,11 +4951,6 @@ private void alter_table_core(final String catName, final String dbname, final S 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);