sankalpsthakur opened a new pull request, #54915:
URL: https://github.com/apache/arrow/pull/54915
### Rationale for this change
`is_in` and `index_in` look floating-point values up through their unsigned
integer bit patterns, so `-0.0` and `+0.0` are different keys even though
`equal` treats them as the same value:
```python
>>> pc.equal(pa.array([-0.0]), pa.array([0.0]))
[true]
>>> pc.is_in(pa.array([-0.0]), value_set=pa.array([0.0]))
[false]
```
This silently drops rows from dataset scans. The Parquet format asks writers
to put `-0.0` in the min and `+0.0` in the max statistic when the computed
value is zero, so a column that only holds zeros gets min=-0.0, max=+0.0.
`EvaluateStatisticsAsExpression` turns that into the guarantee `x == -0.0`,
`SimplifyWithGuarantee` substitutes `x` by `-0.0` and constant-folds `x IN
(0.0)` through the `is_in` kernel, which says false, so the whole fragment is
skipped:
```python
pq.write_table(pa.table({"x": [0.0, 0.0]}), "zeros.parquet")
ds.dataset("zeros.parquet").to_table(filter=ds.field("x").isin([0.0])) #
empty, expected both rows
```
The reporter's analysis in the issue traced the empty result to the signed
zero in the Parquet statistics.
### What changes are included in this PR?
`SetLookupState` in `scalar_set_lookup.cc` folds the bit pattern of `-0.0`
into `+0.0`, both when the value set is built and when values are probed, if
the value set is floating-point. Integers of the same width are not touched
(their smallest value has the same bit pattern as `-0.0`).
I did not change `ScalarHelper<float/double>::ComputeHash` in `hashing.h`,
which is where the issue discussion pointed. These kernels do not use it:
`InitStateVisitor` maps every fixed-width numeric type to an unsigned integer
state, so it is `ScalarMemoTable<uint64_t>` that hashes doubles here. And
`ScalarMemoTable` is also the table behind `unique`, `value_counts`,
`dictionary_encode`, `DictionaryBuilder` and the Parquet dictionary encoder,
where merging `-0.0` into `0.0` would silently change stored values (a
dictionary-encoded Parquet column keeps the sign of zero across a round trip
today; see also ARROW-10211). So this only makes set membership agree with
`equal`. NaN handling, the other hash-based kernels and the Acero hash join /
group_by (which still keep the two zeros apart) are unchanged.
### Are these changes tested?
Yes: C++ tests for `is_in` and `index_in` on float and double (single,
chunked and duplicated value sets, and integer guards) in
`scalar_set_lookup_test.cc`, `Expression.SimplifyIsInSignedZero` for the
dataset guarantee path, and two pytests (`test_is_in_index_in_signed_zero`,
`test_filter_isin_signed_zero`). They fail without the change.
### Are there any user-facing changes?
`is_in` and `index_in` on float and double now match `-0.0` with `+0.0` and
the reverse, as `equal` does.
**This PR contains a "Critical Fix".** It fixes (b), a bug that caused
incorrect data to be produced: dataset filters using `isin` on a float column
silently returned no rows for fragments whose values are all zero.
### Was AI used for this PR?
**PR code and description written by:**
- [ ] Human
- [x] AI
**Reviewed before submission by:**
- [ ] Human
- [x] AI
- [ ] Not reviewed
AI disclosure: Claude Code wrote the fix, the tests and this description;
the tests were run with and without the change before submitting.
--
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]
