This is an automated email from the ASF dual-hosted git repository.
sdf-jkl pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
The following commit(s) were added to refs/heads/main by this push:
new 8634908e14 fix(variant): reject FixedSizeList shredding (#10639)
8634908e14 is described below
commit 8634908e1426019765a1e0a92b2b925ff055e3e8
Author: cakeni <[email protected]>
AuthorDate: Tue Sep 22 01:35:30 2026 +0800
fix(variant): reject FixedSizeList shredding (#10639)
# Which issue does this PR close?
- Closes #10616.
# Rationale for this change
The Variant shredding spec does not define `FixedSizeList` as a valid
shredded `typed_value`. Keep writes strict by rejecting it in
`shred_variant`, while normalizing legacy Arrow `FixedSizeList` metadata
to `List` on read for backwards compatibility.
# What changes are included in this PR?
- Reject `DataType::FixedSizeList` when selecting a shredding type.
- Normalize `FixedSizeList` `typed_value` to `List` in
`VariantArray::try_new`.
- Remove the FSL-specific path from `unshred_variant`.
- Add regression coverage for invalid shredding and persisted Parquet
read/unshred.
# Are these changes tested?
- `cargo test -p parquet-variant-compute --lib`
- Persisted Parquet read/unshred regression for legacy FSL metadata.
# Are there any user-facing changes?
`shred_variant` rejects `FixedSizeList`; existing data with Arrow
`FixedSizeList` metadata is read as a regular `List`.
---------
Co-authored-by: cakeni <[email protected]>
Co-authored-by: Kosta Tarasov <[email protected]>
---
parquet-variant-compute/src/shred_variant.rs | 90 ++------------------------
parquet-variant-compute/src/unshred_variant.rs | 10 +--
parquet-variant-compute/src/variant_array.rs | 46 +++++++++++--
parquet/src/variant.rs | 59 +++++++++++++++--
4 files changed, 103 insertions(+), 102 deletions(-)
diff --git a/parquet-variant-compute/src/shred_variant.rs
b/parquet-variant-compute/src/shred_variant.rs
index 9686be6a92..2ad0342ed2 100644
--- a/parquet-variant-compute/src/shred_variant.rs
+++ b/parquet-variant-compute/src/shred_variant.rs
@@ -157,8 +157,7 @@ pub(crate) fn
make_variant_to_shredded_variant_arrow_row_builder<'a>(
DataType::List(_)
| DataType::LargeList(_)
| DataType::ListView(_)
- | DataType::LargeListView(_)
- | DataType::FixedSizeList(..) => {
+ | DataType::LargeListView(_) => {
let typed_value_builder =
VariantToShreddedArrayVariantRowBuilder::try_new(
data_type,
cast_options,
@@ -330,7 +329,6 @@ impl<'a> VariantToShreddedArrayVariantRowBuilder<'a> {
self.nulls.append_non_null();
self.value_builder.append_null();
- // NOTE: A `FixedSizeList` with incorrect size will hard fail
during shredding.
self.typed_value_builder
.append_value(&Variant::List(list))?;
Ok(true)
@@ -738,9 +736,9 @@ mod tests {
use crate::variant_array::{all_null_value_column, binary_array_value,
variant_from_arrays_at};
use arrow::array::{
Array, BinaryViewArray, Decimal32Array, Decimal64Array,
Decimal128Array,
- FixedSizeBinaryArray, FixedSizeListArray, Float64Array,
GenericListArray,
- GenericListViewArray, Int64Array, LargeBinaryArray, LargeStringArray,
ListArray,
- ListLikeArray, OffsetSizeTrait, PrimitiveArray, StringArray,
StructArray,
+ FixedSizeBinaryArray, Float64Array, GenericListArray,
GenericListViewArray, Int64Array,
+ LargeBinaryArray, LargeStringArray, ListArray, ListLikeArray,
OffsetSizeTrait,
+ PrimitiveArray, StringArray, StructArray,
};
use arrow::datatypes::{
ArrowPrimitiveType, DataType, Field, Fields, Int64Type, TimeUnit,
UnionFields, UnionMode,
@@ -1510,6 +1508,7 @@ mod tests {
DataType::Time32(TimeUnit::Second),
DataType::Time64(TimeUnit::Nanosecond),
DataType::Timestamp(TimeUnit::Millisecond, None),
+ DataType::FixedSizeList(Arc::new(Field::new("item",
DataType::Int64, true)), 2),
DataType::FixedSizeBinary(17),
DataType::Union(
UnionFields::from_fields(vec![
@@ -1694,85 +1693,6 @@ mod tests {
);
}
- #[test]
- fn test_array_shredding_as_fixed_size_list() {
- let input = build_variant_array(vec![
- VariantRow::List(vec![VariantValue::from(1i64),
VariantValue::from(2i64)]),
- VariantRow::Value(VariantValue::from("This should not be
shredded")),
- VariantRow::List(vec![VariantValue::from(3i64),
VariantValue::from(4i64)]),
- ]);
-
- let list_schema =
- DataType::FixedSizeList(Arc::new(Field::new("item",
DataType::Int64, true)), 2);
- let result = shred_variant(&input, &list_schema).unwrap();
- assert_eq!(result.len(), 3);
-
- // The first row should be shredded, so the `value` field should be
null and the
- // `typed_value` field should contain the list
- assert!(result.is_valid(0));
- assert!(result.value_column().is_null(0));
- assert!(result.typed_value_column().unwrap().is_valid(0));
-
- // The second row should not be shredded because the provided schema
for shredding did not
- // match. Hence, the `value` field should contain the raw value and
the `typed_value` field
- // should be null.
- assert!(result.is_valid(1));
- assert!(result.value_column().is_valid(1));
- assert!(result.typed_value_column().unwrap().is_null(1));
-
- // The third row should be shredded, so the `value` field should be
null and the
- // `typed_value` field should contain the list
- assert!(result.is_valid(2));
- assert!(result.value_column().is_null(2));
- assert!(result.typed_value_column().unwrap().is_valid(2));
-
- let typed_value = result.typed_value_column().unwrap();
- let fixed_size_list = typed_value
- .as_any()
- .downcast_ref::<FixedSizeListArray>()
- .expect("Expected FixedSizeListArray");
-
- // Verify that typed value is `FixedSizeList`.
- assert_eq!(fixed_size_list.len(), 3);
- assert_eq!(fixed_size_list.value_length(), 2);
-
- // Verify that the first entry in the `FixedSizeList` contains the
expected value.
- let val0 = fixed_size_list.value(0);
- let val0_struct = val0.as_any().downcast_ref::<StructArray>().unwrap();
- let val0_typed = val0_struct.column_by_name("typed_value").unwrap();
- let val0_ints =
val0_typed.as_any().downcast_ref::<Int64Array>().unwrap();
- assert_eq!(val0_ints.values(), &[1i64, 2i64]);
-
- // Verify that second entry in the `FixedSizeList` cannot be shredded
hence the value is
- // invalid.
- assert!(fixed_size_list.is_null(1));
-
- // Verify that the third entry in the `FixedSizeList` contains the
expected value.
- let val2 = fixed_size_list.value(2);
- let val2_struct = val2.as_any().downcast_ref::<StructArray>().unwrap();
- let val2_typed = val2_struct.column_by_name("typed_value").unwrap();
- let val2_ints =
val2_typed.as_any().downcast_ref::<Int64Array>().unwrap();
- assert_eq!(val2_ints.values(), &[3i64, 4i64]);
- }
-
- #[test]
- fn test_array_shredding_as_fixed_size_list_wrong_size() {
- let input = build_variant_array(vec![VariantRow::List(vec![
- VariantValue::from(1i64),
- VariantValue::from(2i64),
- VariantValue::from(3i64),
- ])]);
- let list_schema =
- DataType::FixedSizeList(Arc::new(Field::new("item",
DataType::Int64, true)), 2);
-
- let err = shred_variant(&input, &list_schema).unwrap_err();
- assert!(
- err.to_string()
- .contains("Expected fixed size list of size 2, got size 3"),
- "got: {err}",
- );
- }
-
#[test]
fn test_array_shredding_with_array_elements() {
let input = build_variant_array(vec![
diff --git a/parquet-variant-compute/src/unshred_variant.rs
b/parquet-variant-compute/src/unshred_variant.rs
index 14afb8db12..40a22a3f54 100644
--- a/parquet-variant-compute/src/unshred_variant.rs
+++ b/parquet-variant-compute/src/unshred_variant.rs
@@ -21,9 +21,8 @@ use crate::variant_array::{binary_array_value,
validate_binary_array};
use crate::{VariantArray, VariantValueArrayBuilder};
use arrow::array::{
Array, ArrayRef, AsArray as _, BinaryArray, BinaryViewArray, BooleanArray,
- FixedSizeBinaryArray, FixedSizeListArray, GenericListArray,
GenericListViewArray,
- LargeBinaryArray, LargeStringArray, ListLikeArray, PrimitiveArray,
StringArray,
- StringViewArray, StructArray,
+ FixedSizeBinaryArray, GenericListArray, GenericListViewArray,
LargeBinaryArray,
+ LargeStringArray, ListLikeArray, PrimitiveArray, StringArray,
StringViewArray, StructArray,
};
use arrow::buffer::NullBuffer;
use arrow::datatypes::{
@@ -185,7 +184,6 @@ enum UnshredVariantRowBuilder<'a> {
LargeList(ListUnshredVariantBuilder<'a, GenericListArray<i64>>),
ListView(ListUnshredVariantBuilder<'a, GenericListViewArray<i32>>),
LargeListView(ListUnshredVariantBuilder<'a, GenericListViewArray<i64>>),
- FixedSizeList(ListUnshredVariantBuilder<'a, FixedSizeListArray>),
Struct(StructUnshredVariantBuilder<'a>),
ValueOnly(ValueOnlyUnshredVariantBuilder<'a>),
Null(NullUnshredVariantBuilder),
@@ -230,7 +228,6 @@ impl<'a> UnshredVariantRowBuilder<'a> {
Self::LargeList(b) => b.append_row(builder, metadata, index),
Self::ListView(b) => b.append_row(builder, metadata, index),
Self::LargeListView(b) => b.append_row(builder, metadata, index),
- Self::FixedSizeList(b) => b.append_row(builder, metadata, index),
Self::Struct(b) => b.append_row(builder, metadata, index),
Self::ValueOnly(b) => b.append_row(builder, metadata, index),
Self::Null(b) => b.append_row(builder, metadata, index),
@@ -342,9 +339,6 @@ impl<'a> UnshredVariantRowBuilder<'a> {
value,
typed_value.as_list_view(),
)?),
- DataType::FixedSizeList(_, _) => Self::FixedSizeList(
- ListUnshredVariantBuilder::try_new(value,
typed_value.as_fixed_size_list())?,
- ),
_ => {
return Err(ArrowError::NotYetImplemented(format!(
"Unshredding not yet supported for type: {}",
diff --git a/parquet-variant-compute/src/variant_array.rs
b/parquet-variant-compute/src/variant_array.rs
index 92bdbe8d4b..da051fe549 100644
--- a/parquet-variant-compute/src/variant_array.rs
+++ b/parquet-variant-compute/src/variant_array.rs
@@ -331,7 +331,8 @@ impl VariantArray {
/// binary_view
///
/// 3. An optional field named `typed_value` which can be any primitive
type
- /// or be a list, large_list, list_view or struct
+ /// or be a list, large_list, fixed_size_list, list_view or struct.
Fixed-size lists are
+ /// normalized to variable-length lists on read.
///
pub fn try_new(inner: &dyn Array) -> Result<Self> {
// Canonicalize shredded typed_value fields (e.g. decimal narrowing)
@@ -1297,7 +1298,15 @@ fn canonicalize_and_verify_data_type_impl(
// UUID maps to 16-byte fixed-size binary; no other width is allowed
FixedSizeBinary(16) => borrow!(),
- FixedSizeBinary(_) | FixedSizeList(..) => fail!(),
+ FixedSizeBinary(_) => fail!(),
+
+ // FixedSizeList is an Arrow-specific distinction. Normalize it to
List on read so
+ // Variant data written by older arrow-rs versions remains readable
without treating
+ // FixedSizeList as a supported shredding target.
+ FixedSizeList(field, _) => match canonicalize_and_verify_field(field)?
{
+ Cow::Borrowed(_) => Cow::Owned(DataType::List(field.clone())),
+ Cow::Owned(new_field) => Cow::Owned(DataType::List(new_field)),
+ },
// List-like containers and struct are allowed, maps and unions are not
List(field) => match canonicalize_and_verify_field(field)? {
@@ -1398,9 +1407,9 @@ mod test {
use super::*;
use arrow::array::{
BinaryArray, BinaryDictionaryBuilder, BinaryRunBuilder,
BinaryViewArray, Decimal32Array,
- Decimal64Array, Decimal128Array, FixedSizeBinaryArray, Int8Array,
Int32Array, Int64Array,
- LargeBinaryArray, LargeListArray, LargeListViewArray, ListArray,
ListViewArray,
- StringArray, Time64MicrosecondArray,
+ Decimal64Array, Decimal128Array, FixedSizeBinaryArray,
FixedSizeListArray, Int8Array,
+ Int32Array, Int64Array, LargeBinaryArray, LargeListArray,
LargeListViewArray, ListArray,
+ ListViewArray, StringArray, Time64MicrosecondArray,
};
use arrow::buffer::{OffsetBuffer, ScalarBuffer};
use arrow_schema::{Field, Fields};
@@ -1718,6 +1727,33 @@ mod test {
}
}
+ #[test]
+ fn variant_array_try_new_normalizes_fixed_size_list_typed_value() {
+ let element_values: ArrayRef =
+
ShreddedVariantFieldArray::perfectly_shredded(Arc::new(Int64Array::from(vec![
+ 1, 2, 3, 4,
+ ])))
+ .into();
+ let item_field = Arc::new(Field::new("item",
element_values.data_type().clone(), true));
+ let typed_value: ArrayRef = Arc::new(FixedSizeListArray::new(
+ item_field.clone(),
+ 2,
+ element_values,
+ None,
+ ));
+ let input = make_variant_struct_with_typed_value(typed_value);
+
+ let variant_array = VariantArray::try_new(&input).unwrap();
+ assert_eq!(
+ variant_array.typed_value_column().unwrap().data_type(),
+ &DataType::List(item_field),
+ );
+
+ let unshredded = crate::unshred_variant(&variant_array).unwrap();
+ assert!(unshredded.typed_value_column().is_none());
+ assert_eq!(unshredded.len(), 2);
+ }
+
#[test]
fn test_try_value_out_of_bounds() {
let mut b = VariantArrayBuilder::new(2);
diff --git a/parquet/src/variant.rs b/parquet/src/variant.rs
index 55df086736..23f45b10ee 100644
--- a/parquet/src/variant.rs
+++ b/parquet/src/variant.rs
@@ -147,11 +147,16 @@ mod tests {
use crate::file::metadata::{ParquetMetaData, ParquetMetaDataReader};
use crate::file::reader::ChunkReader;
use arrow::util::test_util::parquet_test_data;
- use arrow_array::{ArrayRef, RecordBatch};
- use arrow_schema::Schema;
+ use arrow_array::{
+ Array, ArrayRef, BinaryViewArray, FixedSizeListArray, Int64Array,
RecordBatch, StructArray,
+ new_null_array,
+ };
+ use arrow_schema::{DataType, Field, Fields, Schema};
use bytes::Bytes;
- use parquet_variant::{Variant, VariantBuilderExt};
- use parquet_variant_compute::{VariantArray, VariantArrayBuilder,
VariantType};
+ use parquet_variant::{EMPTY_VARIANT_METADATA_BYTES, Variant,
VariantBuilderExt};
+ use parquet_variant_compute::{
+ VariantArray, VariantArrayBuilder, VariantType, unshred_variant,
+ };
use std::path::PathBuf;
use std::sync::Arc;
@@ -181,6 +186,52 @@ mod tests {
assert_eq!(var_value, Variant::from("iceberg"));
}
+ #[test]
+ fn read_fixed_size_list_typed_value_as_list() {
+ let element_values: ArrayRef = Arc::new(Int64Array::from(vec![1, 2, 3,
4]));
+ let element_value = new_null_array(&DataType::BinaryView, 4);
+ let element_fields = Fields::from(vec![
+ Field::new("value", DataType::BinaryView, true),
+ Field::new("typed_value", DataType::Int64, true),
+ ]);
+ let elements: ArrayRef = Arc::new(StructArray::new(
+ element_fields,
+ vec![element_value, element_values],
+ None,
+ ));
+ let item_field = Arc::new(Field::new("item",
elements.data_type().clone(), true));
+ let typed_value: ArrayRef =
+ Arc::new(FixedSizeListArray::new(item_field, 2, elements, None));
+ let metadata: ArrayRef =
Arc::new(BinaryViewArray::from_iter_values(std::iter::repeat_n(
+ EMPTY_VARIANT_METADATA_BYTES,
+ 2,
+ )));
+ let value = new_null_array(&DataType::BinaryView, 2);
+ let fields = Fields::from(vec![
+ Field::new("metadata", DataType::BinaryView, false),
+ Field::new("value", DataType::BinaryView, true),
+ Field::new("typed_value", typed_value.data_type().clone(), true),
+ ]);
+ let source = StructArray::new(fields, vec![metadata, value,
typed_value], None);
+ let field = Field::new("data", source.data_type().clone(), false);
+ let batch =
+ RecordBatch::try_new(Arc::new(Schema::new(vec![field])),
vec![Arc::new(source)])
+ .unwrap();
+
+ let buffer = write_to_buffer(&batch);
+ let result = read_to_batch(Bytes::from(buffer));
+ let column = result.column_by_name("data").unwrap();
+ let variant = VariantArray::try_new(column).unwrap();
+ assert!(matches!(
+ variant.typed_value_column().unwrap().data_type(),
+ DataType::List(_)
+ ));
+
+ let unshredded = unshred_variant(&variant).unwrap();
+ assert!(unshredded.typed_value_column().is_none());
+ assert_eq!(unshredded.len(), 2);
+ }
+
/// Writes a variant to a parquet file and ensures the parquet logical type
/// annotation is correct
#[test]
