andygrove opened a new pull request, #5037:
URL: https://github.com/apache/datafusion-comet/pull/5037
## Which issue does this PR close?
Closes #3183.
## Rationale for this change
The Spark `encode(str, charset)` expression (StringType, StringType) ->
BinaryType previously fell back to Spark: there is no native charset encoder in
Comet, so any query using `encode` broke the native pipeline.
Rather than reimplement charset encoding natively, this routes `encode`
through the JVM codegen dispatcher, which compiles Spark's own `Encode` code
into an Arrow-in/Arrow-out kernel that runs inside the Comet pipeline. Behavior
therefore matches Spark exactly, including Spark 4.0+'s `legacyJavaCharsets` /
`legacyCodingErrorAction` conf-driven behavior, the UTF-8 fast path, and the
strict-mode error on malformed input. This is the same approach already used
for the dual `decode` expression (#4465), and reimplementing the charset logic
natively would risk drifting from Spark's per-version semantics.
## What changes are included in this PR?
`encode` is wired for native execution via codegen dispatch, split by the
two shapes Spark uses across versions:
- Spark 3.4 / 3.5, where `Encode` is a `BinaryExpression`: a new one-line
serde `object CometEncode extends CometCodegenDispatch[Encode]` (in the shared
`spark-3.x` source set, mirroring `CometStringDecode`), registered in the
`sparkVersionSpecificStringExpressions` map of both the 3.4 and 3.5
`CometExprShim`.
- Spark 4.0 / 4.1 / 4.2, where `encode` is `RuntimeReplaceable` and lowers
to `StaticInvoke(classOf[Encode], "encode", ...)`: a `("encode",
classOf[Encode]) -> CometStaticInvokeCodegenDispatch` entry in the
`staticInvokeExpressions` map in `statics.scala`, alongside the existing
`decode` entry.
- The `encode` support marker in the expressions user-guide page is flipped
to supported.
No native Rust is added. `encode` is eligible for the dispatcher because its
output (`BinaryType`) and inputs (`StringType`, plus the Spark 4.0 boolean flag
literals) are all in `CometBatchKernelCodegen.canHandle`'s supported type
surface.
Note on per-version behavior (matched by construction, since Spark's own
code runs): on Spark 3.4/3.5 malformed input substitutes replacement bytes,
while on Spark 4.0+ the strict default coding-error action throws. For example
`encode('café', 'US-ASCII')` throws on 4.0+ and substitutes on 3.x, exactly as
it does without Comet.
## How are these changes tested?
A new SQL file test `encode.sql` under `sql-tests/expressions/string/`
covers `encode` over six charsets (UTF-8, UTF-16, UTF-16BE, UTF-16LE, US-ASCII,
ISO-8859-1), null and empty-string input, a non-literal charset column, and a
`decode(encode(...))` round-trip. Each block is a `query` block, so
`CometSqlFileTestSuite`'s `checkSparkAnswerAndOperator` asserts the expression
executes natively rather than falling back, making the coverage non-vacuous.
The suite passes with all blocks native under the Spark 3.4, 3.5, and 4.0
profiles. The US-ASCII case uses ASCII-only input so the fixture stays portable
across the lenient (3.x) and strict (4.0+) behaviors.
---
This work was scaffolded with the superpowers brainstorming, writing-plans,
and subagent-driven-development skills.
--
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]
