rdblue commented on a change in pull request #3632:
URL: https://github.com/apache/iceberg/pull/3632#discussion_r763457533
##########
File path: core/src/test/java/org/apache/iceberg/TestUpdatePartitionSpec.java
##########
@@ -614,6 +605,65 @@ public void testRemoveAndAddMultiTimes() {
V2Assert.assertEquals("Should match expected spec", v2Expected, updated);
}
+ @Test
+ public void testRemoveAndUpdateWithIdentity() {
+ PartitionSpec expected = PartitionSpec.builderFor(SCHEMA)
+ .identity("ts")
+ .build();
+ PartitionSpec updated = new BaseUpdatePartitionSpec(formatVersion,
expected)
+ .removeField("ts")
+ .addField("ts")
+ .apply();
+
+ if (formatVersion == 1) {
+ Assert.assertEquals("Should match expected spec field size", 2,
updated.fields().size());
+ Assert.assertEquals("Should match expected field name", "ts",
+ updated.fields().get(0).name());
+ Assert.assertEquals("Should match expected field name", "ts",
+ updated.fields().get(1).name());
Review comment:
I think that the reason this tries to modify the name check is this
test, where the field is replaced with a void transform in v1. Instead, I think
this should rename the void partition field like we do in other cases. You can
rename the old field to `ts_1000` using the partition field ID to make the name
unique. That's a better solution than allowing conflicting names.
--
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]
