breken-ai opened a new pull request, #4029:
URL: https://github.com/apache/iceberg-python/pull/4029
<!--
Thanks for opening a pull request!
-->
<!-- In the case this PR will resolve an issue, please replace
${GITHUB_ISSUE_ID} below with the actual Github issue id. -->
<!-- Closes #${GITHUB_ISSUE_ID} -->
# Rationale for this change
The expression parser reads an unquoted number with a decimal point as a
`DecimalLiteral`. When that literal is bound to an `int` or `long` column,
`DecimalLiteral.to(IntegerType/LongType)` rounds it with `to_integral_value()`
(half-even), which changes the predicate and makes a scan return wrong rows:
```python
tbl.append(pa.table({"x": pa.array([1, 2, 3, 4], pa.int32())}))
tbl.scan(row_filter="x > 2.6").to_arrow()["x"] # [4] expected [3, 4]
(bound as x > 3)
tbl.scan(row_filter="x < 2.5").to_arrow()["x"] # [1] expected [1, 2]
(bound as x < 2)
tbl.scan(row_filter="x = 2.5").to_arrow()["x"] # [2] expected []
(bound as x = 2)
tbl.scan(row_filter=GreaterThan("x", Decimal("2.6"))) # same as x > 3
```
The same rewritten predicate is used for partition and metrics pruning, so
this also affects `delete()`/`overwrite()` with such a filter.
This PR makes the conversion raise a `ValueError` (`Could not convert 2.6
into a int, value has a fractional part`) when the decimal has a fractional
part, the same way `DecimalLiteral.to(DecimalType)` already rejects a
mismatched scale, and `partition_to_py` rejects fractional digits for integer
partitions. Integral decimals such as `2.00` still convert, and out-of-range
values still become `IntAboveMax`/`IntBelowMin`. Java has no decimal-to-integer
literal conversion at all, so binding fails there too.
`StringLiteral.to(IntegerType)` truncates quoted values the same way (`x <
'2.5'` binds as `x < 2`), but `test_string_literal` asserts
`literal("3.141").to(IntegerType()) == literal(3)`, so I left that path alone.
Happy to follow up if you'd like it changed too.
## Are these changes tested?
Yes.
- `tests/expressions/test_literals.py`:
`test_fractional_decimal_to_integral_type_raises` (2.5, 2.6, -2.5, 0.1 for int
and long) and `test_integral_decimal_to_integral_type`.
- `tests/catalog/test_catalog_behaviors.py`:
`test_scan_integer_column_with_decimal_literal` appends to a real table (memory
and SQL catalogs) and checks that `x > 2.0` returns `[3, 4]` and `x > 2.6`
raises instead of returning `[4]`.
On `main` the 11 new fractional cases fail with `DID NOT RAISE ValueError`.
With the fix, all 37 selected tests pass. `tests/expressions`,
`tests/test_conversions.py`, `tests/catalog/test_catalog_behaviors.py`,
`tests/catalog/test_sql.py`, `tests/io/test_pyarrow.py` and
`tests/io/test_pyarrow_visitor.py` pass. `prek run --files` (ruff, ruff-format,
mypy, pydocstyle, codespell) passes.
## Are there any user-facing changes?
Yes. A filter that compares an integer column with a fractional number now
raises a `ValueError` instead of silently returning wrong rows. Filters with
integral numbers are unchanged.
AI disclosure: this bug was found, fixed and tested by an AI coding agent
(Claude) running under the breken-ai account; the red/green runs above are its
local results.
<!-- 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]
