sunchao commented on code in PR #5607:
URL: https://github.com/apache/datafusion-comet/pull/5607#discussion_r3903164108
##########
spark/src/main/scala/org/apache/comet/serde/strings.scala:
##########
@@ -82,7 +82,10 @@ object CometUpper extends
CometCaseConversionBase[Upper]("upper")
object CometLower extends CometCaseConversionBase[Lower]("lower")
-object CometLength extends CometScalarFunction[Length]("length") {
+object CometLength extends CometScalarFunction[Length]("length") with
CodegenDispatchFallback {
Review Comment:
[P2] Preserve child evaluation order for compound binary inputs
This marker dispatches the entire binary-producing child, so the existing
kernel null shortcut can now suppress an earlier ANSI error. An unexecuted
source-derived diagnostic is `IF(flag, length(substring(X'00', CAST(1L DIV 0L
AS INT), n)), 0)` over persisted Parquet rows `(true, NULL)` and `(false,
NULL)`, with `flag BOOLEAN`, nullable `n INT`, and ANSI/Comet
projection/codegen dispatch enabled. In the inspected Spark 3.5/4.0 source, the
conditional keeps the failing constant inside a branch. Spark evaluates
Substring's position before its later length argument, so the selected branch
must raise division by zero even when `n` is null.
Here `CometScalaUDF` captures the Length tree with only `n` bound. Its nodes
pass `allNullIntolerant` and the single-input-ordinal guard in
`CometBatchKernelCodegen`, which writes NULL before evaluating the generated
child code. At BASE the unsupported binary Length has no dispatcher marker and
the enclosing projection falls back to Spark. The new BitLength and OctetLength
markers expose the same issue. Please preserve Spark's evaluation order, or
retain fallback for these unsafe compound trees, and add a regression asserting
the ANSI error for all three roots. This is a source trace, not an executed
reproduction.
--
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]
