scovich commented on code in PR #9572:
URL: https://github.com/apache/arrow-rs/pull/9572#discussion_r2955590463
##########
parquet-variant-compute/src/variant_to_arrow.rs:
##########
@@ -76,12 +80,43 @@ impl<'a> VariantToArrowRowBuilder<'a> {
match self {
Primitive(b) => b.finish(),
Array(b) => b.finish(),
+ Struct(b) => b.finish(),
BinaryVariant(b) => b.finish(),
WithPath(path_builder) => path_builder.finish(),
}
}
}
+fn make_typed_variant_to_arrow_row_builder<'a>(
+ data_type: &'a DataType,
+ cast_options: &'a CastOptions,
+ capacity: usize,
+) -> Result<VariantToArrowRowBuilder<'a>> {
+ use VariantToArrowRowBuilder::*;
+
+ match data_type {
+ DataType::Struct(fields) =>
Ok(Struct(StructVariantToArrowRowBuilder::try_new(
+ fields,
+ cast_options,
+ capacity,
+ )?)),
Review Comment:
nit: consider this as shorter and easier to read?
```suggestion
DataType::Struct(fields) => {
let builder = StructVariantToArrowRowBuilder::try_new(fields,
cast_options, capacity)?;
Ok(Struct(builder))
}
```
##########
parquet-variant-compute/src/variant_to_arrow.rs:
##########
@@ -491,6 +508,80 @@ pub(crate) enum ArrayVariantToArrowRowBuilder<'a> {
LargeListView(VariantToListArrowRowBuilder<'a, i64, true>),
}
+pub(crate) struct StructVariantToArrowRowBuilder<'a> {
+ fields: &'a Fields,
+ field_builders: Vec<VariantToArrowRowBuilder<'a>>,
+ nulls: NullBufferBuilder,
+ cast_options: &'a CastOptions<'a>,
+}
+
+impl<'a> StructVariantToArrowRowBuilder<'a> {
+ fn try_new(
+ fields: &'a Fields,
+ cast_options: &'a CastOptions<'a>,
+ capacity: usize,
+ ) -> Result<Self> {
+ let mut field_builders = Vec::with_capacity(fields.len());
+ for field in fields.iter() {
+ field_builders.push(make_typed_variant_to_arrow_row_builder(
+ field.data_type(),
+ cast_options,
+ capacity,
+ )?);
+ }
+ Ok(Self {
+ fields,
+ field_builders,
+ nulls: NullBufferBuilder::new(capacity),
+ cast_options,
+ })
+ }
+
+ fn append_null(&mut self) -> Result<()> {
+ for builder in &mut self.field_builders {
+ builder.append_null()?;
+ }
+ self.nulls.append_null();
+ Ok(())
+ }
+
+ fn append_value(&mut self, value: &Variant<'_, '_>) -> Result<bool> {
+ let Variant::Object(obj) = value else {
+ if self.cast_options.safe {
+ self.append_null()?;
+ return Ok(false);
+ }
+ return Err(ArrowError::CastError(format!(
+ "Failed to extract struct from variant {:?}",
+ value
+ )));
+ };
+
+ for (index, field) in self.fields.iter().enumerate() {
+ if let Some(field_value) = obj.get(field.name()) {
+ self.field_builders[index].append_value(field_value)?;
+ } else {
+ self.field_builders[index].append_null()?;
+ }
Review Comment:
nit: any reason not to match?
```suggestion
match obj.get(field.name()) {
Some(field_value) =>
self.field_builders[index].append_value(field_value)?,
None => self.field_builders[index].append_null()?,
}
```
##########
parquet-variant-compute/src/variant_get.rs:
##########
@@ -3167,7 +3165,7 @@ mod test {
let string_array: Arc<dyn Array> =
Arc::new(StringArray::from(json_strings));
let variant_array = json_to_variant(&string_array).unwrap();
- // Request nested struct - this should fail at the row builder level
+ // Request nested struct - now supported by the row builder.
Review Comment:
temporal reference, pls remove
--
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]
