peterxcli commented on PR #5194: URL: https://github.com/apache/datafusion-comet/pull/5194#issuecomment-5442063886
Thanks for the follow-up pass! Addressed in 390fc9125. **On #5403 / `spark_comparator`:** agreed that's the right endgame for #5191 — it's exactly the comparator these paths need. Since #5403 hasn't merged yet, I'd rather not couple the two PRs: I'll keep the `Incompatible` marking here and, once #5403 lands, follow up in #5191 by lifting `spark_comparator` into a shared module, swapping it into `arrays_overlap`/`array_position`, and dropping the `Incompatible` markings. On the interim cost: only arrays with nested *float* elements route away from the native path — that's exactly the path affected by the correctness bug — and `spark.comet.expression.allowIncompatible` opts back in for users who accept the difference. I'll note this plan on #5191. **On map elements in `hasNestedFloatElements`:** confirmed unreachable. Both `ArraysOverlap.checkInputDataTypes` and `ArrayPosition.checkInputDataTypes` call `TypeUtils.checkForOrderingExpr` on the element type, and `MapType` is not orderable, so the analyzer rejects `array<map<...>>` inputs before planning. Added a comment saying so. **On the docs wording:** good catch — updated both rows in `expressions.md` to say the nested-float case "falls back to Spark by default, and the incompatible native path is opt-in via allowIncompatible". I deliberately didn't copy the `array_intersect`/`array_join` phrasing: those route through the JVM codegen dispatcher (`CodegenDispatchFallback`), whereas these two serdes fall back to Spark entirely, so "codegen dispatcher" would be inaccurate here. -- 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]
