Index: metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java =================================================================== --- metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java (revision 1153927) +++ metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java (working copy) @@ -1019,6 +1019,39 @@ assertTrue("Able to rename table with invalid name: " + invTblName, false); } + + //try an invalid alter table with partition key name + Table tbl_pk = client.getTable(tbl.getDbName(), tbl.getTableName()); + List partitionKeys = tbl_pk.getPartitionKeys(); + for (FieldSchema fs : partitionKeys) { + fs.setName("invalid_to_change_name"); + fs.setComment("can_change_comment"); + } + tbl_pk.setPartitionKeys(partitionKeys); + try { + client.alter_table(dbName, tblName, tbl_pk); + } catch (InvalidOperationException ex) { + failed = true; + } + assertTrue("Should not have succeeded in altering partition key name", failed); + + //try a valid alter table partition key comment + failed = false; + tbl_pk = client.getTable(tbl.getDbName(), tbl.getTableName()); + partitionKeys = tbl_pk.getPartitionKeys(); + for (FieldSchema fs : partitionKeys) { + fs.setComment("can_change_comment"); + } + tbl_pk.setPartitionKeys(partitionKeys); + try { + client.alter_table(dbName, tblName, tbl_pk); + } catch (InvalidOperationException ex) { + failed = true; + } + assertFalse("Should not have failed alter table partition comment", failed); + Table newT = client.getTable(tbl.getDbName(), tbl.getTableName()); + assertEquals(partitionKeys, newT.getPartitionKeys()); + // try a valid alter table tbl2.setTableName(tblName + "_renamed"); tbl2.getSd().setCols(cols); Index: metastore/src/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java =================================================================== --- metastore/src/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java (revision 1153927) +++ metastore/src/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java (working copy) @@ -19,6 +19,7 @@ import java.io.IOException; import java.net.URI; +import java.util.Iterator; import java.util.List; import org.apache.commons.lang.StringUtils; @@ -27,6 +28,7 @@ import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; +import org.apache.hadoop.hive.metastore.api.FieldSchema; import org.apache.hadoop.hive.metastore.api.InvalidObjectException; import org.apache.hadoop.hive.metastore.api.InvalidOperationException; import org.apache.hadoop.hive.metastore.api.MetaException; @@ -97,10 +99,14 @@ + newt.getTableName() + " doesn't exist"); } - // check that partition keys have not changed, except for virtual views + //check that partition keys have not changed, except for virtual views + //however, allow the partition comments to change + boolean partKeysPartiallyEqual = checkPartialPartKeysEqual(oldt.getPartitionKeys(), + newt.getPartitionKeys()); + if(!oldt.getTableType().equals(TableType.VIRTUAL_VIEW.toString())){ if (oldt.getPartitionKeys().size() != newt.getPartitionKeys().size() - || !oldt.getPartitionKeys().containsAll(newt.getPartitionKeys())) { + || !partKeysPartiallyEqual) { throw new InvalidOperationException( "partition keys can not be changed."); } @@ -215,4 +221,29 @@ } } } -} + + private boolean checkPartialPartKeysEqual(List oldPartKeys, + List newPartKeys) { + //return true if both are null, or false if one is null and the other isn't + if (newPartKeys == null || oldPartKeys == null) { + return oldPartKeys == newPartKeys; + } + if (oldPartKeys.size() != newPartKeys.size()) { + return false; + } + Iterator oldPartKeysIter = oldPartKeys.iterator(); + Iterator newPartKeysIter = newPartKeys.iterator(); + FieldSchema oldFs; + FieldSchema newFs; + while (oldPartKeysIter.hasNext()) { + oldFs = oldPartKeysIter.next(); + newFs = newPartKeysIter.next(); + if (!oldFs.getName().equals(newFs.getName()) || + !oldFs.getType().equals(newFs.getType())) { + return false; + } + } + + return true; + } +} \ No newline at end of file