etseidl commented on PR #11278:
URL: https://github.com/apache/arrow-rs/pull/11278#issuecomment-6275869897
I unleashed pi/chatgpt-5.6-sol on this. Here's the one finding:
## Finding
### P1 — Required REE schemas accept nullable dense input and silently
replace nulls
**File:** `parquet/src/arrow/arrow_writer/levels.rs:206-212`
**Related schema conversion:** `parquet/src/arrow/schema/mod.rs:991-995`
`LevelInfoBuilder::try_new` now strips the REE wrapper from the target field
and permits a compatible dense input. However, when both the outer REE field
and its value field are non-nullable, the resolved field remains required.
There is no check that the incoming dense array actually has no logical nulls.
I reproduced this at head with:
- Writer field: non-nullable `RunEndEncoded<Int32>` whose value field is
non-nullable
- Batch column: dense `Int32Array([Some(7), None, Some(9)])`
`ArrowWriter::write` returned `Ok(())`, while reading the resulting file
produced:
```text
[7, 0, 9]
```
The null is silently encoded as the primitive default because the generated
Parquet column has no definition level. This case becomes newly reachable
through the PR’s dense/REE compatibility support. Existing tests cover nullable
REE value fields but not a required REE schema receiving nullable dense data.
**Impact:** Silent data corruption for callers writing compatible dense
batches against required REE schemas.
**Smallest safe fix:** After resolving the target wrappers, reject input
with logical nulls when the resolved field is non-nullable, returning an Arrow
schema/type error before constructing `ArrayLevels`. Add a regression test
using a non-nullable REE value field and nullable dense input, asserting
`write` fails rather than round-tripping a replacement value.
Here's the reproducer code it generated:
```rust
use std::sync::Arc;
use arrow::array::{ArrayRef, Int32Array};
use arrow::datatypes::{DataType, Field, Schema};
use arrow::record_batch::RecordBatch;
use parquet::arrow::arrow_writer::{ArrowWriter, ArrowWriterOptions};
use parquet::arrow::arrow_reader::ParquetRecordBatchReaderBuilder;
use bytes::Bytes;
fn main() {
let dense:ArrayRef=Arc::new(Int32Array::from(vec![Some(7),None,Some(9)]));
let ree=DataType::RunEndEncoded(
Arc::new(Field::new("run_ends",DataType::Int32,false)),
Arc::new(Field::new("values",DataType::Int32,false)));
let writer_schema=Arc::new(Schema::new(vec![Field::new("c",ree,false)]));
let
batch_schema=Arc::new(Schema::new(vec![Field::new("c",DataType::Int32,true)]));
let batch=RecordBatch::try_new(batch_schema,vec![dense]).unwrap();
let mut out=Vec::new();
let options=ArrowWriterOptions::new().with_skip_arrow_metadata(true);
let mut w=ArrowWriter::try_new_with_options(&mut
out,writer_schema,options).unwrap();
println!("write={:?}",w.write(&batch)); w.close().unwrap();
let mut
r=ParquetRecordBatchReaderBuilder::try_new(Bytes::from(out)).unwrap().build().unwrap();
let b=r.next().unwrap().unwrap(); println!("{:#?}",b.column(0));
}
```
--
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]
