Index: hcatalog/core/src/test/java/org/apache/hive/hcatalog/data/schema/TestHCatSchema.java =================================================================== --- hcatalog/core/src/test/java/org/apache/hive/hcatalog/data/schema/TestHCatSchema.java (revision 1537313) +++ hcatalog/core/src/test/java/org/apache/hive/hcatalog/data/schema/TestHCatSchema.java (working copy) @@ -18,12 +18,11 @@ */ package org.apache.hive.hcatalog.data.schema; +import java.util.ArrayList; +import java.util.List; import junit.framework.TestCase; import org.apache.hive.hcatalog.common.HCatException; -import java.util.ArrayList; -import java.util.List; - public class TestHCatSchema extends TestCase { public void testCannotAddFieldMoreThanOnce() throws HCatException { List fieldSchemaList = new ArrayList(); @@ -100,4 +99,27 @@ assertFalse(ex.getMessage(), true); } } + + // HIVE-5336. Re-number the position after remove such that: + // (1) getPosition on a column always returns a value between 0..schema.size()-1 + // (2) getPosition() on 2 different columns should never give the same value. + public void testRemoveAddField2() throws HCatException { + List fieldSchemaList = new ArrayList(); + HCatFieldSchema memberIDField = new HCatFieldSchema("memberID", HCatFieldSchema.Type.INT, "id as number"); + HCatFieldSchema locationField = new HCatFieldSchema("location", HCatFieldSchema.Type.STRING, "loc as string"); + HCatFieldSchema memberNameField = new HCatFieldSchema("memberName", HCatFieldSchema.Type.STRING, "name as string"); + HCatFieldSchema memberSalaryField = new HCatFieldSchema("memberSalary", HCatFieldSchema.Type.INT, "sal as number"); + fieldSchemaList.add(memberIDField); + fieldSchemaList.add(locationField); + fieldSchemaList.add(memberNameField); + fieldSchemaList.add(memberSalaryField); + HCatSchema schema = new HCatSchema(fieldSchemaList); + schema.remove(locationField); + assertTrue("The position of atleast one of the fields is incorrect" , + schema.getPosition(memberIDField.getName()) == 0 && + schema.getPosition(locationField.getName()) == null && + schema.getPosition(memberNameField.getName()) == 1 && + schema.getPosition(memberSalaryField.getName()) == 2); + } + } Index: hcatalog/core/src/main/java/org/apache/hive/hcatalog/data/schema/HCatSchema.java =================================================================== --- hcatalog/core/src/main/java/org/apache/hive/hcatalog/data/schema/HCatSchema.java (revision 1537313) +++ hcatalog/core/src/main/java/org/apache/hive/hcatalog/data/schema/HCatSchema.java (working copy) @@ -91,6 +91,8 @@ } /** + * Note : The position will be re-numbered when one of the preceding columns are removed. + * Hence, the client should not cache this value and expect it to be always valid. * @param fieldName * @return the index of field named fieldName in Schema. If field is not * present, returns null. @@ -115,13 +117,24 @@ return fieldSchemas.size(); } + private void reAlignPositionMap(int startPosition, int offset) { + for (Map.Entry entry : fieldPositionMap.entrySet()) { + // Re-align the columns appearing on or after startPostion(say, column 1) such that + // column 2 becomes column (2+offset), column 3 becomes column (3+offset) and so on. + Integer entryVal = entry.getValue(); + if (entryVal >= startPosition) { + entry.setValue(entryVal+offset); + } + } + } + public void remove(final HCatFieldSchema hcatFieldSchema) throws HCatException { - if (!fieldSchemas.contains(hcatFieldSchema)) { throw new HCatException("Attempt to delete a non-existent column from HCat Schema: " + hcatFieldSchema); - } - + } fieldSchemas.remove(hcatFieldSchema); + // Re-align the positionMap by -1 for the columns appearing after hcatFieldSchema. + reAlignPositionMap(fieldPositionMap.get(hcatFieldSchema.getName())+1, -1); fieldPositionMap.remove(hcatFieldSchema.getName()); fieldNames.remove(hcatFieldSchema.getName()); }