sunchao commented on code in PR #7106:
URL: https://github.com/apache/datafusion-comet/pull/7106#discussion_r4315267020
##########
native/core/src/parquet/parquet_support.rs:
##########
@@ -330,6 +330,26 @@ fn parquet_convert_array_impl(
Ok(Arc::new(string_array))
}
+ // Parquet's unannotated BINARY is read as Spark StringType when
+ // spark.sql.parquet.binaryAsString is enabled or a read schema
declares a string.
+ // Arrow's cast below is safe, so it would turn malformed UTF-8 into
NULL. Use Spark's
+ // cast so nested values (struct fields, list elements, map keys and
values) replace
+ // malformed input with U+FFFD, as the JVM's `new String(bytes,
UTF_8)` does. Top-level
+ // columns don't reach this arm: DataFusion's file-schema coercion
decodes them straight
+ // into Utf8 (#7032).
+ (Binary, Utf8) | (LargeBinary, LargeUtf8) => {
+ let mut cast_options =
+ SparkCastOptions::new(parquet_options.eval_mode,
&parquet_options.timezone);
+ cast_options.allow_cast_unsigned_ints =
parquet_options.allow_cast_unsigned_ints;
+ cast_options.is_adapting_schema = true;
+
+ match spark_cast(ColumnarValue::Array(array), to_type,
&cast_options)? {
Review Comment:
[P2] Preserve the zero-copy path for valid nested strings. When an
unannotated nested BINARY column contains valid UTF-8 and is read as a string,
this unconditional `spark_cast` call now rebuilds every value through
`GenericStringBuilder`. Previously Arrow validated the array and reused its
value and offset buffers. A release-mode probe of 8,192 eight-byte values
measured 6.85 µs before versus 48.47 µs after, with the new output allocating a
separate value buffer. This adds CPU and copying to existing valid-data reads
under either `binaryAsString=true` or an explicit string schema. Could we first
attempt a validated zero-copy conversion and invoke Spark’s replacement decoder
only when validation fails?
Evidence: Ran `cargo run --release --offline --manifest-path
/tmp/comet-pr7106-kernel/Cargo.toml`. The disposable harness extracts the exact
head’s `cast_binary_to_string` and its formatting helpers unchanged, imports
`native/common/src/utils.rs`, and uses Arrow 59.3.0. It compares Arrow
`cast_with_options(..., Utf8, CastOptions::default())` with
`cast_binary_to_string::<i32>` using `binary_output_style=None`, over 8,192
valid ASCII values. Median timings across seven alternating measurement groups
were 6.85→48.47 µs for eight-byte values, 11.48→51.20 µs for 32-byte values,
and 233.76→418.96 µs for 1,024-byte values. Output equality passed.
Buffer-pointer checks confirmed the old output shares the input value buffer
while the new output does not, including an additional 8 MiB value buffer in
the last case. Measurements isolate conversion cost.
--
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]
