sunchao commented on code in PR #7340:
URL: https://github.com/apache/datafusion-comet/pull/7340#discussion_r4428977680
##########
native/core/src/parquet/parquet_support.rs:
##########
@@ -312,6 +318,60 @@ fn parquet_convert_array_impl(
Ok(Arc::new(string_array))
}
+ // Unannotated BINARY read as a string. Spark's cast replaces
malformed UTF-8 with
+ // U+FFFD, where Arrow's safe cast below would return NULL. Top-level
columns don't
+ // reach this arm (#7032). The reader keeps a file's ARROW:schema
types, so an Arrow
+ // writer's `LargeBinary` or `BinaryView` arrives as is; Spark's cast
takes only
+ // `Binary`, and converting `LargeBinary` reuses its value buffer.
+ (Binary | LargeBinary | BinaryView, Utf8) => {
+ let array = match from_type {
+ Binary => array,
+ _ => cast_with_options(&array, &Binary, &PARQUET_OPTIONS)?,
+ };
+ if in_map_key {
+ // Spark tells map keys apart by their bytes. Decoding turns
the keys `FF` and
+ // `EF BF BD` into the same U+FFFD, so a lookup of one returns
the other's value
+ // and `map_from_entries` fails with DUPLICATED_MAP_KEY. A key
keeps its bytes
+ // instead, unchecked, as a top-level value does (#7032).
+ let (offsets, values, nulls) =
array.as_binary::<i32>().clone().into_parts();
+ let keys = unsafe { StringArray::new_unchecked(offsets,
values, nulls) };
Review Comment:
The malformed key in this PR’s own fixture violates the precondition of
`StringArray::new_unchecked`. With Arrow 59.3.0, I passed the fixture’s two
keys (`41 FF 42` and `41 EF BF BD 42`) to `StringArray::try_new` using the same
offsets, bytes and validity. It rejects the first key at byte index 1. The
unchecked constructor requires that the checked one would succeed, and a later
string operation can obtain an invalid Rust `&str`. Preserving distinct map
keys matters, so replacing both with U+FFFD would also lose information. Could
we keep the keys as binary until a byte-preserving boundary, fall back to Spark
for the affected scan, or return a checked error before constructing the string
array? [Pinned Arrow constructor
contract](https://docs.rs/arrow-array/59.3.0/arrow_array/array/struct.GenericByteArray.html#method.new_unchecked)
--
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]
