rdblue commented on a change in pull request #3421:
URL: https://github.com/apache/iceberg/pull/3421#discussion_r739884616



##########
File path: api/src/main/java/org/apache/iceberg/PartitionSpec.java
##########
@@ -350,6 +351,35 @@ private void checkAndAddPartitionName(String name, Integer 
sourceColumnId) {
       }
       Preconditions.checkArgument(name != null && !name.isEmpty(),
           "Cannot use empty or null partition name: %s", name);
+
+      // Rename existing void transformation that conflict with this new name. 
This is an issue with V1 tables which
+      // use void transforms to preserve spec ordering. Void transform names 
do not matter and
+      // by default they take the name of the column which was removed. If we 
attempt to add partitioning on the
+      // column back we can change the name of the void transform to allow for 
this. Currently, BaseUpdatePartitionSpec
+      // should avoid this by adding the field ID to any new void transforms 
but for tables made before that change
+      // the modification below is still needed.
+      if (partitionNames.contains(name)) {

Review comment:
       I would rather this not be here. The job of the spec builder is to 
create the exact spec that is requested and to validate that it is well-formed. 
If a name is used twice, then the spec is malformed and should be rejected. 
Altering the spec field names that were requested violates the expectations of 
the API.
   
   That said, I understand the need to read existing metadata. Maybe we can 
update the builder to have an internal flag that allows this? Then we could 
call that from `PartitionSpecParser` so that there is only one way for this to 
happen?




-- 
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