From 1801209564d3266e6e26e53c596c8be0f0d93e85 Mon Sep 17 00:00:00 2001 From: cakeni <211545599+cakeni@users.noreply.github.com> Date: Fri, 28 Aug 2026 15:11:22 +0800 Subject: [PATCH] fix(variant): reject fixed-size list shredding --- parquet-variant-compute/src/shred_variant.rs | 94 +++----------------- 1 file changed, 10 insertions(+), 84 deletions(-) diff --git a/parquet-variant-compute/src/shred_variant.rs b/parquet-variant-compute/src/shred_variant.rs index 751d926d4a3d..a23c4711dfdc 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, @@ -167,6 +166,11 @@ pub(crate) fn make_variant_to_shredded_variant_arrow_row_builder<'a>( )?; VariantToShreddedVariantRowBuilder::Array(typed_value_builder) } + DataType::FixedSizeList(..) => { + return Err(ArrowError::InvalidArgumentError(format!( + "{data_type} is not a valid variant shredding type" + ))); + } // Supported shredded primitive types, see Variant shredding spec: // https://github.com/apache/parquet-format/blob/master/VariantShredding.md#shredded-value-types DataType::Boolean @@ -701,9 +705,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, @@ -1473,6 +1477,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![ @@ -1649,85 +1654,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::() - .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::().unwrap(); - let val0_typed = val0_struct.column_by_name("typed_value").unwrap(); - let val0_ints = val0_typed.as_any().downcast_ref::().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::().unwrap(); - let val2_typed = val2_struct.column_by_name("typed_value").unwrap(); - let val2_ints = val2_typed.as_any().downcast_ref::().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![