sunchao commented on PR #5403: URL: https://github.com/apache/datafusion-comet/pull/5403#issuecomment-5442466423
Updated in [9cd863dfa](https://github.com/apache/datafusion-comet/commit/9cd863dfaeaf27fe79a8710ac1e05ac26867a517). I checked the proposed Arrow-first fast path against the pinned Arrow/DataFusion implementation. It is not safe to trigger the corrective scan only when Arrow returns zero or NaN: for `array_max([-NaN, 1.0])`, Arrow returns `1.0`, while Spark returns NaN. The mismatch reproduces for FLOAT and DOUBLE with list lengths 2, 32, and 1,024. I added that counterexample beside `float_extrema` so a future optimization does not reintroduce it. A safe vectorized path would also have to establish that the input contains no exceptional values, and would need to be measured across null densities and lengths. I have not substituted an unmeasured two-pass algorithm here. The reported 1.5–1.6× latency applies to the measured long, null-free FLOAT microbenchmark, not every floating array or an end-to-end Spark query. That cost remains explicit in the description; this clarification does not claim a performance improvement. The `FixedSizeList` concern does not reach the fallback arm through the supported function signature: `invoke_with_args` first calls the delegated DataFusion `return_type`, which rejects an outer fixed-size list. Supporting fixed-size lists as nested elements is a different capability. The code now states that boundary rather than assuming that Comet can never construct such an Arrow array. Dictionary values are decoded once when the comparator is constructed for a child array/batch, and the comparator retains that decoded array for its index comparisons. This is not a decode per element comparison; I added that scope to the comment. I have not claimed the decode is free or that deep dictionary nesting was benchmarked. The earlier ignored-case requests are already implemented as native assertions, with separate collation-fallback checks. The final newline in `Cargo.toml` has no behavioral effect. This update only changes explanatory source comments: runtime code, tests, and the previously reported performance results are unchanged. `cargo fmt --all --check` and `git diff --check` pass; no new full Spark build is claimed. -- 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]
