huaxingao commented on PR #17502:
URL: https://github.com/apache/iceberg/pull/17502#issuecomment-5173114852

   Thanks for tracking this down, @DuanRuixiao. The root cause is right: 
`FixedByteBufferWriter` uses `writeBytes()` for a `fixed[N]` field instead of 
`writeFixed()`.
   
   A couple of things:
   
   **1. Iceberg's own reader isn't affected.** `InternalReader`'s `case FIXED` 
falls through to `readBytes()`, which matches the writer's `writeBytes()`, so 
Iceberg reads its own manifests fine (a `ManifestFiles` round-trip returns 
`record_count = 4`). The corruption only shows up in spec-compliant external 
readers that do `readFixed`.
   
   **2. The fix flips the on-disk format both ways.**
   
   | writer \ reader | old reader | new reader |
   |---|---|---|
   | old writer (existing manifests) | ✅ `4` | ❌ corrupted (`-568` in the 
example) |
   | new writer (new manifests) | ❌ error | ✅ `4` |
   
   So the new reader misreads every existing manifest with a `fixed` field, and 
new manifests can't be read by older Iceberg. I think we need an approach that 
doesn't break existing data.


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