parthchandra commented on code in PR #4763:
URL: https://github.com/apache/datafusion-comet/pull/4763#discussion_r3575413519
##########
native/spark-expr/src/conversion_funcs/cast.rs:
##########
@@ -853,20 +859,78 @@ fn spark_binary_formatter(value: &[u8],
binary_output_style: BinaryOutputStyle)
}
}
-fn cast_binary_formatter(value: &[u8]) -> String {
- match String::from_utf8(value.to_vec()) {
- Ok(value) => value,
- Err(_) => unsafe { String::from_utf8_unchecked(value.to_vec()) },
- }
+fn cast_binary_formatter(value: &[u8]) -> Cow<'_, str> {
+ // CAST(binary AS string) reinterprets the bytes as UTF-8, like Spark's
UTF8String.fromBytes.
+ // Spark keeps the raw bytes, but Arrow's Utf8 type requires valid UTF-8,
and building a String
+ // from non-UTF-8 bytes is undefined behaviour (#4488). Decode
JVM-compatibly-lossily instead:
+ // `decode_utf8_spark_lossy` replaces ill-formed sequences with U+FFFD
exactly as Spark's
+ // `new String(bytes, UTF_8)` does (the same decoder Comet's native
shuffle uses, #4521). The
+ // result is memory-safe valid UTF-8, never feeds invalid bytes into
downstream native string
+ // kernels, and matches Spark's rendered output byte-for-byte. It diverges
from Spark only under
+ // byte-level round-trips such as CAST(CAST(x AS string) AS binary), where
Spark still has the
+ // original bytes and Comet has the U+FFFD replacements. Returning `Cow`
lets the valid path
+ // borrow so the caller appends without an intermediate allocation.
+ decode_utf8_spark_lossy(value)
Review Comment:
Should we add a test to verify that invalid characters have been replaced
after this call? (Any future changes in the behavior of
`decode_utf8_spark_lossy` will then be caught by the test.
--
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]
