vishnuprakaz opened a new pull request, #3746:
URL: https://github.com/apache/iceberg-python/pull/3746
Reopening #3523, which was auto-closed for inactivity (GitHub wouldn't let
me reopen it directly). Rebased on latest `main`; the bug is still present and
CI is green.
# Rationale for this change
`bytes_required` (`pyiceberg/utils/decimal.py`) should return the minimum
number of bytes for a value, but it returns one byte too many for negatives
equal to -2^(8k-1) (e.g. -128, -32768).
This matters because the decimal bucket transform hashes those bytes. The
extra byte changes the hash, so PyIceberg can put a value in a different bucket
than Spark/Java for the same input. For example, `Decimal("-1.28")`:
```python
from decimal import Decimal
from pyiceberg.types import DecimalType
from pyiceberg.transforms import BucketTransform
dt = DecimalType(precision=5, scale=2)
print(BucketTransform(num_buckets=16).transform(dt)(Decimal("-1.28"))) #
prints 12; should be 13
```
The fix computes the length from `(value + 1)` for negatives, which gives
the true minimum and matches the Iceberg spec / Java `BigInteger.toByteArray()`.
## Are these changes tested?
Yes. Added `test_bytes_required` covering the -128 / -32768 / -8388608
boundaries plus positive/negative controls, and negative-boundary assertions in
`test_decimal_to_bytes`. Lint and the decimal/conversion/transform tests pass.
## Are there any user-facing changes?
Yes. For decimal values whose unscaled value is exactly -2^(8k-1), the
computed bucket changes (e.g. 12 -> 13) so it matches other Iceberg engines.
--
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]
