twmb opened a new pull request, #1843:
URL: https://github.com/apache/iceberg-go/pull/1843

   Bumps `github.com/twmb/avro` from v1.7.2 to v1.8.0. Two changes in that 
release affect this repo — one is a compile error, the other is silent.
   
   ### `Schema.Root()` returns `*SchemaNode`
   
   Field access auto-derefs, so only the four value-context uses need a change: 
passing the node to `NullableNode`/`getField`, and assigning it to 
`SchemaField.Type`.
   
   ### `Root()` numeric metadata is now `int64`, not `float64`
   
   v1.8.0 preserves numeric precision when decoding schema JSON metadata, 
because integers above 2^53 previously rounded through `float64`. A field's 
`field-id` property therefore comes back as `int64` where it used to be 
`float64`.
   
   `schemaFieldID` matched only `int` and `float64`, so on v1.8.0 it returns 
`false` for **every** field. Nothing errors — `getFieldIDMap` just ends up 
empty, `avroEncodePartitionData` iterates an empty map, and **partition values 
are written to the manifest as null**. The header schema is still written 
correctly, and the read path drops the same keys for the same reason, so the 
loss is silent in both directions.
   
   Accepting `int64` alongside the existing cases keeps this correct on both 
avro versions.
   
   ### Verification
   
   Two existing tests catch this and fail on the bump without the 
`schemaFieldID` change; both pass with it:
   
   - `table/internal` — 
`TestDataFileStatisticsDecimalPartitionManifestRoundTrip` (decimal partition 
value)
   - `table/compaction` — `TestCollectDeadPositionDeletesPartitioned` (string 
partition values, and this package doesn't import avro at all)
   
   The second one confirms the problem is partition values generally, not 
anything decimal-specific.
   
   `gofmt` and `go vet` are clean on the changed packages. I ran the two tests 
above rather than the full suite locally; relying on CI for the rest.
   
   Disclosure: I maintain `twmb/avro`. The `int64` change there was deliberate 
and matches Avro Java, whose `JsonProperties.getObjectProp` yields an integer 
for integral metadata — v1.7.2's `float64` was the divergence. It should have 
been called out in that release's notes, and I'm fixing that separately.


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