rdblue commented on a change in pull request #845: Add persistent IDs to
partition fields
URL: https://github.com/apache/incubator-iceberg/pull/845#discussion_r403257916
##########
File path: core/src/main/java/org/apache/iceberg/PartitionSpecParser.java
##########
@@ -147,7 +156,12 @@ private static void
buildFromJsonFields(PartitionSpec.Builder builder, JsonNode
String transform = JsonUtil.getString(TRANSFORM, element);
int sourceId = JsonUtil.getInt(SOURCE_ID, element);
- builder.add(sourceId, name, transform);
+ // partition field ids are missing in old PartitionSpec, they always
auto-increment from PARTITION_DATA_ID_START
+ if (!hasFieldId) {
Review comment:
For forward-compatibility, I think that this should detect breaking changes
to specs and throw an exception.
If IDs are removed by an older writer, then the IDs will be reassigned. That
means that IDs must be assigned starting at 1000 and should have no gaps. If
there are IDs, this should validate that assumption by checking that the field
actually has the ID that is expected.
We should make a similar change on the write path: for each field, check
that it's field ID is what would be assigned if it were removed by an older
writer. That will prevent newer writers from creating specs that will break.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
