scovich commented on code in PR #9631:
URL: https://github.com/apache/arrow-rs/pull/9631#discussion_r3015998805
##########
parquet-variant-compute/src/variant_to_arrow.rs:
##########
@@ -585,10 +585,17 @@ impl<'a> StructVariantToArrowRowBuilder<'a> {
}
impl<'a> ArrayVariantToArrowRowBuilder<'a> {
+ /// Creates a new list builder for the given data type.
+ ///
+ /// # Arguments
+ /// * `shredded` - If true, element builders produce shredded structs with
`value`/`typed_value`
+ /// fields (for [`crate::shred_variant()`]). If false, element builders
produce strongly typed
+ /// arrays directly (for [`crate::variant_get()`]).
pub(crate) fn try_new(
Review Comment:
It looks like this `shredded` param allows quite a bit of code reuse. Should
we look at other shred-vs-get pairs to see if similar consolidation is
possible? Struct, in particular, is likely to be complex?
##########
parquet-variant-compute/src/variant_to_arrow.rs:
##########
@@ -886,13 +894,45 @@ impl<'a> VariantToUuidArrowRowBuilder<'a> {
}
}
+/// Element builder for list variants, supporting both typed (for
[`crate::variant_get()`])
+/// and shredded (for [`crate::shred_variant()`]) output modes.
+enum ListElementBuilder<'a> {
+ /// Produces the target array type directly.
+ Typed(Box<VariantToArrowRowBuilder<'a>>),
+ /// Produces a shredded struct with `value` and `typed_value` fields.
+ Shredded(Box<VariantToShreddedVariantRowBuilder<'a>>),
Review Comment:
Any particular reason these need to be boxed? They're fully strongly typed,
no?
--
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]
