hzmsh opened a new pull request, #4108:
URL: https://github.com/apache/iceberg-python/pull/4108
# Rationale for this change
Filters on `float` columns can miss rows, and `delete()` can delete the
wrong rows:
```python
import pyarrow as pa
from pyiceberg.catalog.memory import InMemoryCatalog
from pyiceberg.schema import Schema
from pyiceberg.types import FloatType, IntegerType, NestedField
catalog = InMemoryCatalog("default")
catalog.create_namespace("default")
schema = Schema(NestedField(1, "id", IntegerType()), NestedField(2, "value",
FloatType()))
tbl = catalog.create_table("default.t", schema=schema)
tbl.append(pa.table({"id": pa.array([1, 2, 3], pa.int32()), "value":
pa.array([0.1, 0.4, 0.7], pa.float32())}))
print(tbl.scan(row_filter="value = 0.1").to_arrow()["id"].to_pylist()) #
main: [], expected: [1]
tbl.delete("value != 0.1")
print(tbl.scan().to_arrow()["id"].to_pylist()) # main: [], expected: [1]
```
**Why.** A `float` column stores 32-bit values, and `0.1` as a 32-bit float
is `0.10000000149011612`. PyIceberg keeps the filter value as the 64-bit `0.1`.
To decide which files to read, it compares the filter value with each file's
min and max (or with the partition value). These two numbers are not equal, so
the file with `0.1` is skipped. Reading rows inside a file is fine, because
Arrow converts the filter value to 32 bits. So the bug shows up when the value
is a file's min or max, or a partition value.
Deletes use the same min/max check. For `delete("value != 0.1")`, PyIceberg
decides that no row in the file is `0.1` and drops the whole file.
Data `[0.1, 0.4, 0.7]` in one file:
| | main | this PR |
|---|---|---|
| `value = 0.1` | `[]` | `[1]` |
| `value <= 0.1` | `[]` | `[1]` |
| `value >= 0.7` | `[]` | `[3]` |
| `delete("value != 0.1")`, then scan | `[]` | `[1]` |
| `delete("value = 0.1")`, then scan | `[1, 2, 3]` | `[2, 3]` |
**Fix.** Convert the value to 32 bits when the float literal is created, in
`FloatLiteral.__init__`. Java does the same: its float literal holds a `Float`
([Literals.java#L380](https://github.com/apache/iceberg/blob/1e39a61f262730ee9ec23d991d9fe34dfe1737df/api/src/main/java/org/apache/iceberg/expressions/Literals.java#L380)).
This also removes `_value32`, which is now the same as `.value`.
A float `initial-default` read from table metadata is converted too, like
Java
([SingleValueParser.java#L79](https://github.com/apache/iceberg/blob/1e39a61f262730ee9ec23d991d9fe34dfe1737df/core/src/main/java/org/apache/iceberg/SingleValueParser.java#L79)).
Otherwise a filter on a column that is missing from older files would stop
matching its default.
Reproduced on 0.12.0 and `main`.
## Are these changes tested?
Yes.
- `tests/catalog/test_catalog_behaviors.py`: new scan and delete tests on a
float column, with and without an identity partition, for each catalog fixture.
All 18 fail on `main` and pass with this change.
- `tests/expressions/test_literals.py::test_double_to_float_within_bound`
expected the old 64-bit value. It now expects the 32-bit value of 34.56
(`34.560001373291016`).
- `tests/expressions/test_visitors.py`: new test for a missing float column
with `initial_default=0.1`. It fails if only the filter value is converted.
- Unit tests on Linux (Python 3.11, with `pyiceberg-core`): 4123 passed,
4104 on `main`. The difference is the 19 tests above that fail on `main`. 9
other failures are the same on both, from optional packages I don't have
(Kerberos, Azure identity). On Windows the difference is the same 19 tests. My
setup has no Cython Avro decoder, so `tests/avro` was not collected, and I
didn't run the integration tests.
- `prek run --files` on the changed files passes.
## Are there any user-facing changes?
Yes. Filters and deletes on `float` columns now return and remove the right
rows. `double` columns are not affected.
Small visible changes: `FloatLiteral(0.1).value` (and its `repr`) is now
`0.10000000149011612`, and a float `initial-default` of `0.1` is read the same
way. On `main`, `DoubleLiteral(0.1) == FloatLiteral(0.1)` was `True` and the
other way round was `False`; now both are `False`, like Java.
---
Written with AI assistance (Claude Code, Claude Opus 5.5). The Java
references were read from `apache/iceberg` `main`, and the tests above were run
on this branch.
<!-- In the case of user-facing changes, please add the changelog label. -->
--
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]
