andygrove commented on code in PR #4763:
URL: https://github.com/apache/datafusion-comet/pull/4763#discussion_r3538627602
##########
native/spark-expr/src/conversion_funcs/cast.rs:
##########
Review Comment:
test
##########
native/spark-expr/src/conversion_funcs/cast.rs:
##########
@@ -854,19 +856,51 @@ 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()) },
- }
+ // 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.
+ decode_utf8_spark_lossy(value).into_owned()
Review Comment:
Applied in bdf9491. `cast_binary_to_string` now builds with a
`GenericStringBuilder<O>` and appends the value straight from the decoder,
matching the `cast_array_to_string` pattern you pointed to. To keep the doc
comment on the default-cast path I left `cast_binary_formatter` in place but
changed it to return `Cow<'_, str>`, so the valid path borrows and
`append_value` copies once into the Arrow buffer instead of allocating a
throwaway `String` first. The `ToPrettyString` styles still build owned
strings, unchanged.
--
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]
