dwsmith1983 commented on code in PR #5867:
URL: https://github.com/apache/datafusion-comet/pull/5867#discussion_r4073187132
##########
spark/src/main/scala/org/apache/comet/serde/arrays.scala:
##########
@@ -51,7 +51,39 @@ object CometArrayRemove
}
}
-object CometArrayAppend extends CometExpressionSerde[ArrayAppend] with
ArraysBase {
+/**
+ * Shared gate for serdes whose native NULL guard (`CASE WHEN child IS NOT
NULL`) serializes the
+ * child twice: a stateful child drifts between the two copies, so it is
declined and runs through
+ * the JVM codegen dispatcher, where Spark evaluates it once. Nullability is
not consulted: a
+ * non-nullable stateful child only stays in step because DataFusion skips the
filter when the
+ * guard matches every row, which is not a contract to lean on.
+ */
+private[serde] object NullGuardSupport {
+
+ val nondeterministicReason: String =
+ "a nondeterministic operand: the native NULL guard serializes the operand
twice, " +
+ "and the two copies of a stateful operand drift apart"
+
+ /** `Unsupported` when any of `children` is nondeterministic, otherwise
`None`. */
+ def nondeterministicChild(children: Seq[Expression]): Option[SupportLevel] =
+ children
+ .find(child => !child.deterministic)
+ .map(_ => Unsupported(Some(nondeterministicReason)))
+}
+
+object CometArrayAppend
+ extends CometExpressionSerde[ArrayAppend]
+ with ArraysBase
+ with CodegenDispatchFallback {
+
+ override def getUnsupportedReasons(): Seq[String] =
+ Seq(NullGuardSupport.nondeterministicReason)
+
+ // The item sits inside the guard's THEN branch, and DataFusion's CaseExpr
evaluates that
+ // branch only on the rows the guard selects, while Spark's codegen
evaluates the item on
+ // every row. A stateful item therefore drifts the same way a stateful array
does.
+ override def getSupportLevel(expr: ArrayAppend): SupportLevel =
Review Comment:
> Could you link that issue from this comment?
Linked in 0d61cac1e, and the support level changed with it. With ANSI on and
a nullable array, `getSupportLevel` now returns `Incompatible` with the #6086
note, so `array_append` runs through the codegen dispatcher by default and
raises where Spark raises. The native guarded kernel only runs there under
`allowIncompatible`, and the generated guide records the divergence as the
opt-in consequence. With ANSI off, or a non-nullable array, nothing changes. A
new `array_append_ansi_null_array.sql` fixture pins the dispatched raise and
the native path for a non-nullable array. #6086 stays open for evaluating the
item outside the guard, which would bring the native path back under ANSI.
--
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]
