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 don't think this should 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.
--
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]
