scovich commented on code in PR #8446:
URL: https://github.com/apache/arrow-rs/pull/8446#discussion_r2379841177
##########
parquet-variant-compute/src/variant_array.rs:
##########
@@ -901,65 +934,119 @@ impl StructArrayBuilder {
}
}
+/// A simple wrapper that provides VariantBuilderExt for building a single
variant value
+///
+/// This is used specifically by VariantArray::value to build a single variant
from shredded data
+/// without the complexity of array-level state management.
+struct SingleValueVariantBuilder<'m> {
+ value_builder: ValueBuilder,
+ metadata_builder: ReadOnlyMetadataBuilder<'m>,
+}
+
+impl<'m> SingleValueVariantBuilder<'m> {
+ fn new(metadata: VariantMetadata<'m>) -> Self {
+ Self {
+ value_builder: ValueBuilder::new(),
+ metadata_builder: ReadOnlyMetadataBuilder::new(metadata),
+ }
+ }
+
+ fn into_bytes(self) -> Vec<u8> {
+ self.value_builder.into_inner()
+ }
+
+ fn parent_state(&mut self) -> ParentState<'_, ()> {
+ ParentState::variant(&mut self.value_builder, &mut
self.metadata_builder)
+ }
+}
+
+impl VariantBuilderExt for SingleValueVariantBuilder<'_> {
+ type State<'a>
+ = ()
+ where
+ Self: 'a;
+
+ fn append_null(&mut self) {
+ self.value_builder.append_null();
+ }
+
+ fn append_value<'m, 'v>(&mut self, value: impl Into<Variant<'m, 'v>>) {
+ ValueBuilder::append_variant_bytes(self.parent_state(), value.into());
+ }
+
+ fn try_new_list(&mut self) -> Result<ListBuilder<'_, Self::State<'_>>,
ArrowError> {
+ Ok(ListBuilder::new(self.parent_state(), false))
+ }
+
+ fn try_new_object(&mut self) -> Result<ObjectBuilder<'_, Self::State<'_>>,
ArrowError> {
+ Ok(ObjectBuilder::new(self.parent_state(), false))
+ }
+}
+
/// returns the non-null element at index as a Variant
-fn typed_value_to_variant<'a>(
- typed_value: &'a ArrayRef,
+fn typed_value_to_variant(
+ typed_value: &ArrayRef,
value: Option<&BinaryViewArray>,
index: usize,
-) -> VariantArrayValue<'a, 'a> {
- let data_type = typed_value.data_type();
- if value.is_some_and(|v| !matches!(data_type, DataType::Struct(_)) &&
v.is_valid(index)) {
- // Only a partially shredded struct is allowed to have values for both
columns
- panic!("Invalid variant, conflicting value and typed_value");
- }
- let value = match data_type {
+ metadata: &VariantMetadata,
+ builder: &mut impl VariantBuilderExt,
+) -> Result<(), ArrowError> {
+ match typed_value.data_type() {
DataType::Boolean => {
let boolean_array = typed_value.as_boolean();
let value = boolean_array.value(index);
- Variant::from(value)
+ builder.append_value(value);
}
DataType::Date32 => {
let array = typed_value.as_primitive::<Date32Type>();
let value = array.value(index);
let date = Date32Type::to_naive_date(value);
- Variant::from(date)
+ builder.append_value(date);
}
// 16-byte FixedSizeBinary alway corresponds to a UUID; all other
sizes are illegal.
DataType::FixedSizeBinary(16) => {
let array = typed_value.as_fixed_size_binary();
let value = array.value(index);
- Uuid::from_slice(value).unwrap().into() // unwrap is safe: slice
is always 16 bytes
+ let value = Uuid::from_slice(value).unwrap(); // unwrap safety:
slice is always 16 bytes
+ builder.append_value(value);
}
DataType::BinaryView => {
let array = typed_value.as_binary_view();
let value = array.value(index);
- Variant::from(value)
+ builder.append_value(value);
}
DataType::Utf8 => {
let array = typed_value.as_string::<i32>();
let value = array.value(index);
- Variant::from(value)
+ builder.append_value(value);
}
DataType::Int8 => {
- primitive_conversion_single_value!(Int8Type, typed_value, index)
+ let variant = primitive_conversion_single_value!(Int8Type,
typed_value, index);
+ builder.append_value(variant);
}
DataType::Int16 => {
- primitive_conversion_single_value!(Int16Type, typed_value, index)
+ let variant = primitive_conversion_single_value!(Int16Type,
typed_value, index);
+ builder.append_value(variant);
}
DataType::Int32 => {
- primitive_conversion_single_value!(Int32Type, typed_value, index)
+ let variant = primitive_conversion_single_value!(Int32Type,
typed_value, index);
+ builder.append_value(variant);
}
DataType::Int64 => {
- primitive_conversion_single_value!(Int64Type, typed_value, index)
- }
- DataType::Float16 => {
- primitive_conversion_single_value!(Float16Type, typed_value, index)
Review Comment:
* https://github.com/apache/arrow-rs/pull/8438 missed this case
--
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]