scovich commented on code in PR #8366:
URL: https://github.com/apache/arrow-rs/pull/8366#discussion_r2357460801
##########
parquet-variant-compute/src/variant_to_arrow.rs:
##########
@@ -26,28 +26,34 @@ use crate::{VariantArray, VariantValueArrayBuilder};
use std::sync::Arc;
-/// Builder for converting variant values into strongly typed Arrow arrays.
-///
-/// Useful for variant_get kernels that need to extract specific paths from
variant values, possibly
-/// with casting of leaf values to specific types.
-pub(crate) enum VariantToArrowRowBuilder<'a> {
- // Direct builders (no path extraction)
+/// Builder for converting variant values to primitive Arrow arrays. It is
used by both
+/// `VariantToArrowRowBuilder` (below) and
`VariantToShreddedPrimitiveVariantRowBuilder` (in
+/// `shred_variant.rs`).
+pub(crate) enum PrimitiveVariantToArrowRowBuilder<'a> {
Review Comment:
> why not unify them all into a single enum
I started out that way, but both builders handle primitive types separately
from complex types, and the handling of complex types is different in the two
type trees:
* `PrimitiveVariantToArrowRowBuilder`
* `VariantToShreddedPrimitiveVariantRowBuilder::typed_value`
* `VariantToShreddedVariantRowBuilder::Primitive`
* NOTE: The corresponding `Object` enum variant needs to (partially)
shred rows into `value` and `typed_value` columns according to the shredding
schema.
* `VariantToArrowRowBuilder::Primitive`
* NOTE: The corresponding `Object` enum variant would eventually need to
convert rows into structs, blowing up (or producing NULL) if any field has the
wrong type or if there are any unexpected fields.
--
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: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
