alamb opened a new pull request, #27181: URL: https://github.com/apache/datafusion/pull/27181
## Which issue does this PR close? - Closes #27121. - Alternative to #27122 (thank you @jonasHanhan for the report, the analysis and the test case, which this PR builds on). ## Rationale for this change `x IN (0.0, 5.0)` returns `false` for `x = -0.0`, while `x = 0.0` returns `true`. #27122 fixes this in the expression simplifier by not keeping short float lists as membership filters when they contain a zero literal. That only covers lists of up to three literals on a bare column. The same inconsistency remains for: - lists with four or more literals, e.g. `x IN (0.0, 1.0, 2.0, 3.0)` - non-column expressions on the left, e.g. `-x IN (0.0, -5.0)` - a column in the list, e.g. `x IN (y, 7.0, 8.0, 9.0)` - `InListExpr` built directly (`try_new_from_array`) or decoded from protobuf The root cause is in the physical `InListExpr`: its static filters (branchless, bitmap, hash set) and the direct call to Arrow's `eq` compare floats by bit pattern, while `=` goes through `apply_cmp`, which normalizes `-0.0` to `+0.0` first using `datafusion_common::float_semantics` (added in #26595). This PR applies the same normalization in `InListExpr`, so `IN` agrees with `=` on every path, and the short-list membership fast path is kept for lists that contain zero. ## What changes are included in this PR? - `instantiate_static_filter` runs the constant list through `normalize_floats` before building any static filter (covers both `try_new` and `try_new_from_array`). - `InListExpr::evaluate` normalizes the input value and any runtime-evaluated list items. Plain float scalars use `ScalarValue::normalize`; dictionary or nested float scalars go through the array form. - Doc comments on the branchless filter and the `OrderedFloat` wrappers note that signed zeros are normalized before reaching them. The hot-path cost is one read-only scan of the input values per batch (allocation-free when nothing needs normalizing), which is the same scan `=` already pays. ## What is the testing strategy for this PR? - `signed_zeros` sqllogictest in `in_list.slt` covers the comparison chain, branchless, bitmap (Float16) and hash set filters, runtime comparison with a column in the list, dictionary input and constant folding, for both `IN` and `NOT IN`. Every column except `x = 0.0` fails without the fix. - `simplify_inlist_signed_zero` in the optimizer now evaluates long lists as well as short ones, instead of only asserting the long form is left unchanged (which hid the remaining inconsistency). - `in_list_float_signed_zero` in physical-expr covers the branchless, hash set, runtime column and `try_new_from_array` paths directly. ## Are there any user-facing changes? `x IN (...)` and `x NOT IN (...)` on `Float16`, `Float32` and `Float64` now treat `-0.0` and `+0.0` as equal, consistent with `=`, `GROUP BY` and joins. No API changes. -- 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]
