0lai0 commented on code in PR #5451:
URL: https://github.com/apache/datafusion-comet/pull/5451#discussion_r3853862799
##########
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:
Thanks @sunchao for review.
I went with whole-tree dispatch for non-leaf children.
`getSupportLevel` now returns `Compatible()` only for `Attribute` /
`Literal` children. Anything else is `Unsupported` and stays on the codegen
dispatcher via `CodegenDispatchFallback`, so Spark's `concat` short-circuit is
preserved. Native `unbase64` is only used for `unbase64(col)` and
`unbase64('literal')`.
Regression coverage:
- `unbase64.sql`: `CASE WHEN` over Parquet columns (NULL branch first,
`unbase64('A')` in the other), works across 3.4–4.1.
- `unbase64_concat_short_circuit_spark_3_5.sql`: your exact `concat` query,
gated to Spark 3.5. On 4.x Spark itself raises on `(NULL, 'A')`, so that form
cannot be checked against a 4.x reference.
Kernel is unchanged. Local `CometSqlFileTestSuite` passes on both profiles:
`unbase64` / `to_binary` / `base64` on `-Pspark-4.1`, and both
`unbase64.sql` and the concat fixture on `-Pspark-3.5`.
The 3.5-only fixture is `SKIPPED` on 4.1 as expected.
--
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]
