tanmayrauth commented on issue #1027:
URL: https://github.com/apache/iceberg-go/issues/1027#issuecomment-4445845677
Thanks for the thorough analysis @mzzz-zzm, the bug is real and
well-documented, and the Java reference walkthrough nails the correct shape.
I agree with the three-part approach in your second comment:
1. Rename fieldIDToFixedSize → fieldIDToScale (it's already being used as
scale on the read path at line 1903, so the name is just wrong today)
2. Add fieldIDToPrecision map populated from typ.Precision in getFieldIDMap
3. Thread precision into convertDecimalValue via avroPartitionData →
convertLogicalTypeValue
One additional thing worth noting from the write path: [getFieldIDMap is
called at line
1174](https://github.com/tanmayrauth/iceberg-go/blob/775d8aa56e5f509ecd6a0be148787ec6230139bc/manifest.go#L1174-L1173)
for ManifestWriter but the third return value (the current fixedSizes map) is
discarded with _. The fix should wire the new precision map into the writer so
it's available when serializing partition values.
Regarding the sign-extension point for negative decimals, let's keep that
as a separate issue/PR since MarshalBinary on DecimalLiteral should already
produce two's complement bytes, but it's worth verifying independently.
--
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]
