alamb commented on code in PR #8021:
URL: https://github.com/apache/arrow-rs/pull/8021#discussion_r2256914173
##########
parquet-variant-compute/src/variant_array.rs:
##########
@@ -135,36 +140,189 @@ impl VariantArray {
self.inner
}
+ /// Return the shredding state of this `VariantArray`
+ pub fn shredding_state(&self) -> &ShreddingState {
+ &self.shredding_state
+ }
+
/// Return the [`Variant`] instance stored at the given row
///
- /// Panics if the index is out of bounds.
+ /// Consistently with other Arrow arrays types, this API requires you to
+ /// check for nulls first using [`Self::is_valid`].
+ ///
+ /// # Panics
+ /// * if the index is out of bounds
+ /// * if the array value is null
+ ///
+ /// If this is a shredded variant but has no value at the shredded
location, it
+ /// will return [`Variant::Null`].
+ ///
+ ///
+ /// # Performance Note
+ ///
+ /// This is certainly not the most efficient way to access values in a
+ /// `VariantArray`, but it is useful for testing and debugging.
///
/// Note: Does not do deep validation of the [`Variant`], so it is up to
the
/// caller to ensure that the metadata and value were constructed
correctly.
pub fn value(&self, index: usize) -> Variant {
- let metadata = self.metadata_field().as_binary_view().value(index);
- let value = self.value_field().as_binary_view().value(index);
- Variant::new(metadata, value)
+ match &self.shredding_state {
+ ShreddingState::Unshredded { metadata, value } => {
+ Variant::new(metadata.value(index), value.value(index))
+ }
+ ShreddingState::FullyShredded {
+ metadata: _,
+ typed_value,
+ } => {
+ if typed_value.is_null(index) {
+ Variant::Null
+ } else {
+ typed_value_to_variant(typed_value, index)
+ }
+ }
+ ShreddingState::PartiallyShredded {
+ metadata,
+ value,
+ typed_value,
+ } => {
+ if typed_value.is_null(index) {
+ Variant::new(metadata.value(index), value.value(index))
+ } else {
+ typed_value_to_variant(typed_value, index)
+ }
+ }
+ }
}
- fn find_metadata_field(array: &StructArray) -> Option<ArrayRef> {
- array.column_by_name("metadata").cloned()
+ /// Return a reference to the metadata field of the [`StructArray`]
+ pub fn metadata_field(&self) -> &BinaryViewArray {
+ self.shredding_state.metadata_field()
}
- fn find_value_field(array: &StructArray) -> Option<ArrayRef> {
- array.column_by_name("value").cloned()
+ /// Return a reference to the value field of the `StructArray`
+ pub fn value_field(&self) -> Option<&BinaryViewArray> {
+ self.shredding_state.value_field()
}
- /// Return a reference to the metadata field of the [`StructArray`]
- pub fn metadata_field(&self) -> &ArrayRef {
- // spec says fields order is not guaranteed, so we search by name
- &self.metadata_ref
+ /// Return a reference to the typed_value field of the `StructArray`, if
present
+ pub fn typed_value_field(&self) -> Option<&ArrayRef> {
+ self.shredding_state.typed_value_field()
}
+}
- /// Return a reference to the value field of the `StructArray`
- pub fn value_field(&self) -> &ArrayRef {
- // spec says fields order is not guaranteed, so we search by name
- &self.value_ref
+/// Variant arrays can be shredded in one of three states, encoded here
+#[derive(Debug)]
+pub enum ShreddingState {
+ /// This variant has no typed_value field
+ Unshredded {
+ metadata: BinaryViewArray,
+ value: BinaryViewArray,
+ },
+ /// This variant has a typed_value field and no value field
+ /// meaning it is fully shredded (aka the value is stored in typed_value)
+ FullyShredded {
+ metadata: BinaryViewArray,
+ typed_value: ArrayRef,
+ },
+ /// This variant has both a value field and a typed_value field
+ /// meaning it is partially shredded: first the typed_value is used, and
+ /// if that is null, the value field is used.
+ PartiallyShredded {
+ metadata: BinaryViewArray,
+ value: BinaryViewArray,
+ typed_value: ArrayRef,
+ },
+}
+
+impl ShreddingState {
+ /// try to create a new `ShreddingState` from the given fields
+ pub fn try_new(
+ 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::FullyShredded {
+ metadata,
+ typed_value,
+ }),
+ (_metadata_field, None, None) =>
Err(ArrowError::InvalidArgumentError(String::from(
+ "VariantArray has neither value nor typed_value field",
+ ))),
+ }
+ }
+
+ /// Return a reference to the metadata field
+ pub fn metadata_field(&self) -> &BinaryViewArray {
+ match self {
+ ShreddingState::Unshredded { metadata, .. } => metadata,
+ ShreddingState::FullyShredded { metadata, .. } => metadata,
+ ShreddingState::PartiallyShredded { metadata, .. } => metadata,
+ }
+ }
+
+ /// Return a reference to the value field, if present
+ pub fn value_field(&self) -> Option<&BinaryViewArray> {
+ match self {
+ ShreddingState::Unshredded { value, .. } => Some(value),
+ ShreddingState::FullyShredded { .. } => None,
+ ShreddingState::PartiallyShredded { value, .. } => Some(value),
+ }
+ }
+
+ /// Return a reference to the typed_value field, if present
+ pub fn typed_value_field(&self) -> Option<&ArrayRef> {
+ match self {
+ ShreddingState::Unshredded { .. } => None,
+ ShreddingState::FullyShredded { typed_value, .. } =>
Some(typed_value),
+ ShreddingState::PartiallyShredded { typed_value, .. } =>
Some(typed_value),
+ }
+ }
+
+ /// Slice all the underlying arrays
+ pub fn slice(&self, offset: usize, length: usize) -> Self {
+ match self {
+ ShreddingState::Unshredded { metadata, value } =>
ShreddingState::Unshredded {
+ metadata: metadata.slice(offset, length),
+ value: value.slice(offset, length),
+ },
+ ShreddingState::FullyShredded {
+ metadata,
+ typed_value,
+ } => ShreddingState::FullyShredded {
+ 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),
+ },
+ }
+ }
+}
+
+/// returns the non-null element at index as a Variant
+fn typed_value_to_variant(typed_value: &ArrayRef, index: usize) -> Variant {
Review Comment:
other Arrow Array apis don't return `Option`, they instead return the value
directly requiring you to check `is_null` instead -- see
https://docs.rs/arrow/latest/arrow/array/struct.PrimitiveArray.html#method.value
for example
```rust
/// Consistently with other Arrow arrays types, this API requires you to
/// check for nulls first using [`Self::is_valid`].
```
I think the reason it to allow a better chance at LLVM vectorizing the code,
which I don't think is likely relevant here.
We could make the Variant API deviate from this pattern (it is already
different in several other ways) and return `Option<Variant>`.
--
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]
