RussellSpitzer commented on a change in pull request #3411:
URL: https://github.com/apache/iceberg/pull/3411#discussion_r813393371
##########
File path: core/src/main/java/org/apache/iceberg/Partitioning.java
##########
@@ -228,11 +229,22 @@ public static StructType partitionType(Table table) {
fieldMap.put(fieldId, field);
NestedField structField = spec.partitionType().field(fieldId);
structFields.add(structField);
+
+ if (Transforms.alwaysNull().equals(field.transform())) {
+ fieldToIndex.put(fieldId, structFields.size() - 1);
+ }
} else {
// verify the fields are compatible as they may conflict in v1 tables
ValidationException.check(equivalentIgnoringNames(field,
existingField),
"Conflicting partition fields: ['%s', '%s']",
field, existingField);
+
+ if (Transforms.alwaysNull().equals(existingField.transform()) &&
+ !Transforms.alwaysNull().equals(field.transform())) {
+ fieldMap.put(fieldId, field);
+ NestedField structField = spec.partitionType().field(fieldId);
+ structFields.set(fieldToIndex.get(fieldId), structField);
Review comment:
I think this may be easier (and we wouldn't need to build up the index
map) to just do
```java
fieldMap.put(fieldId, field);
NestedField previousTransform = spec.partitionType().field(fieldId);
voidTransformIndex = structFields.indexOf(existingField)
structFields.set(voidTransformIndex, previousTransform)
```
I recommend this because I think it more clearly expresses what we are
trying to do here. When initially looked at the code It wasn't clear to me that
the fieldToIndex was solely for looking up old void transforms for this edge
case.
--
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]
