pyiceberg BinaryDecoder does not correctly read 4-byte little-endian int values

[alec-heif]

Apache Iceberg version

No response

Query engine

No response

Please describe the bug 🐞

The logic in https://github.com/apache/iceberg/blob/master/python/pyiceberg/avro/decoder.py#L70 appears to be incorrect.

The spec for a binary-encoded int in the manifest files is as follows:

int | Stored as 4-byte little-endian

so, an example bytestring of 0xad4a0000 should be read as the decimal 19117:

1. lsb 0xad is 173
2. 2nd lsb 0x4a is 74
3. (74 * 256) + 173 == 19117

however BinaryDecoder does not read this correctly:

import io
def as_fo(x):
    return io.BytesIO(bytes.fromhex(x))

assert as_fo('ad4a0000').read(4).hex() == 'ad4a0000'
assert BinaryDecoder(as_io('ad4a0000')).read_int() == -4759

it is not obvious by inspection of BinaryDecoder.read_int where the bug is, but it is clearly a bug.

[alec-heif]

fyi @Fokko since this logic appears to have been introduced by #4920

[alec-heif]

somewhat disturbingly, the unit test coverage for this function also verifies the (incorrect) behavior:

mis = MemoryInputStream(b"\xBC\x7D")
decoder = BinaryDecoder(mis)
assert decoder.read_date_from_int() == date(1991, 12, 27)

I believe that 0xBC7D0000 should not be 1991-12-27

1. lsb 0xbc == 188
2. 2nd lsb 0x7d == 125
3. (125 * 256) + 188 == 32188 which is 32188 days, or 2058-02-16

[Fokko]

Hey @alec-heif Thanks for opening this PR. I think we're mixing up different file types. The decoder that you pointed out in the example, is an Avro decoder that adheres to the Avro spec. int and long values are written using variable-length zig-zag coding.

The decoding of the single values happens using the _from_byte_buffer method, which produces the same result as your example:

[alec-heif]

hoooo wow ok that's embarrassing, thanks for correcting!! sorry for the false alarm here, i don't know how i missed this.

[Fokko]

No problem at all, happy to help