[FIX] fix merge conflict issues

Author: carpecodeum

parquet-variant-compute/src/variant_array.rs

@@ -105,11 +105,26 @@ impl VariantArray {
             )));
         };
 
+        // Extract value and typed_value fields
+        let value = if let Some(value_col) = inner.column_by_name("value") {
+            if let Some(binary_view) = value_col.as_binary_view_opt() {
+                Some(binary_view.clone())
+            } else {
+                return Err(ArrowError::NotYetImplemented(format!(
+                    "VariantArray 'value' field must be BinaryView, got {}",
+                    value_col.data_type()
+                )));
+            }
+        } else {
+            None
+        };
+        let typed_value = inner.column_by_name("typed_value").cloned();
+
         // Note these clones are cheap, they just bump the ref count
         Ok(Self {
             inner: inner.clone(),
             metadata: metadata.clone(),
-            shredding_state: ShreddingState::try_new(inner)?,
+            shredding_state: ShreddingState::try_new(metadata.clone(), value, typed_value)?,
         })
     }
 
@@ -135,7 +150,7 @@ impl VariantArray {
         // This would be a lot simpler if ShreddingState were just a pair of Option... we already
         // have everything we need.
         let inner = builder.build();
-        let shredding_state = ShreddingState::try_new(&inner).unwrap(); // valid by construction
+        let shredding_state = ShreddingState::try_new(metadata.clone(), value, typed_value).unwrap(); // valid by construction
         Self {
             inner,
             metadata,
@@ -200,7 +215,8 @@ impl VariantArray {
                     typed_value_to_variant(typed_value, index)
                 }
             }
-            ShreddingState::AllNull { .. } => {
+            (None, None) => {
+                // AllNull case: neither value nor typed_value fields exist
                 // NOTE: This handles the case where neither value nor typed_value fields exist.
                 // For top-level variants, this returns Variant::Null (JSON null).
                 // For shredded object fields, this technically should indicate SQL NULL,
@@ -296,11 +312,18 @@ impl ShreddedVariantFieldArray {
             ));
         };
 
+        // Extract value and typed_value fields (metadata is not expected in ShreddedVariantFieldArray)
+        let value = inner_struct.column_by_name("value").and_then(|col| col.as_binary_view_opt().cloned());
+        let typed_value = inner_struct.column_by_name("typed_value").cloned();
+        
+        // Use a dummy metadata for the constructor (ShreddedVariantFieldArray doesn't have metadata)
+        let dummy_metadata = arrow::array::BinaryViewArray::new_null(inner_struct.len());
+
         // Note this clone is cheap, it just bumps the ref count
         let inner = inner_struct.clone();
         Ok(Self {
             inner: inner.clone(),
-            shredding_state: ShreddingState::try_new(&inner)?,
+            shredding_state: ShreddingState::try_new(dummy_metadata, value, typed_value)?,
         })
     }
 
@@ -398,114 +421,36 @@ impl Array for ShreddedVariantFieldArray {
 ///
 /// [Parquet Variant Shredding Spec]: https://github.com/apache/parquet-format/blob/master/VariantShredding.md#value-shredding
 #[derive(Debug)]
-pub enum ShreddingState {
-    /// This variant has no typed_value field
-    Unshredded { value: BinaryViewArray },
-    /// This variant has a typed_value field and no value field
-    /// meaning it is the shredded type
-    PerfectlyShredded { typed_value: ArrayRef },
-    /// Imperfectly shredded: Shredded values reside in `typed_value` while those that failed to
-    /// shred reside in `value`. Missing field values are NULL in both columns, while NULL primitive
-    /// values have NULL `typed_value` and `Variant::Null` in `value`.
-    ///
-    /// NOTE: A partially shredded struct is a special kind of imperfect shredding, where
-    /// `typed_value` and `value` are both non-NULL. The `typed_value` is a struct containing the
-    /// subset of fields for which shredding was attempted (each field will then have its own value
-    /// and/or typed_value sub-fields that indicate how shredding actually turned out). Meanwhile,
-    /// the `value` is a variant object containing the subset of fields for which shredding was
-    /// not even attempted.
-    ImperfectlyShredded {
-        value: BinaryViewArray,
-        typed_value: ArrayRef,
-    },
-    /// All values are null, only metadata is present.
-    ///
-    /// This state occurs when neither `value` nor `typed_value` fields exist in the schema.
-    /// Note: By strict spec interpretation, this should only be valid for shredded object fields,
-    /// not top-level variants. However, we allow it and treat as Variant::Null for pragmatic
-    /// handling of missing data.
-    AllNull { metadata: BinaryViewArray },
+pub struct ShreddingState {
+    pub value: Option<BinaryViewArray>,
+    pub typed_value: Option<ArrayRef>,
 }
 
 impl ShreddingState {
     /// try to create a new `ShreddingState` from the given fields
     pub fn try_new(
-        metadata: BinaryViewArray,
+        _metadata: BinaryViewArray,
         value: Option<BinaryViewArray>,
         typed_value: Option<ArrayRef>,
     ) -> Result<Self, ArrowError> {
-        match (metadata, value, typed_value) {
-            (metadata, Some(value), Some(typed_value)) => Ok(Self::PartiallyShredded {
-                metadata,
-                value,
-                typed_value,
-            }),
-            (metadata, Some(value), None) => Ok(Self::Unshredded { metadata, value }),
-            (metadata, None, Some(typed_value)) => Ok(Self::Typed {
-                metadata,
-                typed_value,
-            }),
-            (metadata, None, None) => Ok(Self::AllNull { metadata }),
-        }
-    }
-
-    /// Return a reference to the metadata field
-    pub fn metadata_field(&self) -> &BinaryViewArray {
-        match self {
-            ShreddingState::Unshredded { metadata, .. } => metadata,
-            ShreddingState::Typed { metadata, .. } => metadata,
-            ShreddingState::PartiallyShredded { metadata, .. } => metadata,
-            ShreddingState::AllNull { metadata } => metadata,
-        }
-
         Ok(Self { value, typed_value })
     }
 
     /// Return a reference to the value field, if present
     pub fn value_field(&self) -> Option<&BinaryViewArray> {
-        match self {
-            ShreddingState::Unshredded { value, .. } => Some(value),
-            ShreddingState::Typed { .. } => None,
-            ShreddingState::PartiallyShredded { value, .. } => Some(value),
-            ShreddingState::AllNull { .. } => None,
-        }
+        self.value.as_ref()
     }
 
     /// Return a reference to the typed_value field, if present
     pub fn typed_value_field(&self) -> Option<&ArrayRef> {
-        match self {
-            ShreddingState::Unshredded { .. } => None,
-            ShreddingState::Typed { typed_value, .. } => Some(typed_value),
-            ShreddingState::PartiallyShredded { typed_value, .. } => Some(typed_value),
-            ShreddingState::AllNull { .. } => None,
-        }
+        self.typed_value.as_ref()
     }
 
     /// Slice all the underlying arrays
     pub fn slice(&self, offset: usize, length: usize) -> Self {
-        match self {
-            ShreddingState::Unshredded { value } => ShreddingState::Unshredded {
-                value: value.slice(offset, length),
-            },
-            ShreddingState::Typed {
-                metadata,
-                typed_value,
-            } => ShreddingState::Typed {
-                metadata: metadata.slice(offset, length),
-                typed_value: typed_value.slice(offset, length),
-            },
-            ShreddingState::PartiallyShredded {
-                metadata,
-                value,
-                typed_value,
-            } => ShreddingState::PartiallyShredded {
-                metadata: metadata.slice(offset, length),
-                value: value.slice(offset, length),
-                typed_value: typed_value.slice(offset, length),
-            },
-            ShreddingState::AllNull { metadata } => ShreddingState::AllNull {
-                metadata: metadata.slice(offset, length),
-            },
+        Self {
+            value: self.value.as_ref().map(|v| v.slice(offset, length)),
+            typed_value: self.typed_value.as_ref().map(|tv| tv.slice(offset, length)),
         }
     }
 }
@@ -664,11 +609,10 @@ mod test {
         // This is a pragmatic decision to handle missing data gracefully.
         let variant_array = VariantArray::try_new(Arc::new(array)).unwrap();
 
-        // Verify the shredding state is AllNull
-        assert!(matches!(
-            variant_array.shredding_state(),
-            ShreddingState::AllNull { .. }
-        ));
+        // Verify the shredding state is AllNull (both value and typed_value are None)
+        let shredding_state = variant_array.shredding_state();
+        assert!(shredding_state.value_field().is_none());
+        assert!(shredding_state.typed_value_field().is_none());
 
         // Verify that value() returns Variant::Null (compensating for spec violation)
         for i in 0..variant_array.len() {
@@ -727,13 +671,11 @@ mod test {
         let metadata = BinaryViewArray::from(vec![b"test" as &[u8]]);
         let shredding_state = ShreddingState::try_new(metadata.clone(), None, None).unwrap();
 
-        assert!(matches!(shredding_state, ShreddingState::AllNull { .. }));
+        // Verify the shredding state is AllNull (both value and typed_value are None)
+        assert!(shredding_state.value_field().is_none());
+        assert!(shredding_state.typed_value_field().is_none());
 
-        // Verify metadata is preserved correctly
-        if let ShreddingState::AllNull { metadata: m } = shredding_state {
-            assert_eq!(m.len(), metadata.len());
-            assert_eq!(m.value(0), metadata.value(0));
-        }
+        // Note: metadata is no longer stored in the struct, it's passed to try_new but not stored
     }
 
     #[test]
@@ -746,11 +688,10 @@ mod test {
 
         let variant_array = VariantArray::try_new(Arc::new(struct_array)).unwrap();
 
-        // Verify the shredding state is AllNull
-        assert!(matches!(
-            variant_array.shredding_state(),
-            ShreddingState::AllNull { .. }
-        ));
+        // Verify the shredding state is AllNull (both value and typed_value are None)
+        let shredding_state = variant_array.shredding_state();
+        assert!(shredding_state.value_field().is_none());
+        assert!(shredding_state.typed_value_field().is_none());
 
         // Verify all values are null
         assert_eq!(variant_array.len(), 3);
@@ -797,9 +738,8 @@ mod test {
         let variant_array = VariantArray::try_new(Arc::new(struct_array)).unwrap();
 
         // This should be Unshredded, not AllNull, because value field exists in schema
-        assert!(matches!(
-            variant_array.shredding_state(),
-            ShreddingState::Unshredded { .. }
-        ));
+        let shredding_state = variant_array.shredding_state();
+        assert!(shredding_state.value_field().is_some());
+        assert!(shredding_state.typed_value_field().is_none());
     }
 }