sunchao commented on code in PR #5451:
URL: https://github.com/apache/datafusion-comet/pull/5451#discussion_r3845591792
##########
spark/src/main/scala/org/apache/comet/serde/strings.scala:
##########
@@ -713,7 +713,34 @@ object CometBase64 extends CometExpressionSerde[Base64] {
}
}
-object CometUnBase64 extends CometCodegenDispatch[UnBase64]
+// Base64.getMimeDecoder() semantics: skips non-alphabet bytes and matches
Spark's codegen
+// path. The native path handles the default UnBase64 (failOnError = false,
reachable from SQL
+// `unbase64(...)`). When failOnError = true (from `to_binary('base64')` /
`try_to_binary`),
+// Spark uses a stricter RFC 4648 validator, so those cases stay on the JVM
codegen dispatcher
+// via CodegenDispatchFallback. Error messages match Spark byte-for-byte
(pinned in the Rust
+// unit tests), but the wrapping exception class does not; kept as
Compatible() because Spark
+// surfaces these as bare IllegalArgumentException without a SQL error class.
+object CometUnBase64 extends CometExpressionSerde[UnBase64] with
CodegenDispatchFallback {
+
+ private val failOnErrorReason =
+ "unbase64 with failOnError = true uses stricter RFC 4648 validation that
is not yet" +
+ " implemented natively"
+
+ override def getUnsupportedReasons(): Seq[String] = Seq(failOnErrorReason)
+
+ override def getSupportLevel(expr: UnBase64): SupportLevel = {
+ if (expr.failOnError) Unsupported(Some(failOnErrorReason)) else
Compatible()
+ }
+
+ override def convert(expr: UnBase64, inputs: Seq[Attribute], binding:
Boolean): Option[Expr] = {
+ val childExpr = exprToProtoInternal(expr.child, inputs, binding)
Review Comment:
[P2] Preserve null-short-circuiting when lowering the child tree
Recursively serializing `expr.child` changes evaluation of trees that
previously ran entirely through Spark's codegen dispatcher. With a Parquet
table `t(n STRING, bad STRING)` containing `(NULL, 'A')`, `SELECT
unbase64(concat(n, cast(unbase64(bad) AS STRING))) FROM t` returns NULL in
Spark's default generated execution and with the pre-change dispatcher, but
this head fails with `Last unit does not have enough valid bits` (reproduced on
Spark 3.5.9/JDK 17 with `CometProject` + `CometNativeScan`). Spark's generated
concat stops after its first NULL argument; native `ScalarFunctionExpr`
evaluates the inner decoder before concat can apply its null mask. An isolated
control using the exact base serializer restores NULL. Please retain whole-tree
dispatch for affected children, or preserve their short-circuit semantics
natively, before marking all `failOnError = false` trees compatible.
--
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]
