sunchao commented on code in PR #7106:
URL: https://github.com/apache/datafusion-comet/pull/7106#discussion_r4324232246
##########
native/spark-expr/src/conversion_funcs/cast.rs:
##########
@@ -876,7 +878,52 @@ fn cast_binary_to_string<O: OffsetSizeTrait>(
let num_rows = input.len();
let offsets = input.value_offsets();
- let value_bytes = offsets[num_rows].as_usize() - offsets[0].as_usize();
+ let start = offsets[0].as_usize();
+ let end = offsets[num_rows].as_usize();
+
+ // Check the bytes the rows span once. If they are valid UTF-8, reuse the
input's value buffer;
+ // otherwise copy the rows in the valid prefix and decode the rest.
+ let mut valid_prefix = "";
+ if matches!(
+ spark_cast_options.binary_output_style,
+ None | Some(BinaryOutputStyle::Utf8)
+ ) {
+ let data = input.value_data();
+ match std::str::from_utf8(&data[start..end]) {
Review Comment:
[P2] Avoid validating substantial payload hidden beneath null rows. For
`CAST(CASE WHEN selected THEN payload WHEN other THEN payload2 END AS STRING)`,
a batch where only the first branch matches can retain the entire binary value
buffer while marking most rows null. Both payloads are columns, so Spark’s
`PushFoldableIntoBranches` leaves the outer cast in place. The previous decoder
skipped null payloads, but this whole-span validation scans them all. With 82
non-null rows out of 8,192, conversion increased from 56.3 to 260.2 µs for 1
KiB values and from 699.8 to 4,070.5 µs for 8 KiB values, with identical
results. This slows existing binary casts independently of the Parquet fix.
Could the fast path retain row-based conversion when nulls hide substantial
payload, or otherwise avoid scanning those bytes?
Evidence: Ran
`/tmp/comet-pr7106-kernel/target/release/comet-pr7106-79b-kernel
--null-confirm`. The release harness uses Arrow 59.3.0, the repository decoder,
and cast/formatting helpers verified unchanged against the supplied base and
exact head. Arrow `nullif` masks 8,110 of 8,192 valid ASCII values without
copying their buffers. Median times over nine alternating groups of 100 casts
were 56.329→260.183 µs for an 8 MiB buffer and 699.812→4070.534 µs for a 64 MiB
buffer. Output equality and full Arrow validation passed. An exact-head native
`CaseWhenExpr` test independently confirmed 82 live rows, 83,968 live bytes and
a shared 8 MiB value buffer for two non-foldable binary branches. Its
disposable fixture is preserved at
`/tmp/comet-pr7106-79b-review-null-payload.rs`. Measurements isolate conversion
cost, not whole-query runtime.
--
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]
