This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-4029-e6940798b32f14fc7388eb36066e43457d678f6d in repository https://gitbox.apache.org/repos/asf/iceberg-python.git
commit 58df2cca5c82c0e6cdb2668da7f5834b88d3bee9 Author: breken <[email protected]> AuthorDate: Sat Oct 24 07:05:52 2026 +0000 fix(expressions): reject fractional decimal literals on integer columns (#4029) * fix(expressions): reject fractional decimal literals on integer columns DecimalLiteral.to(IntegerType/LongType) rounded the value with to_integral_value(), so an unquoted filter such as "x > 2.6" on an int column was bound as x > 3 and silently dropped x = 3; "x < 2.5" became x < 2 and "x = 2.5" matched x = 2. Raise a ValueError for a value with a fractional part instead, the same way a Decimal with a mismatched scale is rejected. Integral values such as 2.00 still convert. Generated-by: Claude Opus 5.5 Co-Authored-By: Claude Opus 5.5 <[email protected]> * Check for a fractional part before the integer range check The range check rounded the decimal first, so a fractional value near the int/long bounds became an above/below-max sentinel instead of raising, depending on half-even rounding. Reject any fractional value up front. Also replace parametrize with loops in the new literal tests. Co-authored-by: Copilot App <[email protected]> --------- Co-authored-by: breken-ai <[email protected]> Co-authored-by: Claude Opus 5.5 <[email protected]> Co-authored-by: Kevin Liu <[email protected]> Co-authored-by: Copilot App <[email protected]> --- pyiceberg/expressions/literals.py | 15 +++++++++++---- tests/catalog/test_catalog_behaviors.py | 14 ++++++++++++++ tests/expressions/test_literals.py | 27 +++++++++++++++++++++++++++ 3 files changed, 52 insertions(+), 4 deletions(-) diff --git a/pyiceberg/expressions/literals.py b/pyiceberg/expressions/literals.py index 6f9b79e77..b91e40462 100644 --- a/pyiceberg/expressions/literals.py +++ b/pyiceberg/expressions/literals.py @@ -75,6 +75,13 @@ def _parse_numeric_string(value: str) -> Decimal: return number +def _to_integral(value: Decimal, type_var: IcebergType) -> int: + """Convert a Decimal to an int, rejecting values with a fractional part instead of rounding them.""" + if value != value.to_integral_value(): + raise ValueError(f"Could not convert {value} into a {type_var}, value has a fractional part") + return int(value) + + class Literal(IcebergRootModel[L], Generic[L], ABC): # type: ignore """Literal which has a value and can be converted between types.""" @@ -527,8 +534,8 @@ class DecimalLiteral(Literal[Decimal]): raise ValueError(f"Could not convert {self.value} into a {type_var}") @to.register(IntegerType) - def _(self, _: IntegerType) -> Literal[int]: - value_int = int(self.value.to_integral_value()) + def _(self, type_var: IntegerType) -> Literal[int]: + value_int = _to_integral(self.value, type_var) if value_int > IntegerType.max: return IntAboveMax() elif value_int < IntegerType.min: @@ -537,8 +544,8 @@ class DecimalLiteral(Literal[Decimal]): return LongLiteral(value_int) @to.register(LongType) - def _(self, _: LongType) -> Literal[int]: - value_int = int(self.value.to_integral_value()) + def _(self, type_var: LongType) -> Literal[int]: + value_int = _to_integral(self.value, type_var) if value_int > LongType.max: return LongAboveMax() elif value_int < LongType.min: diff --git a/tests/catalog/test_catalog_behaviors.py b/tests/catalog/test_catalog_behaviors.py index 4310ed1eb..1148ca852 100644 --- a/tests/catalog/test_catalog_behaviors.py +++ b/tests/catalog/test_catalog_behaviors.py @@ -1345,6 +1345,20 @@ def test_append_nan_to_identity_partitioned_table(catalog: Catalog) -> None: assert sorted(tbl.scan(row_filter="value is nan").to_arrow()["id"].to_pylist()) == [2, 4] +def test_scan_integer_column_with_decimal_literal(catalog: Catalog) -> None: + catalog.create_namespace("default") + identifier = f"default.scan_integer_decimal_literal_{catalog.name}" + tbl = catalog.create_table(identifier=identifier, schema=pa.schema([pa.field("x", pa.int32())])) + tbl.append(pa.table({"x": pa.array([1, 2, 3, 4], pa.int32())})) + + # A literal with no fractional part, such as 2.0, still filters exactly + assert sorted(tbl.scan(row_filter="x > 2.0").to_arrow()["x"].to_pylist()) == [3, 4] + assert tbl.scan(row_filter="x = 2.0").to_arrow()["x"].to_pylist() == [2] + # Rounding 2.6 to 3 would turn x > 2.6 into x > 3 and silently drop x = 3 + with pytest.raises(ValueError, match="Could not convert 2.6 into a int"): + tbl.scan(row_filter="x > 2.6").to_arrow() + + def test_record_batch_reader_consumed_exactly_once(catalog: Catalog) -> None: """The streaming path must consume the underlying generator exactly once. A regression that drained the reader twice (e.g. an extra .schema access diff --git a/tests/expressions/test_literals.py b/tests/expressions/test_literals.py index 9b73a5022..d3a22d381 100644 --- a/tests/expressions/test_literals.py +++ b/tests/expressions/test_literals.py @@ -922,6 +922,33 @@ def test_decimal_to_long_below_min() -> None: assert isinstance(DecimalLiteral(Decimal(LongType.min - 1)).to(LongType()), LongBelowMin) +def test_fractional_decimal_to_integral_type_raises() -> None: + # Rounding would change the predicate, e.g. x > 2.6 would become x > 3 and drop x = 3 + for target_type in [IntegerType(), LongType()]: + for value in ["2.5", "2.6", "-2.5", "0.1"]: + with pytest.raises(ValueError, match=f"Could not convert {value} into a {target_type}"): + _ = DecimalLiteral(Decimal(value)).to(target_type) + + +def test_integral_decimal_to_integral_type() -> None: + # A decimal literal without a fractional part, such as 2.0, still converts exactly + for target_type in [IntegerType(), LongType()]: + for value in ["2", "2.0", "2.00", "-2.0"]: + assert DecimalLiteral(Decimal(value)).to(target_type) == LongLiteral(int(Decimal(value))) + + +def test_fractional_decimal_near_integral_type_bounds_raises() -> None: + # Rejected even when rounding would cross the bound + for target_type, bound in [ + (IntegerType(), IntegerType.max), + (IntegerType(), IntegerType.min), + (LongType(), LongType.max), + (LongType(), LongType.min), + ]: + with pytest.raises(ValueError, match="value has a fractional part"): + _ = DecimalLiteral(Decimal(f"{bound}.9")).to(target_type) + + def test_string_to_integer_type_invalid_value() -> None: with pytest.raises(ValueError) as e: _ = literal("abc").to(IntegerType())
