PDGGK opened a new pull request, #4274:
URL: https://github.com/apache/iceberg-python/pull/4274
<!--
Thanks for opening a pull request!
-->
# Rationale for this change
Appending to a table with an identity partition on a `double` or `float`
column writes some rows twice when the data holds both `-0.0` and `0.0`. Signed
zeros are easy to get, for example by rounding small negative values:
```python
import pyarrow as pa
import pyarrow.compute as pc
df = pa.table({"id": [1, 2, 3], "reading": pc.round(pa.array([-0.04, 0.04,
1.23]), 1)})
# reading: [-0.0, 0.0, 1.2]
tbl = catalog.create_table("default.readings", schema=df.schema)
with tbl.update_spec() as update:
update.add_identity("reading")
tbl.append(df)
```
On `main` (and on 0.12.0), the three rows come back as five:
```
rows appended: 3
rows scanned: 5
ids scanned: [1, 1, 2, 2, 3]
file {'reading': -0.0} record_count 2
file {'reading': 0.0} record_count 2
file {'reading': 1.2} record_count 1
```
With this change:
```
rows appended: 3
rows scanned: 3
ids scanned: [1, 2, 3]
file {'reading': -0.0} record_count 1
file {'reading': 0.0} record_count 1
file {'reading': 1.2} record_count 1
```
`_determine_partitions` groups the rows by partition value, then selects
each group's rows with `pc.field(name) == value`. `group_by` keeps `-0.0` and
`0.0` as two groups, but `-0.0 == 0.0` is true, so each of the two filters
selects the rows of both groups, and those rows are written into both data
files. #4030 fixed the NaN case of this filter, where NaN never equals itself
and its partition came out empty. This PR handles the opposite case: for a zero
partition value, the filter also matches the sign of zero, using `1 / x`, which
is `-inf` for `-0.0` and `inf` for `0.0`.
The partitioning itself is unchanged. `-0.0` and `0.0` are still two
partitions, as before. Each row now lands in exactly one of them.
`overwrite()` and `dynamic_partition_overwrite()` write through the same
function and had the same problem (3 rows in, 5 read back). Both return 3 rows
with this change.
## Are these changes tested?
Yes.
- `tests/io/test_pyarrow.py::test_determine_partitions_identity_signed_zero`
checks, for both `double` and `float`, that the `-0.0`, `0.0` and `1.0` rows
each land in exactly one partition. On `main`, both zero partitions hold rows
`[1, 2, 4]`.
-
`tests/catalog/test_catalog_behaviors.py::test_append_signed_zero_to_identity_partitioned_table`
appends to a real table (memory and SQL catalogs). It checks that a full scan
returns each row once and that `value = 0` returns both zero rows. On `main`,
the scan returns `[1, 1, 2, 2, 3]`.
All 4 new test cases fail on `main` and pass with the fix.
What I ran locally:
- the whole of `tests/io/test_pyarrow.py` and
`tests/catalog/test_catalog_behaviors.py`: 931 passed, 3 skipped;
- the partition, append and overwrite tests in
`tests/table/test_partitioning.py` and `tests/catalog/test_sql.py`: 31 passed;
- ruff, ruff-format, mypy and codespell on the changed files.
I also checked the new filter expression on pyarrow 18.0.0, the minimum
supported version. CI covers the rest of the suite and the integration tests.
## Are there any user-facing changes?
Yes, a bug fix. Appending, overwriting or dynamically overwriting data that
holds both `-0.0` and `0.0` into an identity-partitioned `float` or `double`
column no longer writes those rows twice. Rows that were already duplicated in
existing tables stay as they are.
<!-- 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]
