andygrove opened a new issue, #6590: URL: https://github.com/apache/datafusion-comet/issues/6590
### What is the problem the feature request solves? Since #6447, every native comparison of `FLOAT` or `DOUBLE` operands (`=`, `<>`, `<=>`, `<`, `<=`, `>`, `>=` and `IS DISTINCT FROM`) follows Spark's SQL ordering, in which `-0.0` equals `0.0`, all NaNs are equal and NaN sorts above every other value. `spark_comparison` gets there by normalizing each operand that is not a literal into a copy, through `NormalizeNaNAndZero` for a scalar and `NormalizeNestedFloats` for an array or struct, and then running Arrow's comparison kernels, which use IEEE 754 total order. That makes a comparison about 50% slower than DataFusion's. From `benches/float_comparison.rs` on an M3 Max, per batch of 8192 doubles: | Comparison | DataFusion `BinaryExpr` | `spark_comparison` | | --- | --- | --- | | column `<` column | 6.8 µs | 10.4 µs | | column `<` literal | 3.8 µs | 5.6 µs | Comparisons in Project and Filter already paid for a copy before #6447, through the `CometExecRule.normalize` rewrite. Elsewhere (aggregate arguments, `FILTER` clauses, join conditions, sort keys and generators) the cost is new. For arrays and structs, `<=>`, `<`, `<=`, `>` and `>=` deep-copy both operands in every batch through `normalize_nested_floats`, which costs more the wider the nested columns are, while nested `=` and `<>` already compare in place with `spark_equality`. Skipping the copy when an operand holds no NaN or `-0.0` does not pay. On 8192 doubles without either, the copy takes 1.71 µs, a branch-free check in blocks of 64 values takes 1.66 µs, the same check on the raw bits 1.28 µs, and an `any()` that stops at the first match 4.4 µs, because it does not vectorize. ### Describe the potential solution Compare in Spark's order directly, without normalized copies: - Scalars: a kernel over the two value buffers, or a buffer and a scalar, that evaluates each operator with Spark's semantics in the style of `float_semantics::float_gt` (`l > r || (l.is_nan() && !r.is_nan())`), which vectorizes in loops over slices, and combines the null buffers the way Arrow's `cmp` kernels do. `<=>` and `IS DISTINCT FROM` need the null-aware forms. - Arrays and structs: evaluate the ordering operators with `spark_comparator`, which already orders nested floats the Spark way in place and backs `spark_equality`, as `NestedPredicate` does for `=` and `<>`. Planning should keep its current behavior: a literal is normalized while the plan is built, and `FloatOperands::Raw` leaves a float column compared with a literal unwrapped in a scan's data filters, so that Parquet pruning still recognizes it. The `float_comparison` bench measures the gap. `float_comparisons.sql` and `CometFloatSemanticsSuite` cover the semantics across operators, and the `spark_comparison` unit tests check every pair of edge values against `compare_floats`. ### Additional context Raised in the #6447 review: a [read-only fast path for `normalize_floats`](https://github.com/apache/datafusion-comet/pull/6447#discussion_r4146865166) and the [per-batch copies for nested ordering](https://github.com/apache/datafusion-comet/pull/6447#issuecomment-5919313999). Part of #6385. -- 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]
