andygrove opened a new issue, #27065:
URL: https://github.com/apache/datafusion/issues/27065
### Describe the bug
Reading a Parquet file with a nested BINARY column (for example
`list<binary>`) through a table schema that declares the nested field as `Utf8`
produces a `Utf8` array that has not been checked for valid UTF-8. When a value
is not valid UTF-8:
- in a debug build, the parquet reader panics in
`parquet/src/arrow/buffer/offset_buffer.rs:139`
(`ArrayDataBuilder::build().unwrap()`)
- in a release build, the scan returns a `StringArray` that holds invalid
UTF-8 (`validate_full()` fails), which is unsound for any kernel that reads it
as `&str`
On 55.2.0 the same read fails with a normal error: `Arrow error: Invalid
argument error: Encountered non UTF-8 data: invalid utf-8 sequence of 1 bytes
from index 3`.
This is a regression from #25193 ("make Parquet file schema type coercion
work on nested schemas"), which is on `main` and `branch-56`.
`apply_file_schema_type_coercions` now rewrites nested `Binary` / `LargeBinary`
/ `BinaryView` file fields to `Utf8` when the table schema has `Utf8` there,
and hands that schema to the parquet reader with
`ArrowReaderOptions::with_schema`. The parquet reader validates UTF-8 only when
the column carries the `UTF8` annotation (`validate_utf8 =
desc.converted_type() == ConvertedType::UTF8` in
`parquet/src/arrow/array_reader/byte_array.rs`), which a BINARY column does
not. Before #25193, nested binary fields were read as binary and converted by a
cast, which validates.
The doc comment on `coerce_data_type` already notes that the conversion
"hands back an unvalidated string array". The same gap exists for top-level
fields on 55.2, but #25193 extends it to every nested field.
### To Reproduce
Against `branch-56` (`1635ed94742c1c060852bc34dc6ebb2302241c7b`), with
`parquet`/`arrow` 60.1.0 and `tokio`:
```rust
use std::sync::Arc;
use arrow::array::{Array, AsArray, BinaryArray, ListArray};
use arrow::buffer::OffsetBuffer;
use arrow::datatypes::{DataType, Field, Schema};
use arrow::record_batch::RecordBatch;
use datafusion::prelude::*;
use parquet::arrow::ArrowWriter;
#[tokio::main]
async fn main() -> datafusion::error::Result<()> {
// A list<binary> column whose second element is not valid UTF-8.
let element = Arc::new(Field::new("element", DataType::Binary, true));
let values = BinaryArray::from(vec![b"one".as_slice(), &[0xff]]);
let list = ListArray::new(element, OffsetBuffer::new(vec![0, 2].into()),
Arc::new(values), None);
let file_schema = Arc::new(Schema::new(vec![Field::new("s",
list.data_type().clone(), true)]));
let batch = RecordBatch::try_new(Arc::clone(&file_schema),
vec![Arc::new(list)])?;
let path = std::env::temp_dir().join("nested_binary.parquet");
let mut writer = ArrowWriter::try_new(std::fs::File::create(&path)?,
file_schema, None)?;
writer.write(&batch)?;
writer.close()?;
// Read it with a table schema that declares the elements as strings.
let table_schema = Schema::new(vec![Field::new(
"s",
DataType::List(Arc::new(Field::new("element", DataType::Utf8,
true))),
true,
)]);
let ctx = SessionContext::new();
ctx.register_parquet(
"t",
path.to_str().unwrap(),
ParquetReadOptions::default().schema(&table_schema),
)
.await?;
let batches = ctx.sql("SELECT s FROM t").await?.collect().await?;
let strings = batches[0].column(0).as_list::<i32>().values().clone();
println!("element type: {}", strings.data_type());
println!("validate_full: {:?}", strings.to_data().validate_full());
Ok(())
}
```
Debug build:
```
thread 'main' panicked at
parquet-60.1.0/src/arrow/buffer/offset_buffer.rs:139:48:
called `Result::unwrap()` on an `Err` value: InvalidArgumentError("Invalid
UTF8 sequence at string index 1 (3..4): invalid utf-8 sequence of 1 bytes from
index 0")
```
Release build:
```
element type: Utf8
validate_full: Err(InvalidArgumentError("Invalid UTF8 sequence at string
index 1 (3..4): invalid utf-8 sequence of 1 bytes from index 0"))
```
With `datafusion = "55.2.0"` and arrow/parquet 59.2.0, the same program
returns:
```
Error: ArrowError(InvalidArgumentError("Encountered non UTF-8 data: invalid
utf-8 sequence of 1 bytes from index 3"), Some(""))
```
### Expected behavior
Reading BINARY data as a string should never produce an unvalidated
`StringArray`. Either the read fails with an error, as on 55.2.0, or the values
are validated.
### Additional context
Possible fixes:
- Have the parquet reader validate UTF-8 whenever the target Arrow type is a
string type, not only when the column has the `UTF8` annotation. That also
closes the existing top-level gap. It probably needs an arrow-rs change.
- Leave binary-to-string out of the nested coercion (keep the nested
view-type coercions) so those fields still go through the validating cast.
Found while upgrading DataFusion Comet to DataFusion 56:
apache/datafusion-comet#7469.
--
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]
