This is an automated email from the ASF dual-hosted git repository.
mbrobbel pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
The following commit(s) were added to refs/heads/main by this push:
new 378e9c256e [Variant] Make VariantToArrowRowBuilder an enum (#8345)
378e9c256e is described below
commit 378e9c256ec8c0c72bd060d5a035b69920282db5
Author: Ryan Johnson <[email protected]>
AuthorDate: Mon Sep 15 11:39:09 2025 -0600
[Variant] Make VariantToArrowRowBuilder an enum (#8345)
# Which issue does this PR close?
We generally require a GitHub issue to be filed for all bug fixes and
enhancements and this helps us generate change logs for our releases.
You can link an issue to this PR using the GitHub syntax.
- Closes #NNN.
# Rationale for this change
Enum dispatch in rust is more efficient than virtual method dispatch,
and enums require far less boxing which makes them more memory efficient
as well. `ArrowToVariantRowBuilder` is already an enum, so it makes
sense for `VariantToArrowRowBuilder` to take the same approach.
# What changes are included in this PR?
Replace the trait with an enum.
# Are these changes tested?
Yes, existing row builder tests continue to pass.
# Are there any user-facing changes?
No.
---
parquet-variant-compute/src/variant_to_arrow.rs | 160 ++++++++++++++----------
1 file changed, 95 insertions(+), 65 deletions(-)
diff --git a/parquet-variant-compute/src/variant_to_arrow.rs
b/parquet-variant-compute/src/variant_to_arrow.rs
index 7cb3c4e281..4deeaffe4e 100644
--- a/parquet-variant-compute/src/variant_to_arrow.rs
+++ b/parquet-variant-compute/src/variant_to_arrow.rs
@@ -17,8 +17,7 @@
use arrow::array::{ArrayRef, PrimitiveBuilder};
use arrow::compute::CastOptions;
-use arrow::datatypes;
-use arrow::datatypes::ArrowPrimitiveType;
+use arrow::datatypes::{self, ArrowPrimitiveType, DataType};
use arrow::error::{ArrowError, Result};
use parquet_variant::{Variant, VariantPath};
@@ -27,40 +26,90 @@ use crate::VariantArrayBuilder;
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)
+ Int8(VariantToPrimitiveArrowRowBuilder<'a, datatypes::Int8Type>),
+ Int16(VariantToPrimitiveArrowRowBuilder<'a, datatypes::Int16Type>),
+ Int32(VariantToPrimitiveArrowRowBuilder<'a, datatypes::Int32Type>),
+ Int64(VariantToPrimitiveArrowRowBuilder<'a, datatypes::Int64Type>),
+ Float16(VariantToPrimitiveArrowRowBuilder<'a, datatypes::Float16Type>),
+ Float32(VariantToPrimitiveArrowRowBuilder<'a, datatypes::Float32Type>),
+ Float64(VariantToPrimitiveArrowRowBuilder<'a, datatypes::Float64Type>),
+ BinaryVariant(VariantToBinaryVariantArrowRowBuilder),
+
+ // Path extraction wrapper - contains a boxed enum for any of the above
+ WithPath(VariantPathRowBuilder<'a>),
+}
+
+impl<'a> VariantToArrowRowBuilder<'a> {
+ pub fn append_null(&mut self) -> Result<()> {
+ use VariantToArrowRowBuilder::*;
+ match self {
+ Int8(b) => b.append_null(),
+ Int16(b) => b.append_null(),
+ Int32(b) => b.append_null(),
+ Int64(b) => b.append_null(),
+ Float16(b) => b.append_null(),
+ Float32(b) => b.append_null(),
+ Float64(b) => b.append_null(),
+ BinaryVariant(b) => b.append_null(),
+ WithPath(path_builder) => path_builder.append_null(),
+ }
+ }
+
+ pub fn append_value(&mut self, value: &Variant<'_, '_>) -> Result<bool> {
+ use VariantToArrowRowBuilder::*;
+ match self {
+ Int8(b) => b.append_value(value),
+ Int16(b) => b.append_value(value),
+ Int32(b) => b.append_value(value),
+ Int64(b) => b.append_value(value),
+ Float16(b) => b.append_value(value),
+ Float32(b) => b.append_value(value),
+ Float64(b) => b.append_value(value),
+ BinaryVariant(b) => b.append_value(value),
+ WithPath(path_builder) => path_builder.append_value(value),
+ }
+ }
+
+ pub fn finish(&mut self) -> Result<ArrayRef> {
+ use VariantToArrowRowBuilder::*;
+ match self {
+ Int8(b) => b.finish(),
+ Int16(b) => b.finish(),
+ Int32(b) => b.finish(),
+ Int64(b) => b.finish(),
+ Float16(b) => b.finish(),
+ Float32(b) => b.finish(),
+ Float64(b) => b.finish(),
+ BinaryVariant(b) => b.finish(),
+ WithPath(path_builder) => path_builder.finish(),
+ }
+ }
+}
+
pub(crate) fn make_variant_to_arrow_row_builder<'a>(
//metadata: &BinaryViewArray,
path: VariantPath<'a>,
- data_type: Option<&'a datatypes::DataType>,
+ data_type: Option<&'a DataType>,
cast_options: &'a CastOptions,
-) -> Result<Box<dyn VariantToArrowRowBuilder + 'a>> {
- use datatypes::{
- Float16Type, Float32Type, Float64Type, Int16Type, Int32Type,
Int64Type, Int8Type,
- };
+) -> Result<VariantToArrowRowBuilder<'a>> {
+ use VariantToArrowRowBuilder::*;
- let builder = match data_type {
+ let mut builder = match data_type {
// If no data type was requested, build an unshredded VariantArray.
- None => VariantToBinaryVariantArrowRowBuilder::new(16).with_path(path),
- Some(datatypes::DataType::Int8) => {
-
VariantToPrimitiveArrowRowBuilder::<Int8Type>::new(cast_options).with_path(path)
- }
- Some(datatypes::DataType::Int16) => {
-
VariantToPrimitiveArrowRowBuilder::<Int16Type>::new(cast_options).with_path(path)
- }
- Some(datatypes::DataType::Int32) => {
-
VariantToPrimitiveArrowRowBuilder::<Int32Type>::new(cast_options).with_path(path)
- }
- Some(datatypes::DataType::Int64) => {
-
VariantToPrimitiveArrowRowBuilder::<Int64Type>::new(cast_options).with_path(path)
- }
- Some(datatypes::DataType::Float16) => {
-
VariantToPrimitiveArrowRowBuilder::<Float16Type>::new(cast_options).with_path(path)
- }
- Some(datatypes::DataType::Float32) => {
-
VariantToPrimitiveArrowRowBuilder::<Float32Type>::new(cast_options).with_path(path)
- }
- Some(datatypes::DataType::Float64) => {
-
VariantToPrimitiveArrowRowBuilder::<Float64Type>::new(cast_options).with_path(path)
- }
+ None => BinaryVariant(VariantToBinaryVariantArrowRowBuilder::new(16)),
+ Some(DataType::Int8) =>
Int8(VariantToPrimitiveArrowRowBuilder::new(cast_options)),
+ Some(DataType::Int16) =>
Int16(VariantToPrimitiveArrowRowBuilder::new(cast_options)),
+ Some(DataType::Int32) =>
Int32(VariantToPrimitiveArrowRowBuilder::new(cast_options)),
+ Some(DataType::Int64) =>
Int64(VariantToPrimitiveArrowRowBuilder::new(cast_options)),
+ Some(DataType::Float16) =>
Float16(VariantToPrimitiveArrowRowBuilder::new(cast_options)),
+ Some(DataType::Float32) =>
Float32(VariantToPrimitiveArrowRowBuilder::new(cast_options)),
+ Some(DataType::Float64) =>
Float64(VariantToPrimitiveArrowRowBuilder::new(cast_options)),
_ => {
return Err(ArrowError::NotYetImplemented(format!(
"variant_get with path={:?} and data_type={:?} not yet
implemented",
@@ -68,46 +117,26 @@ pub(crate) fn make_variant_to_arrow_row_builder<'a>(
)));
}
};
- Ok(builder)
-}
-/// 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) trait VariantToArrowRowBuilder {
- fn append_null(&mut self) -> Result<()>;
-
- fn append_value(&mut self, value: &Variant<'_, '_>) -> Result<bool>;
+ // Wrap with path extraction if needed
+ if !path.is_empty() {
+ builder = WithPath(VariantPathRowBuilder {
+ builder: Box::new(builder),
+ path,
+ })
+ };
- fn finish(&mut self) -> Result<ArrayRef>;
+ Ok(builder)
}
/// A thin wrapper whose only job is to extract a specific path from a variant
value and pass the
/// result to a nested builder.
-struct VariantPathRowBuilder<'a, T: VariantToArrowRowBuilder> {
- builder: T,
+pub(crate) struct VariantPathRowBuilder<'a> {
+ builder: Box<VariantToArrowRowBuilder<'a>>,
path: VariantPath<'a>,
}
-trait VariantToArrowRowBuilderWithPath<'a>: VariantToArrowRowBuilder {
- fn with_path(self, path: VariantPath<'a>) -> Box<dyn
VariantToArrowRowBuilder + 'a>;
-}
-
-impl<'a, T: VariantToArrowRowBuilder + 'a>
VariantToArrowRowBuilderWithPath<'a> for T {
- fn with_path(self, path: VariantPath<'a>) -> Box<dyn
VariantToArrowRowBuilder + 'a> {
- if path.is_empty() {
- Box::new(self)
- } else {
- Box::new(VariantPathRowBuilder {
- builder: self,
- path,
- })
- }
- }
-}
-
-impl<T: VariantToArrowRowBuilder> VariantToArrowRowBuilder for
VariantPathRowBuilder<'_, T> {
+impl<'a> VariantPathRowBuilder<'a> {
fn append_null(&mut self) -> Result<()> {
self.builder.append_null()
}
@@ -120,6 +149,7 @@ impl<T: VariantToArrowRowBuilder> VariantToArrowRowBuilder
for VariantPathRowBui
Ok(false)
}
}
+
fn finish(&mut self) -> Result<ArrayRef> {
self.builder.finish()
}
@@ -144,7 +174,7 @@ fn get_type_name<T: ArrowPrimitiveType>() -> &'static str {
}
/// Builder for converting variant values to primitive values
-struct VariantToPrimitiveArrowRowBuilder<'a, T: ArrowPrimitiveType> {
+pub(crate) struct VariantToPrimitiveArrowRowBuilder<'a, T: ArrowPrimitiveType>
{
builder: arrow::array::PrimitiveBuilder<T>,
cast_options: &'a CastOptions<'a>,
}
@@ -158,7 +188,7 @@ impl<'a, T: ArrowPrimitiveType>
VariantToPrimitiveArrowRowBuilder<'a, T> {
}
}
-impl<'a, T> VariantToArrowRowBuilder for VariantToPrimitiveArrowRowBuilder<'a,
T>
+impl<'a, T> VariantToPrimitiveArrowRowBuilder<'a, T>
where
T: ArrowPrimitiveType,
for<'m, 'v> Variant<'m, 'v>: VariantAsPrimitive<T>,
@@ -193,7 +223,7 @@ where
}
/// Builder for creating VariantArray output (for path extraction without type
conversion)
-struct VariantToBinaryVariantArrowRowBuilder {
+pub(crate) struct VariantToBinaryVariantArrowRowBuilder {
builder: VariantArrayBuilder,
}
@@ -205,7 +235,7 @@ impl VariantToBinaryVariantArrowRowBuilder {
}
}
-impl VariantToArrowRowBuilder for VariantToBinaryVariantArrowRowBuilder {
+impl VariantToBinaryVariantArrowRowBuilder {
fn append_null(&mut self) -> Result<()> {
self.builder.append_null();
Ok(())
