sunchao commented on PR #5469: URL: https://github.com/apache/datafusion-comet/pull/5469#issuecomment-5442862621
Updated in [38825e26a](https://github.com/apache/datafusion-comet/commit/38825e26a1324297a46821a06fe5ce33e8b47779). The follow-up adds the sort benchmark and links the two remaining compatibility issues from the guide. I checked the six points separately: 1. **Sort cost:** [the new benchmark](https://github.com/apache/datafusion-comet/blob/38825e26a1324297a46821a06fe5ce33e8b47779/native/spark-expr/benches/sort_float_keys.rs) runs DataFusion 54.1.0 `SortExec` with bare versus normalized keys in the same release binary, using 262,144 rows, 32 batches, one partition/thread, and both finite/non-null and null/NaN/signed-zero inputs. Two passes reverse the variant order. FLOAT median changes were small (−1.7% to +4.7%, with mixed-input direction varying); DOUBLE cost was measurable in both passes: +6.6–11.4% finite and +5.2–12.5% mixed. The description contains both passes and reproduction commands. This is a SortExec benchmark, not an end-to-end Spark result. Peak pool reservations were unchanged, but they exclude temporary normalized arrays: each evaluation adds a 4/8-byte-per-row values buffer and shares validity. Already-normalized keys are reused, so this measures previously bare keys rather than another copy at every call. Fo lding normalization into sorting/encoding is a worthwhile optimization to investigate separately; this change does not claim the cost is free. 2. **Strict-mode admission:** tracked in [#5506](https://github.com/apache/datafusion-comet/issues/5506), now linked from the guide. The follow-up is scoped to scalar Sort admission with strict-mode execution tests; this PR retains the existing guards for other consumers and nested keys. 3. **Nested ordering/rank:** tracked in [#5507](https://github.com/apache/datafusion-comet/issues/5507), also linked from the guide. Diagnostic SQL reproduced array/struct ORDER BY and array RANK differences for FLOAT and DOUBLE with native operators asserted. Struct RANK already falls back in those probes; it is not a reproduced native wrong result. Strict-mode controls matched Spark. The issue records the reused-native-build limitation, so these are evidence for the remaining limitation, not a fresh full-head validation claim. 4. **Sort-merge-join options:** the observation is correct, but that site constructs and discards a planning-time expression. It does not evaluate the normalizer or copy a column there; it already built an expression just to obtain options before this PR. I left the optional helper cleanup out of this correctness fix. 5. **`normalize_array` coverage:** the existing doc comment specifies scalar floating comparison keys and says other types are unchanged. Its Float32/Float64 match and `create_normalized_key_expr` use the same gate: dictionary, list, and struct arrays deliberately pass through. There is no recursive or dictionary-normalization promise in that API; the nested limitation now has the explicit tracker above. 6. **Explicit `strictFloatingPoint=false` in tests:** it pins the native execution path under test, independently of externally supplied configuration or a future default change. The tests assert native Sort/WindowGroupLimit operators; `true` deliberately still invokes the conservative fallback tracked by #5506. I kept the setting for that reason. The new release benchmark built, all eight smoke cases passed, and benchmark Clippy (`-D warnings`), formatting, and diff checks passed. I also corrected the stale validation paragraph: the earlier full-suite results are historical, not rerun for this benchmark/documentation-only follow-up. -- 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]
