From ea264c4464eda65571f4e35a3f6c9b84b18afe4d Mon Sep 17 00:00:00 2001 From: Matthew Kim <38759997+friendlymatthew@users.noreply.github.com> Date: Wed, 15 Oct 2025 10:16:20 -0400 Subject: [PATCH 1/3] Remove ceremony from iterator of variants into VariantArray --- parquet-variant-compute/src/variant_array.rs | 60 ++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/parquet-variant-compute/src/variant_array.rs b/parquet-variant-compute/src/variant_array.rs index 5f7f826819d8..b4ff9f78b50a 100644 --- a/parquet-variant-compute/src/variant_array.rs +++ b/parquet-variant-compute/src/variant_array.rs @@ -17,6 +17,7 @@ //! [`VariantArray`] implementation +use crate::VariantArrayBuilder; use crate::type_conversion::{generic_conversion_single_value, primitive_conversion_single_value}; use arrow::array::{Array, ArrayRef, AsArray, BinaryViewArray, StructArray}; use arrow::buffer::NullBuffer; @@ -439,6 +440,20 @@ impl From for ArrayRef { } } +impl<'m, 'v> FromIterator>> for VariantArray { + fn from_iter>>>(iter: T) -> Self { + let mut b = VariantArrayBuilder::new(0); + b.extend(iter); + b.build() + } +} + +impl<'m, 'v> FromIterator> for VariantArray { + fn from_iter>>(iter: T) -> Self { + Self::from_iter(iter.into_iter().map(Some)) + } +} + /// An iterator over [`VariantArray`] /// /// This iterator returns `Option>>` where: @@ -1141,6 +1156,7 @@ mod test { use super::*; use arrow::array::{BinaryViewArray, Int32Array}; use arrow_schema::{Field, Fields}; + use parquet_variant::ShortString; #[test] fn invalid_not_a_struct_array() { @@ -1405,4 +1421,48 @@ mod test { assert!(i.next().is_none()); assert!(i.next_back().is_none()); } + + #[test] + fn test_from_variant_opts_into_variant_array() { + let v = vec![None, Some(Variant::Null), Some(Variant::BooleanFalse), None]; + + let variant_array = VariantArray::from_iter(v); + + assert_eq!(variant_array.len(), 4); + + assert!(variant_array.is_null(0)); + + assert!(!variant_array.is_null(1)); + assert_eq!(variant_array.value(1), Variant::Null); + + assert!(!variant_array.is_null(2)); + assert_eq!(variant_array.value(2), Variant::BooleanFalse); + + assert!(variant_array.is_null(3)); + } + + #[test] + fn test_from_variants_into_variant_array() { + let v = vec![ + Variant::Null, + Variant::BooleanFalse, + Variant::ShortString(ShortString::try_new("norm").unwrap()), + ]; + + let variant_array = VariantArray::from_iter(v); + + assert_eq!(variant_array.len(), 3); + + assert!(!variant_array.is_null(0)); + assert_eq!(variant_array.value(0), Variant::Null); + + assert!(!variant_array.is_null(1)); + assert_eq!(variant_array.value(1), Variant::BooleanFalse); + + assert!(!variant_array.is_null(3)); + assert_eq!( + variant_array.value(2), + Variant::ShortString(ShortString::try_new("norm").unwrap()) + ); + } } From 39d9eaa1d1664a26b49b0c15b0126cdf2e22885c Mon Sep 17 00:00:00 2001 From: Matthew Kim <38759997+friendlymatthew@users.noreply.github.com> Date: Wed, 15 Oct 2025 10:48:25 -0400 Subject: [PATCH 2/3] Impl PartialEq for VariantArray --- parquet-variant-compute/src/variant_array.rs | 33 ++++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/parquet-variant-compute/src/variant_array.rs b/parquet-variant-compute/src/variant_array.rs index b4ff9f78b50a..af356eb997f6 100644 --- a/parquet-variant-compute/src/variant_array.rs +++ b/parquet-variant-compute/src/variant_array.rs @@ -208,7 +208,7 @@ impl ExtensionType for VariantType { /// assert_eq!(variant_array.value(0), Variant::from("such wow")); /// ``` /// -#[derive(Clone, Debug)] +#[derive(Debug, Clone, PartialEq)] pub struct VariantArray { /// Reference to the underlying StructArray inner: StructArray, @@ -739,7 +739,7 @@ impl From for StructArray { /// (partial shredding). /// /// [Parquet Variant Shredding Spec]: https://github.com/apache/parquet-format/blob/master/VariantShredding.md#value-shredding -#[derive(Clone, Debug)] +#[derive(Debug, Clone, PartialEq)] pub struct ShreddingState { value: Option, typed_value: Option, @@ -1465,4 +1465,33 @@ mod test { Variant::ShortString(ShortString::try_new("norm").unwrap()) ); } + + #[test] + fn test_variant_equality() { + let v_iter = [None, Some(Variant::BooleanFalse), Some(Variant::Null), None]; + let v = VariantArray::from_iter(v_iter.clone()); + + { + let v_copy = v.clone(); + assert_eq!(v, v_copy); + } + + { + let v_iter_reversed = { + let mut v = v_iter.clone(); + v.reverse(); + + v + }; + + let v_reversed = VariantArray::from_iter(v_iter_reversed); + + assert_ne!(v, v_reversed); + } + + { + let v_sliced = v.slice(0, 1); + assert_ne!(v, v_sliced); + } + } } From 13ca9fd9524605ba716b5026ce24790278de42ba Mon Sep 17 00:00:00 2001 From: Matthew Kim <38759997+friendlymatthew@users.noreply.github.com> Date: Thu, 16 Oct 2025 14:55:22 -0400 Subject: [PATCH 3/3] Nits --- parquet-variant-compute/src/variant_array.rs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/parquet-variant-compute/src/variant_array.rs b/parquet-variant-compute/src/variant_array.rs index 977f87c14952..2b38081d07e2 100644 --- a/parquet-variant-compute/src/variant_array.rs +++ b/parquet-variant-compute/src/variant_array.rs @@ -1479,13 +1479,7 @@ mod test { } { - let v_iter_reversed = { - let mut v = v_iter.clone(); - v.reverse(); - - v - }; - + let v_iter_reversed = v_iter.iter().cloned().rev(); let v_reversed = VariantArray::from_iter(v_iter_reversed); assert_ne!(v, v_reversed);