From 17274e887577a08299cc912213b7297402d70113 Mon Sep 17 00:00:00 2001 From: hzmsh <35788891+hzmsh@users.noreply.github.com> Date: Wed, 14 Oct 2026 17:36:22 -0500 Subject: [PATCH 1/2] fix(expressions): compare float column filters at float32 precision FloatLiteral kept the double value of the literal, so the evaluators compared it with float32 bounds and partition values. Scans skipped files with matching rows and delete() removed or kept the wrong rows. Narrow the literal to float32 like Java, and do the same for a float initial-default read from metadata. Generated-by: Claude Code (Claude Opus 5.5) Co-Authored-By: Claude Opus 5.5 --- pyiceberg/conversions.py | 7 ++++- pyiceberg/expressions/literals.py | 16 +++++----- tests/catalog/test_catalog_behaviors.py | 41 +++++++++++++++++++++++-- tests/expressions/test_literals.py | 2 +- tests/expressions/test_visitors.py | 21 +++++++++++++ 5 files changed, 76 insertions(+), 11 deletions(-) diff --git a/pyiceberg/conversions.py b/pyiceberg/conversions.py index 6dd964f436..b09f3045a0 100644 --- a/pyiceberg/conversions.py +++ b/pyiceberg/conversions.py @@ -614,8 +614,13 @@ def _(_: TimestamptzType, val: str | int | datetime) -> datetime: @from_json.register(FloatType) +def _(_: FloatType, val: float) -> float: + """JSON float deserializes into a Python float, rounded to float32.""" + return _FLOAT_STRUCT.unpack(_FLOAT_STRUCT.pack(val))[0] + + @from_json.register(DoubleType) -def _(_: FloatType | DoubleType, val: float) -> float: +def _(_: DoubleType, val: float) -> float: """JSON float deserializes into a Python float.""" return val diff --git a/pyiceberg/expressions/literals.py b/pyiceberg/expressions/literals.py index 61581d9b3c..20acd425a9 100644 --- a/pyiceberg/expressions/literals.py +++ b/pyiceberg/expressions/literals.py @@ -366,32 +366,34 @@ def _(self, type_var: DecimalType) -> Literal[Decimal]: class FloatLiteral(Literal[float]): def __init__(self, value: float) -> None: + if isinstance(value, float): + # Narrow to float32 like Java, so the value matches float32 bounds and partition values + value = struct.unpack(" bool: """Return the equality of two instances of the FloatLiteral class.""" - return self._value32 == other + return self.value == other def __lt__(self, other: Any) -> bool: """Return if one instance of the FloatLiteral class is less than another instance.""" - return self._value32 < other + return self.value < other def __gt__(self, other: Any) -> bool: """Return if one instance of the FloatLiteral class is greater than another instance.""" - return self._value32 > other + return self.value > other def __le__(self, other: Any) -> bool: """Return if one instance of the FloatLiteral class is less than or equal to another instance.""" - return self._value32 <= other + return self.value <= other def __ge__(self, other: Any) -> bool: """Return if one instance of the FloatLiteral class is greater than or equal to another instance.""" - return self._value32 >= other + return self.value >= other def __hash__(self) -> int: """Return a hashed representation of the FloatLiteral class.""" - return hash(self._value32) + return hash(self.value) @singledispatchmethod def to(self, type_var: IcebergType) -> Literal: # type: ignore diff --git a/tests/catalog/test_catalog_behaviors.py b/tests/catalog/test_catalog_behaviors.py index 4310ed1eb6..081e924998 100644 --- a/tests/catalog/test_catalog_behaviors.py +++ b/tests/catalog/test_catalog_behaviors.py @@ -42,13 +42,13 @@ from pyiceberg.io.pyarrow import _dataframe_to_data_files, schema_to_pyarrow from pyiceberg.partitioning import UNPARTITIONED_PARTITION_SPEC, PartitionField, PartitionSpec from pyiceberg.schema import Schema -from pyiceberg.table import TableProperties +from pyiceberg.table import Table, TableProperties from pyiceberg.table.snapshots import Operation from pyiceberg.table.sorting import NullOrder, SortDirection, SortField, SortOrder from pyiceberg.table.update import AddSchemaUpdate, SetCurrentSchemaUpdate from pyiceberg.transforms import IdentityTransform from pyiceberg.typedef import Identifier -from pyiceberg.types import BooleanType, DoubleType, IntegerType, LongType, NestedField, StringType +from pyiceberg.types import BooleanType, DoubleType, FloatType, IntegerType, LongType, NestedField, StringType # Name parsing tests @@ -1395,3 +1395,40 @@ def test_record_batch_reader_schema_mismatch_writes_no_files(catalog: Catalog) - # _append_snapshot_producer opens. assert tbl.metadata.current_snapshot() is None assert len(tbl.scan().to_arrow()) == 0 + + +def _create_float_table(catalog: Catalog, name: str, partitioned: bool) -> Table: + catalog.create_namespace("default") + iceberg_schema = Schema( + NestedField(1, "id", IntegerType(), required=False), + NestedField(2, "value", FloatType(), required=False), + ) + partition_spec = ( + PartitionSpec(PartitionField(source_id=2, field_id=1000, transform=IdentityTransform(), name="value")) + if partitioned + else UNPARTITIONED_PARTITION_SPEC + ) + tbl = catalog.create_table(identifier=f"default.{name}_{catalog.name}", schema=iceberg_schema, partition_spec=partition_spec) + # 0.1 and 0.7 are not exact in float32, and they end up as the bounds of the data file (or as partition values) + tbl.append(pa.Table.from_pydict({"id": [1, 2, 3], "value": [0.1, 0.4, 0.7]}, schema=schema_to_pyarrow(iceberg_schema))) + return tbl + + +@pytest.mark.parametrize("partitioned", [False, True]) +def test_scan_float_column_by_value(catalog: Catalog, partitioned: bool) -> None: + tbl = _create_float_table(catalog, "scan_float_column", partitioned) + + assert tbl.scan(row_filter="value = 0.1").to_arrow()["id"].to_pylist() == [1] + assert tbl.scan(row_filter="value <= 0.1").to_arrow()["id"].to_pylist() == [1] + assert tbl.scan(row_filter="value >= 0.7").to_arrow()["id"].to_pylist() == [3] + assert sorted(tbl.scan(row_filter="value in (0.1, 0.7)").to_arrow()["id"].to_pylist()) == [1, 3] + + +@pytest.mark.parametrize("partitioned", [False, True]) +@pytest.mark.parametrize("delete_filter, remaining", [("value != 0.1", [1]), ("value = 0.1", [2, 3])]) +def test_delete_float_column_by_value(catalog: Catalog, partitioned: bool, delete_filter: str, remaining: list[int]) -> None: + tbl = _create_float_table(catalog, "delete_float_column", partitioned) + + tbl.delete(delete_filter) + + assert sorted(tbl.scan().to_arrow()["id"].to_pylist()) == remaining diff --git a/tests/expressions/test_literals.py b/tests/expressions/test_literals.py index 9251e79a7d..f5a0089c68 100644 --- a/tests/expressions/test_literals.py +++ b/tests/expressions/test_literals.py @@ -255,7 +255,7 @@ def test_double_to_float_within_bound() -> None: lit = literal(34.56).to(DoubleType()) float_lit = lit.to(FloatType()) - assert lit.value == float_lit.value + assert float_lit.value == 34.560001373291016 def test_double_to_float_outside_bound() -> None: diff --git a/tests/expressions/test_visitors.py b/tests/expressions/test_visitors.py index 2bc77d50e9..d51fd5dbfd 100644 --- a/tests/expressions/test_visitors.py +++ b/tests/expressions/test_visitors.py @@ -1901,6 +1901,27 @@ def test_translate_column_names_missing_column_with_initial_default_mismatch() - assert translated_expr == AlwaysFalse() +def test_translate_column_names_missing_float_column_with_initial_default() -> None: + """Test translate_column_names when a missing float column's initial_default is not exact in float32.""" + original_schema = Schema( + NestedField(field_id=2, name="missing_col", field_type=FloatType(), required=False, initial_default=0.1), + schema_id=1, + ) + + unbound_expr = EqualTo("missing_col", 0.1) + bound_expr = visit(unbound_expr, visitor=BindVisitor(schema=original_schema, case_sensitive=True)) + + file_schema = Schema( + NestedField(field_id=1, name="other_col", field_type=StringType(), required=False), + schema_id=1, + ) + + translated_expr = translate_column_names(bound_expr, file_schema, case_sensitive=True) + + # Both the literal and the initial_default are rounded to float32, so they match + assert translated_expr == AlwaysTrue() + + def test_translate_column_names_missing_column_with_projected_field_matches() -> None: """Test translate_column_names with projected field value that matches expression.""" # Original schema with a field that has no initial_default (defaults to None) From 09a4e869ed56dbd3d7b757c64b5e35b3be32e197 Mon Sep 17 00:00:00 2001 From: hzmsh <35788891+hzmsh@users.noreply.github.com> Date: Wed, 14 Oct 2026 18:52:58 -0500 Subject: [PATCH 2/2] test: cover float32 defaults read from JSON Float defaults are now stored as float32 values, like Java, so the REST round-trip test's float default of 22.27 read back as 22.270000457763672. Use 22.25, which is exact in float32, and add a unit test for reading a float default from JSON. Generated-by: Claude Code (Claude Opus 5.5) Co-Authored-By: Claude Opus 5.5 --- tests/integration/test_rest_schema.py | 2 +- tests/test_conversions.py | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/integration/test_rest_schema.py b/tests/integration/test_rest_schema.py index 4462da1c8c..d50899af6b 100644 --- a/tests/integration/test_rest_schema.py +++ b/tests/integration/test_rest_schema.py @@ -1100,7 +1100,7 @@ def test_add_required_column(catalog: Catalog) -> None: (BooleanType(), True, False), (IntegerType(), 123, 456), (LongType(), 123, 456), - (FloatType(), 19.25, 22.27), + (FloatType(), 19.25, 22.25), # exact in float32, so it reads back unchanged (DoubleType(), 19.25, 22.27), (DecimalType(10, 2), Decimal("19.25"), Decimal("22.27")), (DecimalType(10, 2), Decimal("19.25"), Decimal("22.27")), diff --git a/tests/test_conversions.py b/tests/test_conversions.py index e786ae0683..afa73ac9a7 100644 --- a/tests/test_conversions.py +++ b/tests/test_conversions.py @@ -609,6 +609,11 @@ def test_json_serialize_roundtrip(primitive_type: PrimitiveType, value: Any) -> assert value == conversions.from_json(primitive_type, conversions.to_json(primitive_type, value)) +def test_json_float_is_read_as_float32() -> None: + assert conversions.from_json(FloatType(), 22.27) == 22.270000457763672 + assert conversions.from_json(DoubleType(), 22.27) == 22.27 + + def test_unknown_type_conversions() -> None: """Unknown values are always null, so they have no binary or partition representation.""" unknown = UnknownType()