manuzhang commented on code in PR #4763:
URL: https://github.com/apache/datafusion-comet/pull/4763#discussion_r3578445699
##########
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:
This lossy conversion collapses distinct Spark strings before downstream
native operations. For example, if column `b` contains `X'FF'`:
```sql
CAST(b AS STRING) = CAST(X'EFBFBD' AS STRING)
```
Spark returns `false` because `CAST(binary AS string)` preserves the raw
bytes and `UTF8String` equality compares them directly. Comet will decode both
values to U+FFFD and return `true`. The same issue can affect joins, grouping,
ordering, and byte-based string operations such as `contains`.
--
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]
