Backport rewrite delete file fix (#10020) to iceberg 1.5 - #277
jiang95-dev wants to merge 2 commits into
Conversation
| "1.5.2.21": | ||
| com.linkedin.iceberg:iceberg-api: | ||
| - code: "java.class.defaultSerializationChanged" | ||
| old: "class org.apache.iceberg.PartitionSpec" | ||
| new: "class org.apache.iceberg.PartitionSpec" | ||
| justification: "Serialization across versions is not supported" | ||
| - code: "java.class.defaultSerializationChanged" | ||
| old: "class org.apache.iceberg.Schema" | ||
| new: "class org.apache.iceberg.Schema" | ||
| justification: "Serialization across versions is not supported" |
There was a problem hiding this comment.
I am unfamiliar with .palantir revapi
What is this checking?
There was a problem hiding this comment.
RevAPI is the Java API/ABI compatibility check run by the API Binary Compatibility Checks CI job (./gradlew revapi). It compares the current iceberg-api JAR with the last published 1.5.2 baseline.
This backport adds public ID-remapping APIs to Schema and PartitionSpec. Because both classes are Serializable and do not declare an explicit serialVersionUID, those additions change the JVM-computed default serialization ID. RevAPI reports this as java.class.defaultSerializationChanged, so these entries acknowledge the two expected changes.
|
what version are we backporting from? |
|
Its unclear from your commits what is backported vs fix forward. I might backport and commit merge conflicts, then update forward with new commits. |
Adapt field ID reassignment to preserve initial and write defaults, add regression coverage, and accept the expected metadata schema serialization changes. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
946c353 to
ee05306
Compare
1.6.0 |
Currently there are 2 commits. First for backport. Second for fix forward. |
| } else { | ||
| newFields.add(Types.NestedField.required(newIds.get(i), field.name(), type, field.doc())); | ||
| } | ||
| newFields.add(Types.NestedField.from(field).withId(newIds.get(i)).ofType(type).build()); |
There was a problem hiding this comment.
Is there no more differentiation or is it implicit with the from?
A +1 response to this comment to indicate it actually was clean I'll take vs redoing. |
2 participants
Summary
Backport the fix from iceberg #10020. This is needed because we ran into the same issue in production.
Testing Done
gradle build.