karubian opened a new pull request, #3635:
URL: https://github.com/apache/iceberg-rust/pull/3635

   ## Which issue does this PR close?
   
   No issue yet.
   
   ## What changes are included in this PR?
   
   Iceberg Java and PyIceberg don't write the `schema-id` key in manifest 
metadata. The id is only in the `schema` JSON. `ManifestMetadata::parse` 
defaults a missing key to 0, so `schema_id()` is 0 for every manifest they 
write, even when `schema().schema_id()` is something else.
   
   This falls back to the id of the parsed `schema` when the key is missing. A 
key that is there is still used as is. The spec says the key is required for v2 
and v3, but Java leaves it out in every version, so the fallback applies to all 
of them.
   
   - Java: 
https://github.com/apache/iceberg/blob/b56b4bf9d6f3f4547fd3773e8fd78239869fad4b/core/src/main/java/org/apache/iceberg/ManifestWriter.java#L474-L478
   - PyIceberg: 
https://github.com/apache/iceberg-python/blob/e3564a54c10960307147377c578680276865ed18/pyiceberg/manifest.py#L1161-L1169
   
   #2683 touches the same lines.
   
   ## Are these changes tested?
   
   Yes. Unit tests in `metadata.rs` cover a missing key for v1 to v3 and a 
present key. The missing key test fails on main.
   
   ## AI Disclosure
   
   I used an AI assistant to help write the change and the tests, and reviewed 
both myself.
   


-- 
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]

Reply via email to