From ca9d9d35b60fa11603ac4724e17178b0312a6ed2 Mon Sep 17 00:00:00 2001 From: Ayush Kumar Saha <225407166+AP24110011695@users.noreply.github.com> Date: Wed, 4 Nov 2026 23:21:24 +0530 Subject: [PATCH] fix: handle negative-scale Decimals in increment, decrement and truncate_decimal Decimal.as_tuple().exponent carries the negated scale, so a Decimal with a non-negative exponent (1E+2) has a negative scale. abs() turned that exponent into a positive scale, so increment()/decrement() returned values on the wrong order of magnitude (1E+2 -> 0.02 instead of 2E+2) and truncate_decimal() landed on a wrong scale. Same root cause as #3997, remaining call sites. Signed-off-by: Ayush Kumar Saha <225407166+AP24110011695@users.noreply.github.com> --- pyiceberg/expressions/literals.py | 6 ++++-- pyiceberg/utils/decimal.py | 4 +++- tests/expressions/test_literals.py | 18 ++++++++++++++++++ tests/utils/test_decimal.py | 15 ++++++++++++++- 4 files changed, 39 insertions(+), 4 deletions(-) diff --git a/pyiceberg/expressions/literals.py b/pyiceberg/expressions/literals.py index b91e404627..1920c9aedf 100644 --- a/pyiceberg/expressions/literals.py +++ b/pyiceberg/expressions/literals.py @@ -514,12 +514,14 @@ def __init__(self, value: Decimal) -> None: super().__init__(value, Decimal) def increment(self) -> Literal[Decimal]: - original_scale = abs(int(self.value.as_tuple().exponent)) + # A Decimal carries the negated scale as its exponent, so a value with a + # non-negative exponent (1E+2) has a negative scale and abs() would read it wrong + original_scale = -int(self.value.as_tuple().exponent) unscaled = decimal_to_unscaled(self.value) return DecimalLiteral(unscaled_to_decimal(unscaled + 1, original_scale)) def decrement(self) -> Literal[Decimal]: - original_scale = abs(int(self.value.as_tuple().exponent)) + original_scale = -int(self.value.as_tuple().exponent) unscaled = decimal_to_unscaled(self.value) return DecimalLiteral(unscaled_to_decimal(unscaled - 1, original_scale)) diff --git a/pyiceberg/utils/decimal.py b/pyiceberg/utils/decimal.py index 6d1f73a73c..462d68c04e 100644 --- a/pyiceberg/utils/decimal.py +++ b/pyiceberg/utils/decimal.py @@ -112,7 +112,9 @@ def truncate_decimal(value: Decimal, width: int) -> Decimal: """ unscaled_value = decimal_to_unscaled(value) applied_value = unscaled_value - (((unscaled_value % width) + width) % width) - return unscaled_to_decimal(applied_value, abs(int(value.as_tuple().exponent))) + # A Decimal carries the negated scale as its exponent, so a value with a + # non-negative exponent (1E+2) has a negative scale and abs() would read it wrong + return unscaled_to_decimal(applied_value, -int(value.as_tuple().exponent)) MAX_PRECISION = tuple(math.floor(math.log10(math.fabs(math.pow(2, 8 * pos - 1) - 1))) for pos in range(24)) diff --git a/tests/expressions/test_literals.py b/tests/expressions/test_literals.py index a7f1c2d43d..eb92a47368 100644 --- a/tests/expressions/test_literals.py +++ b/tests/expressions/test_literals.py @@ -1000,6 +1000,24 @@ def test_decimal_literal_decrement() -> None: assert dec.decrement().value.as_tuple() == Decimal("10.122").as_tuple() +def test_decimal_literal_increment_negative_scale() -> None: + dec = DecimalLiteral(Decimal("1E+2")) + # Twice to check that we don't mutate the value + assert dec.increment() == DecimalLiteral(Decimal("2E+2")) + assert dec.increment() == DecimalLiteral(Decimal("2E+2")) + # To check that the scale is still the same (a non-negative exponent means a negative scale) + assert dec.increment().value.as_tuple() == Decimal("2E+2").as_tuple() + + +def test_decimal_literal_decrement_negative_scale() -> None: + dec = DecimalLiteral(Decimal("1E+2")) + # Twice to check that we don't mutate the value + assert dec.decrement() == DecimalLiteral(Decimal("0E+2")) + assert dec.decrement() == DecimalLiteral(Decimal("0E+2")) + # To check that the scale is still the same, also when stepping below zero + assert dec.decrement().decrement() == DecimalLiteral(Decimal("-1E+2")) + + def test_uuid_literal_initialization() -> None: test_uuid = uuid.UUID("f79c3e09-677c-4bbd-a479-3f349cb785e7") uuid_literal = literal(test_uuid) diff --git a/tests/utils/test_decimal.py b/tests/utils/test_decimal.py index d50f7c5707..a20e2ef765 100644 --- a/tests/utils/test_decimal.py +++ b/tests/utils/test_decimal.py @@ -18,7 +18,7 @@ import pytest -from pyiceberg.utils.decimal import bytes_required, decimal_required_bytes, decimal_to_bytes +from pyiceberg.utils.decimal import bytes_required, decimal_required_bytes, decimal_to_bytes, truncate_decimal def test_decimal_required_bytes() -> None: @@ -65,3 +65,16 @@ def test_decimal_to_bytes() -> None: # 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" + + +def test_truncate_decimal() -> None: + assert truncate_decimal(Decimal("12.34"), 10) == Decimal("12.30") + # Negative values truncate toward negative infinity + assert truncate_decimal(Decimal("-12.34"), 10) == Decimal("-12.40") + + +def test_truncate_decimal_negative_scale() -> None: + # A Decimal like 1.5E+3 carries a negative scale; reading its exponent as a + # positive scale would return a value on the wrong order of magnitude + assert truncate_decimal(Decimal("1.5E+3"), 4) == Decimal("1.2E+3") + assert truncate_decimal(Decimal("1.5E+3"), 4).as_tuple() == Decimal("1.2E+3").as_tuple()