sunchao commented on code in PR #6563:
URL: https://github.com/apache/datafusion-comet/pull/6563#discussion_r4190775999
##########
spark/src/main/scala/org/apache/comet/serde/arrays.scala:
##########
@@ -540,39 +540,73 @@ object CometSlice extends CometExpressionSerde[Slice] {
private[comet] object ArraySetSupport {
val floatingPointReason: String =
- "Floating-point elements match Spark's signed-zero and NaN semantics
natively only on " +
- "Spark 4.2.0, whose optimizer normalizes the arguments (SPARK-54918)"
-
- // The native kernels match Spark only when the plan has already normalized
the arguments, and
- // only Spark 4.2.0 does that (SPARK-54918). Earlier releases keep flat
signed zeros apart.
- // From 4.0.5, 4.1.4 and 4.2.1, SPARK-59602 normalizes during evaluation
instead, which the
- // native kernels do not match for NaN payloads or nested zeros. A top-level
- // KnownFloatingPointNormalized marker cannot replace the version check:
Spark also normalizes
- // CreateArray, If, CaseWhen, and Coalesce recursively without wrapping the
resulting array.
- def normalizesArgumentsInPlan(version: String): Boolean =
- Utils.majorMinorPatchVersion(version).contains((4, 2, 0))
+ "Floating-point elements match Spark's signed-zero semantics natively only
on Spark " +
+ "4.0.5+, 4.1.4+ and 4.2+, which treat -0.0 and 0.0 as one value in these
functions " +
+ "(SPARK-54918, SPARK-59602)"
+
+ val collationReason: String =
+ "Elements that hold both a floating-point value and a non-UTF8_BINARY
collated string fall " +
+ "back to Spark, which compares the strings under their collation, while
Comet's native " +
+ "kernels compare their raw bytes"
+
+ // Spark 4.2.0 normalizes the arguments of these functions in the plan
(SPARK-54918), and 4.0.5,
+ // 4.1.4 and 4.2.1 normalize while evaluating them (SPARK-59602). Either
way, Spark treats -0.0
+ // and 0.0, and every NaN, as one value at any depth, which the spark_
variants match. Earlier
+ // releases keep -0.0 and 0.0 apart in a flat array. The check reads the
version rather than a
+ // KnownFloatingPointNormalized marker, because SPARK-59602 adds no marker,
and SPARK-54918
+ // normalizes CreateArray, If, CaseWhen and Coalesce without wrapping the
resulting array.
+ def normalizesFloats(version: String): Boolean =
+ Utils.majorMinorPatchVersion(version).exists {
+ case (4, 0, patch) => patch >= 5
+ case (4, 1, patch) => patch >= 4
Review Comment:
[P2] Preserve null short-circuiting when enabling native `array_union`. On
newly admitted Spark 4.1.4, consider nullable Parquet columns `a:
ARRAY<DOUBLE>` and `b: ARRAY<DOUBLE>` with `a = NULL` and `b = [1.0]`. `SELECT
array_union(a, slice(b, 0, 1)) FROM t` returns NULL in Spark because it skips
the right operand. Comet evaluates both children through `ScalarFunctionExpr`
and throws the invalid-start error from `slice`, aborting the query. The base
revision’s float fallback protected this input. Please retain fallback for
nullable-left/fallible-right shapes or implement Spark-compatible conditional
evaluation before admitting them.
Evidence: Compiled verified Spark `v4.1.4-rc2` expression sources against
Spark 4.1.3 dependencies. Both `ArrayUnion.eval` and its generated
`UnsafeProjection` returned NULL for this row. A probe using the exact-head
`SparkArraySetOp::union`, `SparkArraySlice`, and DataFusion 55.1.0 physical
expressions threw `Unexpected value for start in function slice. Expected a
positive or negative number, but got 0.` Extracted base/head support code
reported `Incompatible` versus `Compatible` for this array type on 4.1.4.
`CometSlice` admits the child, and the native planner constructs an eager
`ScalarFunctionExpr`. This is a split Spark/native reproduction, not a full
Comet integration run.
--
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]
