(iceberg-python) branch main updated: fix(decimal): use minimal byte length for negative powers of two (#3746)

[ke...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

kevinjqliu pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg-python.git


The following commit(s) were added to refs/heads/main by this push:
     new 8544bb409 fix(decimal): use minimal byte length for negative powers of two (#3746)
8544bb409 is described below

commit 8544bb40964cc9f6ae8f9ef92b605abf910ff0fe
Author: vishnu prakash <nv...@gmail.com>
AuthorDate: Fri Aug 21 01:39:50 2026 +0530

    fix(decimal): use minimal byte length for negative powers of two (#3746)
    
    * fix(decimal): use minimal byte length for negative powers of two
    
    * test(decimal): add positive Decimal case to
      bytes_required
    
    * test(decimal): cover negative boundary bucketing
    
    ---------
    
    Co-authored-by: Kevin Liu <ke...@gmail.com>
---
 pyiceberg/utils/decimal.py  | 13 +++++++++----
 tests/test_transforms.py    |  1 +
 tests/utils/test_decimal.py | 20 +++++++++++++++++++-
 3 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/pyiceberg/utils/decimal.py b/pyiceberg/utils/decimal.py
index 5ef82640d..0ed2de1d5 100644
--- a/pyiceberg/utils/decimal.py
+++ b/pyiceberg/utils/decimal.py
@@ -58,11 +58,16 @@ def bytes_required(value: int | Decimal) -> int:
         int: the minimum number of bytes needed to serialize the value.
     """
     if isinstance(value, int):
-        return (value.bit_length() + 8) // 8
+        unscaled = value
     elif isinstance(value, Decimal):
-        return (decimal_to_unscaled(value).bit_length() + 8) // 8
-
-    raise ValueError(f"Unsupported value: {value}")
+        unscaled = decimal_to_unscaled(value)
+    else:
+        raise ValueError(f"Unsupported value: {value}")
+
+    # bit_length() ignores the sign, so -128 appears to need 9 signed bits instead of 8.
+    # Adding 1 before counting avoids the extra byte at negative boundaries.
+    num_bits = unscaled.bit_length() if unscaled >= 0 else (unscaled + 1).bit_length()
+    return (num_bits + 8) // 8
 
 
 def decimal_to_bytes(value: Decimal, byte_length: int | None = None) -> bytes:
diff --git a/tests/test_transforms.py b/tests/test_transforms.py
index d296fcdb2..c977fcea1 100644
--- a/tests/test_transforms.py
+++ b/tests/test_transforms.py
@@ -187,6 +187,7 @@ def test_bucket_hash_values(test_input: Any, test_type: PrimitiveType, expected:
         (BucketTransform(100).transform(TimeType()), 81068000000, 59),
         (BucketTransform(100).transform(TimestampType()), 1510871468000000, 7),
         (BucketTransform(100).transform(DecimalType(9, 2)), Decimal("14.20"), 59),
+        (BucketTransform(16).transform(DecimalType(5, 2)), Decimal("-1.28"), 13),
         (BucketTransform(100).transform(StringType()), "iceberg", 89),
         (
             BucketTransform(100).transform(UUIDType()),
diff --git a/tests/utils/test_decimal.py b/tests/utils/test_decimal.py
index 3e67bf691..d50f7c570 100644
--- a/tests/utils/test_decimal.py
+++ b/tests/utils/test_decimal.py
@@ -18,7 +18,7 @@ from decimal import Decimal
 
 import pytest
 
-from pyiceberg.utils.decimal import decimal_required_bytes, decimal_to_bytes
+from pyiceberg.utils.decimal import bytes_required, decimal_required_bytes, decimal_to_bytes
 
 
 def test_decimal_required_bytes() -> None:
@@ -42,8 +42,26 @@ def test_decimal_required_bytes() -> None:
     assert "(0, 40]" in str(exc_info.value)
 
 
+def test_bytes_required() -> None:
+    assert bytes_required(0) == 1
+    assert bytes_required(127) == 1
+    assert bytes_required(128) == 2
+    # Check negative signed-byte boundaries and their neighbors.
+    assert bytes_required(-127) == 1
+    assert bytes_required(-128) == 1
+    assert bytes_required(-129) == 2
+    assert bytes_required(-32768) == 2
+    assert bytes_required(-8388608) == 3
+    assert bytes_required(Decimal("1.27")) == 1
+    assert bytes_required(Decimal("-1.28")) == 1
+    assert bytes_required(Decimal("-327.68")) == 2
+
+
 def test_decimal_to_bytes() -> None:
     # Check the boundary between 2 and 3 bytes.
     # 2 bytes has a minimum of -32,768 and a maximum value of 32,767 (inclusive).
     assert decimal_to_bytes(Decimal("32767.")) == b"\x7f\xff"
     assert decimal_to_bytes(Decimal("32768.")) == b"\x00\x80\x00"
+    # Unscaled values -128 and -32768 require no sign padding.
+    assert decimal_to_bytes(Decimal("-1.28")) == b"\x80"
+    assert decimal_to_bytes(Decimal("-327.68")) == b"\x80\x00"