You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "breken-ai (via GitHub)" <gi...@apache.org> on 2026/09/30 03:47:55 UTC
[PR] fix(expressions): reject fractional decimal literals on integer columns [iceberg-python]
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] fix(expressions): reject fractional decimal literals on integer columns [iceberg-python]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu merged PR #4029:
URL: https://github.com/apache/iceberg-python/pull/4029
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] fix(expressions): reject fractional decimal literals on integer columns [iceberg-python]
Posted by "breken-ai (via GitHub)" <gi...@apache.org>.
breken-ai commented on PR #4029:
URL: https://github.com/apache/iceberg-python/pull/4029#issuecomment-6049475706
@Fokko Fixed. I rebased onto `main` at 068aae5. The only conflict was in `tests/catalog/test_catalog_behaviors.py`, where `test_append_nan_to_identity_partitioned_table` landed in the same spot. I kept both tests. The head is now c4489d7. It also adds the `2.0` cases @rambleraptor asked for (see the inline thread). `pytest tests/expressions tests/catalog/test_catalog_behaviors.py`: 1307 passed. ruff check and format are clean.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] fix(expressions): reject fractional decimal literals on integer columns [iceberg-python]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on PR #4029:
URL: https://github.com/apache/iceberg-python/pull/4029#issuecomment-5939664008
@breken-ai Can you fix the merge conflict?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] fix(expressions): reject fractional decimal literals on integer columns [iceberg-python]
Posted by "rambleraptor (via GitHub)" <gi...@apache.org>.
rambleraptor commented on code in PR #4029:
URL: https://github.com/apache/iceberg-python/pull/4029#discussion_r4160144642
##########
tests/catalog/test_catalog_behaviors.py:
##########
@@ -1322,6 +1322,18 @@ def test_append_invalid_input_type_raises(catalog: Catalog) -> None:
tbl.append("not an arrow object")
+def test_scan_integer_column_with_decimal_literal(catalog: Catalog) -> None:
Review Comment:
Could you also add a test for 2.0? That should also pass
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] fix(expressions): reject fractional decimal literals on integer columns [iceberg-python]
Posted by "breken-ai (via GitHub)" <gi...@apache.org>.
breken-ai commented on code in PR #4029:
URL: https://github.com/apache/iceberg-python/pull/4029#discussion_r4213330838
##########
tests/catalog/test_catalog_behaviors.py:
##########
@@ -1322,6 +1322,18 @@ def test_append_invalid_input_type_raises(catalog: Catalog) -> None:
tbl.append("not an arrow object")
+def test_scan_integer_column_with_decimal_literal(catalog: Catalog) -> None:
Review Comment:
Added in c4489d7. `2.0` (and `2`, `2.00`, `-2.0`) has no fractional part, so it still converts and filters exactly:
- `tests/expressions/test_literals.py::test_integral_decimal_to_integral_type` is now parametrized over `"2"`, `"2.0"`, `"2.00"`, `"-2.0"` for both `IntegerType` and `LongType`, and checks each one becomes `LongLiteral(int(value))`.
- `tests/catalog/test_catalog_behaviors.py::test_scan_integer_column_with_decimal_literal` checks `x > 2.0` returns `[3, 4]` and now also `x = 2.0` returns `[2]`, on the memory, sql and sql_without_rowcount catalogs.
The 2.0 cases pass on both `main` and this branch, as you expected. The fractional cases (`2.5`, `2.6`, `-2.5`, `0.1`, and the `x > 2.6` scan) fail on `main` and pass here: 11 failed / 8 passed with `main`'s `literals.py`, and 19 passed with the fix. `pytest tests/expressions tests/catalog/test_catalog_behaviors.py`: 1307 passed. `ruff check` and `ruff format --check` are clean.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org