aokolnychyi commented on a change in pull request #3411:
URL: https://github.com/apache/iceberg/pull/3411#discussion_r823151850



##########
File path: core/src/main/java/org/apache/iceberg/Partitioning.java
##########
@@ -210,7 +211,8 @@ public static StructType partitionType(Table table) {
     }
 
     Map<Integer, PartitionField> fieldMap = Maps.newHashMap();
-    List<NestedField> structFields = Lists.newArrayList();
+    Map<Integer, Type> typeMap = Maps.newHashMap();
+    Map<Integer, String> nameMap = Maps.newHashMap();

Review comment:
       I guess you are right, @szlta. Here is an example to reproduce.
   
   ```
   PartitionSpec initialSpec = PartitionSpec.builderFor(SCHEMA)
       .identity("data")
       .build();
   TestTables.TestTable table = TestTables.create(tableDir, "test", SCHEMA, 
initialSpec, V2_FORMAT_VERSION);
   
   table.updateSpec()
       .removeField("data")
       .commit();
   
   table.updateSpec()
       .addField("data")
       .addField("id")
       .commit();
   
   struct<1000: data: optional string, 1001: data: optional string, 1002: id: 
optional int>
   ```
   
   While it would be great to update the logic that evolves the spec, I think 
we have to adapt the method that builds a common representation too. Otherwise, 
existing tables may be forever broken. Maintaining a set of used names and 
appending a suffix of the field ID sounds like a reasonable approach.
   
   Thoughts, @rdblue @RussellSpitzer @szehon-ho @flyrain?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to