diff --git a/metastore/src/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java b/metastore/src/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java index bae39ac..4ce6a65 100644 --- a/metastore/src/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java +++ b/metastore/src/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java @@ -29,7 +29,6 @@ import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; import org.apache.hadoop.hive.common.FileUtils; -import org.apache.hadoop.hive.common.ObjectPair; import org.apache.hadoop.hive.common.StatsSetupConst; import org.apache.hadoop.hive.conf.HiveConf; import org.apache.hadoop.hive.metastore.api.AlreadyExistsException; @@ -111,10 +110,9 @@ public void alterTable(RawStore msdb, Warehouse wh, String dbname, FileSystem destFs = null; boolean success = false; - boolean moveData = false; + boolean dataWasMoved = false; boolean rename = false; Table oldt = null; - List> altps = new ArrayList>(); List transactionalListeners = null; if (handler != null) { transactionalListeners = handler.getTransactionalListeners(); @@ -212,7 +210,6 @@ public void alterTable(RawStore msdb, Warehouse wh, String dbname, destFs = wh.getFs(destPath); newt.getSd().setLocation(destPath.toString()); - moveData = true; // check that destination does not exist otherwise we will be // overwriting data @@ -222,18 +219,22 @@ public void alterTable(RawStore msdb, Warehouse wh, String dbname, + " is on a different file system than the old location " + srcPath + ". This operation is not supported"); } + try { - srcFs.exists(srcPath); // check that src exists and also checks - // permissions necessary if (destFs.exists(destPath)) { throw new InvalidOperationException("New location for this table " + newt.getDbName() + "." + newt.getTableName() + " already exists : " + destPath); } + // check that src exists and also checks permissions necessary, rename src to dest + if (srcFs.exists(srcPath) && srcFs.rename(srcPath, destPath)) { + dataWasMoved = true; + } } catch (IOException e) { - throw new InvalidOperationException("Unable to access new location " - + destPath + " for table " + newt.getDbName() + "." - + newt.getTableName()); + LOG.error("Alter Table operation for " + dbname + "." + name + " failed.", e); + throw new InvalidOperationException("Alter Table operation for " + dbname + "." + name + + " failed to move data due to: '" + getSimpleMessage(e) + + "' See hive log file for details."); } String oldTblLocPath = srcPath.toUri().getPath(); String newTblLocPath = destPath.toUri().getPath(); @@ -246,7 +247,6 @@ public void alterTable(RawStore msdb, Warehouse wh, String dbname, URI oldUri = new Path(oldPartLoc).toUri(); String newPath = oldUri.getPath().replace(oldTblLocPath, newTblLocPath); Path newPartLocPath = new Path(oldUri.getScheme(), oldUri.getAuthority(), newPath); - altps.add(ObjectPair.create(part, part.getSd().getLocation())); part.getSd().setLocation(newPartLocPath.toString()); String oldPartName = Warehouse.makePartName(oldt.getPartitionKeys(), part.getValues()); try { @@ -289,52 +289,23 @@ public void alterTable(RawStore msdb, Warehouse wh, String dbname, + " Check metastore logs for detailed stack." + e.getMessage()); } finally { if (!success) { + LOG.error("Failed to alter table " + dbname + "." + name); msdb.rollbackTransaction(); - } - - if (success && moveData) { - // change the file name in hdfs - // check that src exists otherwise there is no need to copy the data - // rename the src to destination - try { - if (srcFs.exists(srcPath) && !srcFs.rename(srcPath, destPath)) { - throw new IOException("Renaming " + srcPath + " to " + destPath + " failed"); - } - } catch (IOException e) { - LOG.error("Alter Table operation for " + dbname + "." + name + " failed.", e); - boolean revertMetaDataTransaction = false; + if (dataWasMoved) { try { - msdb.openTransaction(); - msdb.alterTable(newt.getDbName(), newt.getTableName(), oldt); - for (ObjectPair pair : altps) { - Partition part = pair.getFirst(); - part.getSd().setLocation(pair.getSecond()); - msdb.alterPartition(newt.getDbName(), name, part.getValues(), part); - } - revertMetaDataTransaction = msdb.commitTransaction(); - } catch (Exception e1) { - // we should log this for manual rollback by administrator - LOG.error("Reverting metadata by HDFS operation failure failed During HDFS operation failed", e1); - LOG.error("Table " + Warehouse.getQualifiedName(newt) + - " should be renamed to " + Warehouse.getQualifiedName(oldt)); - LOG.error("Table " + Warehouse.getQualifiedName(newt) + - " should have path " + srcPath); - for (ObjectPair pair : altps) { - LOG.error("Partition " + Warehouse.getQualifiedName(pair.getFirst()) + - " should have path " + pair.getSecond()); - } - if (!revertMetaDataTransaction) { - msdb.rollbackTransaction(); + if (destFs.exists(destPath)) { + if (!destFs.rename(destPath, srcPath)) { + LOG.error("Failed to restore data from " + destPath + " to " + srcPath + + " in alter table failure. Manual restore is needed."); + } } + } catch (IOException e) { + LOG.error("Failed to restore data from " + destPath + " to " + srcPath + + " in alter table failure. Manual restore is needed."); } - throw new InvalidOperationException("Alter Table operation for " + dbname + "." + name + - " failed to move data due to: '" + getSimpleMessage(e) + "' See hive log file for details."); } } } - if (!success) { - throw new MetaException("Committing the alter table transaction was not successful."); - } } /** diff --git a/ql/src/test/queries/clientpositive/encryption_move_tbl.q b/ql/src/test/queries/clientpositive/encryption_move_tbl.q index 7a5de7b..0b7771c 100644 --- a/ql/src/test/queries/clientpositive/encryption_move_tbl.q +++ b/ql/src/test/queries/clientpositive/encryption_move_tbl.q @@ -19,9 +19,15 @@ CRYPTO CREATE_ZONE --keyName key_128_2 --path ${hiveconf:hive.metastore.warehous INSERT OVERWRITE TABLE encrypted_table SELECT * FROM src; SHOW TABLES; --- should fail, since they are in different encryption zones +ANALYZE TABLE encrypted_table COMPUTE STATISTICS FOR COLUMNS; +DESCRIBE FORMATTED encrypted_table key; +DESCRIBE FORMATTED encrypted_table value; + +-- should fail, since they are in different encryption zones, but table columns statistics should not change ALTER TABLE default.encrypted_table RENAME TO encrypted_db.encrypted_table_2; SHOW TABLES; +DESCRIBE FORMATTED encrypted_table key; +DESCRIBE FORMATTED encrypted_table value; -- should succeed in Hadoop 2.7 but fail in 2.6 (HDFS-7530) ALTER TABLE default.encrypted_table RENAME TO default.plain_table; diff --git a/ql/src/test/results/clientpositive/encrypted/encryption_move_tbl.q.out b/ql/src/test/results/clientpositive/encrypted/encryption_move_tbl.q.out index cc363ac..8ac9bfc 100644 --- a/ql/src/test/results/clientpositive/encrypted/encryption_move_tbl.q.out +++ b/ql/src/test/results/clientpositive/encrypted/encryption_move_tbl.q.out @@ -45,6 +45,32 @@ POSTHOOK: type: SHOWTABLES POSTHOOK: Input: database:default encrypted_table src +PREHOOK: query: ANALYZE TABLE encrypted_table COMPUTE STATISTICS FOR COLUMNS +PREHOOK: type: QUERY +PREHOOK: Input: default@encrypted_table +#### A PARTIAL masked pattern was here #### data/warehouse/encrypted_table/.hive-staging +POSTHOOK: query: ANALYZE TABLE encrypted_table COMPUTE STATISTICS FOR COLUMNS +POSTHOOK: type: QUERY +POSTHOOK: Input: default@encrypted_table +#### A PARTIAL masked pattern was here #### data/warehouse/encrypted_table/.hive-staging +PREHOOK: query: DESCRIBE FORMATTED encrypted_table key +PREHOOK: type: DESCTABLE +PREHOOK: Input: default@encrypted_table +POSTHOOK: query: DESCRIBE FORMATTED encrypted_table key +POSTHOOK: type: DESCTABLE +POSTHOOK: Input: default@encrypted_table +# col_name data_type min max num_nulls distinct_count avg_col_len max_col_len num_trues num_falses comment + +key int 0 498 0 196 from deserializer +PREHOOK: query: DESCRIBE FORMATTED encrypted_table value +PREHOOK: type: DESCTABLE +PREHOOK: Input: default@encrypted_table +POSTHOOK: query: DESCRIBE FORMATTED encrypted_table value +POSTHOOK: type: DESCTABLE +POSTHOOK: Input: default@encrypted_table +# col_name data_type min max num_nulls distinct_count avg_col_len max_col_len num_trues num_falses comment + +value string 0 214 6.812 7 from deserializer PREHOOK: query: ALTER TABLE default.encrypted_table RENAME TO encrypted_db.encrypted_table_2 PREHOOK: type: ALTERTABLE_RENAME PREHOOK: Input: default@encrypted_table @@ -58,6 +84,24 @@ POSTHOOK: type: SHOWTABLES POSTHOOK: Input: database:default encrypted_table src +PREHOOK: query: DESCRIBE FORMATTED encrypted_table key +PREHOOK: type: DESCTABLE +PREHOOK: Input: default@encrypted_table +POSTHOOK: query: DESCRIBE FORMATTED encrypted_table key +POSTHOOK: type: DESCTABLE +POSTHOOK: Input: default@encrypted_table +# col_name data_type min max num_nulls distinct_count avg_col_len max_col_len num_trues num_falses comment + +key int 0 498 0 196 from deserializer +PREHOOK: query: DESCRIBE FORMATTED encrypted_table value +PREHOOK: type: DESCTABLE +PREHOOK: Input: default@encrypted_table +POSTHOOK: query: DESCRIBE FORMATTED encrypted_table value +POSTHOOK: type: DESCTABLE +POSTHOOK: Input: default@encrypted_table +# col_name data_type min max num_nulls distinct_count avg_col_len max_col_len num_trues num_falses comment + +value string 0 214 6.812 7 from deserializer PREHOOK: query: ALTER TABLE default.encrypted_table RENAME TO default.plain_table PREHOOK: type: ALTERTABLE_RENAME PREHOOK: Input: default@encrypted_table