apache / apache/datafusion-comet

Narrow strict floating-point admission for corrected scalar sort keys

Open
#5,506 1 comment 0 reactions 1 assignee Claimed by @0lai0 View on GitHub
area:expressions enhancement
Dominant language
Scala
Stars
1.3k
Forks
373
Avg merge
2d 4h
Merged PRs (30d)
198

Description

### What is the problem the feature request solves?

After #5469, native Sort, Window, WindowGroupLimit, and range-partition comparison keys normalize scalar `FLOAT` and `DOUBLE` NaNs and signed zeros consistently with Spark. Admission is still more conservative: `CometSortOrder.getSupportLevel` calls the shared `strictFloatingPointReason` helper, which rejects both scalar and nested floating types when `spark.comet.exec.strictFloatingPoint=true`.

Strict-mode users therefore still lose native scalar floating sorts even though #5469 fixes their comparison semantics. This follow-up should evaluate and test narrowing that specific admission decision, without removing the remaining nested-type protections or changing other callers of the shared helper.

### Describe the potential solution

Use a Spark session with Comet enabled and `CometShuffleManager` configured. Create a Parquet input so constant folding cannot remove the sort:

```sql
CREATE TABLE comet_scalar_sort_case (id INT, v DOUBLE) USING parquet;
INSERT INTO comet_scalar_sort_case VALUES
(1, CAST('-0.0' AS DOUBLE)), (2, CAST('0.0' AS DOUBLE)), (3, 1.0);

SET spark.sql.adaptive.enabled=false;
SET spark.sql.shuffle.partitions=1;
SET spark.comet.exec.strictFloatingPoint=true;
EXPLAIN FORMATTED
SELECT id, v FROM comet_scalar_sort_case ORDER BY v, id DESC;

SET spark.comet.exec.strictFloatingPoint=false;
EXPLAIN FORMATTED
SELECT id, v FROM comet_scalar_sort_case ORDER BY v, id DESC;
```

Require the strict-mode plan to use native `CometSortExec` only after its compatibility is established; the current strict-mode plan falls back. Check results against Comet disabled, not only against the non-strict native path. The zero peers must be ordered by the secondary key (`id` 2 before 1), and returned zero signs must remain unchanged.

The acceptance tests should cover `FLOAT` and `DOUBLE`, both sort directions and null orders, compound keys, signed zeros, and independently constructed positive/negative NaN payloads. Cover ordinary window ordering and `RANK`/`DENSE_RANK` cutoffs with actual native operator assertions. Retain strict-mode fallback controls for arrays/structs containing floats, and test all supported Spark versions before narrowing the policy.

### Additional context

The current [SortOrder gate](https://github.com/apache/datafusion-comet/blob/96eafdfe14384976cb18b7364c4cf84d7d1fb723/spark/src/main/scala/org/apache/comet/serde/CometSortOrder.scala#L33-L39) delegates to a [recursive shared policy](https://github.com/apache/datafusion-comet/blob/96eafdfe14384976cb18b7364c4cf84d7d1fb723/spark/src/main/scala/org/apache/comet/serde/SupportLevel.scala#L87-L104). The native [scalar key normalization](https://github.com/apache/datafusion-comet/blob/96eafdfe14384976cb18b7364c4cf84d7d1fb723/native/core/src/execution/planner.rs#L927-L963) is intentionally narrower. Keep this admission follow-up separate from #5468's scalar wrong-result fix and from the nested ordering/rank limitation. #2626 is closed and does not track this remaining admission work.

A fresh Spark 4.1.3 planning probe confirmed native scalar sorts with strict mode disabled and Spark sorts with it enabled for both FLOAT and DOUBLE. The reused JVM build's `CometSortOrder`, `SupportLevel`, and `CometWindowExec` sources are byte-identical to the reviewed #5469 head. This checks admission; it does not claim a new full-binary runtime validation of the scalar normalization fix.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.