Skip to content

Commit 2824638

Browse files
authored
[Variant] Remove boilerplate from make_shredding_row_builder (#8322)
# 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 The type handling code is replicated twice in `make_shredding_row_builder` -- for empty and non-empty paths, respectively. The code is virtually identical in both cases, and constructing any one row builder will always need two branches: (1) check for empty path; and (2) dispatch on the correct type. It also uses a macro to reduce boilerplate a bit. # What changes are included in this PR? Replace the macro with a new extension trait, and swap the control flow: (1) dispatch on the correct type; and (2) check for empty path. This cuts the affected code to 1/3 its original size. # Are these changes tested? Existing unit tests cover it. # Are there any user-facing changes? No.
1 parent 567f441 commit 2824638

File tree

1 file changed

+44
-122
lines changed

1 file changed

+44
-122
lines changed

parquet-variant-compute/src/variant_get/output/row_builder.rs

Lines changed: 44 additions & 122 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
// specific language governing permissions and limitations
1616
// under the License.
1717

18-
use arrow::array::ArrayRef;
18+
use arrow::array::{ArrayRef, PrimitiveBuilder};
1919
use arrow::compute::CastOptions;
2020
use arrow::datatypes;
2121
use arrow::datatypes::ArrowPrimitiveType;
@@ -32,146 +32,42 @@ pub(crate) fn make_shredding_row_builder<'a>(
3232
data_type: Option<&'a datatypes::DataType>,
3333
cast_options: &'a CastOptions,
3434
) -> Result<Box<dyn VariantShreddingRowBuilder + 'a>> {
35-
use arrow::array::PrimitiveBuilder;
3635
use datatypes::{
3736
Float16Type, Float32Type, Float64Type, Int16Type, Int32Type, Int64Type, Int8Type,
3837
};
3938

40-
// support non-empty paths (field access) and some empty path cases
41-
if path.is_empty() {
42-
return match data_type {
43-
Some(datatypes::DataType::Int8) => {
44-
let builder = PrimitiveVariantShreddingRowBuilder {
45-
builder: PrimitiveBuilder::<Int8Type>::new(),
46-
cast_options,
47-
};
48-
Ok(Box::new(builder))
49-
}
50-
Some(datatypes::DataType::Int16) => {
51-
let builder = PrimitiveVariantShreddingRowBuilder {
52-
builder: PrimitiveBuilder::<Int16Type>::new(),
53-
cast_options,
54-
};
55-
Ok(Box::new(builder))
56-
}
57-
Some(datatypes::DataType::Int32) => {
58-
let builder = PrimitiveVariantShreddingRowBuilder {
59-
builder: PrimitiveBuilder::<Int32Type>::new(),
60-
cast_options,
61-
};
62-
Ok(Box::new(builder))
63-
}
64-
Some(datatypes::DataType::Int64) => {
65-
let builder = PrimitiveVariantShreddingRowBuilder {
66-
builder: PrimitiveBuilder::<Int64Type>::new(),
67-
cast_options,
68-
};
69-
Ok(Box::new(builder))
70-
}
71-
Some(datatypes::DataType::Float16) => {
72-
let builder = PrimitiveVariantShreddingRowBuilder {
73-
builder: PrimitiveBuilder::<Float16Type>::new(),
74-
cast_options,
75-
};
76-
Ok(Box::new(builder))
77-
}
78-
Some(datatypes::DataType::Float32) => {
79-
let builder = PrimitiveVariantShreddingRowBuilder {
80-
builder: PrimitiveBuilder::<Float32Type>::new(),
81-
cast_options,
82-
};
83-
Ok(Box::new(builder))
84-
}
85-
Some(datatypes::DataType::Float64) => {
86-
let builder = PrimitiveVariantShreddingRowBuilder {
87-
builder: PrimitiveBuilder::<Float64Type>::new(),
88-
cast_options,
89-
};
90-
Ok(Box::new(builder))
91-
}
92-
None => {
93-
// Return VariantArrayBuilder for VariantArray output
94-
let builder = VariantArrayShreddingRowBuilder::new(16);
95-
Ok(Box::new(builder))
96-
}
97-
_ => Err(ArrowError::NotYetImplemented(format!(
98-
"variant_get with empty path and data_type={:?} not yet implemented",
99-
data_type
100-
))),
101-
};
102-
}
103-
104-
// Non-empty paths: field access functionality
105-
// Helper macro to reduce duplication when wrapping builders with path functionality
106-
macro_rules! wrap_with_path {
107-
($inner_builder:expr) => {
108-
Ok(Box::new(VariantPathRowBuilder {
109-
builder: $inner_builder,
110-
path,
111-
}) as Box<dyn VariantShreddingRowBuilder + 'a>)
112-
};
113-
}
114-
115-
match data_type {
39+
let builder = match data_type {
40+
// If no data type was requested, build an unshredded VariantArray.
41+
None => VariantArrayShreddingRowBuilder::new(16).with_path(path),
11642
Some(datatypes::DataType::Int8) => {
117-
let inner_builder = PrimitiveVariantShreddingRowBuilder {
118-
builder: PrimitiveBuilder::<Int8Type>::new(),
119-
cast_options,
120-
};
121-
wrap_with_path!(inner_builder)
43+
PrimitiveVariantShreddingRowBuilder::<Int8Type>::new(cast_options).with_path(path)
12244
}
12345
Some(datatypes::DataType::Int16) => {
124-
let inner_builder = PrimitiveVariantShreddingRowBuilder {
125-
builder: PrimitiveBuilder::<Int16Type>::new(),
126-
cast_options,
127-
};
128-
wrap_with_path!(inner_builder)
46+
PrimitiveVariantShreddingRowBuilder::<Int16Type>::new(cast_options).with_path(path)
12947
}
13048
Some(datatypes::DataType::Int32) => {
131-
let inner_builder = PrimitiveVariantShreddingRowBuilder {
132-
builder: PrimitiveBuilder::<Int32Type>::new(),
133-
cast_options,
134-
};
135-
wrap_with_path!(inner_builder)
49+
PrimitiveVariantShreddingRowBuilder::<Int32Type>::new(cast_options).with_path(path)
13650
}
13751
Some(datatypes::DataType::Int64) => {
138-
let inner_builder = PrimitiveVariantShreddingRowBuilder {
139-
builder: PrimitiveBuilder::<Int64Type>::new(),
140-
cast_options,
141-
};
142-
wrap_with_path!(inner_builder)
52+
PrimitiveVariantShreddingRowBuilder::<Int64Type>::new(cast_options).with_path(path)
14353
}
14454
Some(datatypes::DataType::Float16) => {
145-
let inner_builder = PrimitiveVariantShreddingRowBuilder {
146-
builder: PrimitiveBuilder::<Float16Type>::new(),
147-
cast_options,
148-
};
149-
wrap_with_path!(inner_builder)
55+
PrimitiveVariantShreddingRowBuilder::<Float16Type>::new(cast_options).with_path(path)
15056
}
15157
Some(datatypes::DataType::Float32) => {
152-
let inner_builder = PrimitiveVariantShreddingRowBuilder {
153-
builder: PrimitiveBuilder::<Float32Type>::new(),
154-
cast_options,
155-
};
156-
wrap_with_path!(inner_builder)
58+
PrimitiveVariantShreddingRowBuilder::<Float32Type>::new(cast_options).with_path(path)
15759
}
15860
Some(datatypes::DataType::Float64) => {
159-
let inner_builder = PrimitiveVariantShreddingRowBuilder {
160-
builder: PrimitiveBuilder::<Float64Type>::new(),
161-
cast_options,
162-
};
163-
wrap_with_path!(inner_builder)
61+
PrimitiveVariantShreddingRowBuilder::<Float64Type>::new(cast_options).with_path(path)
16462
}
165-
None => {
166-
// Create a variant array builder and wrap it with path functionality
167-
let inner_builder = VariantArrayShreddingRowBuilder::new(16);
168-
wrap_with_path!(inner_builder)
63+
_ => {
64+
return Err(ArrowError::NotYetImplemented(format!(
65+
"variant_get with path={:?} and data_type={:?} not yet implemented",
66+
path, data_type
67+
)));
16968
}
170-
_ => Err(ArrowError::NotYetImplemented(format!(
171-
"variant_get with path={:?} and data_type={:?} not yet implemented",
172-
path, data_type
173-
))),
174-
}
69+
};
70+
Ok(builder)
17571
}
17672

17773
/// Builder for shredding variant values into strongly typed Arrow arrays.
@@ -193,6 +89,23 @@ struct VariantPathRowBuilder<'a, T: VariantShreddingRowBuilder> {
19389
path: VariantPath<'a>,
19490
}
19591

92+
trait VariantShreddingRowBuilderWithPath<'a>: VariantShreddingRowBuilder {
93+
fn with_path(self, path: VariantPath<'a>) -> Box<dyn VariantShreddingRowBuilder + 'a>;
94+
}
95+
96+
impl<'a, T: VariantShreddingRowBuilder + 'a> VariantShreddingRowBuilderWithPath<'a> for T {
97+
fn with_path(self, path: VariantPath<'a>) -> Box<dyn VariantShreddingRowBuilder + 'a> {
98+
if path.is_empty() {
99+
Box::new(self)
100+
} else {
101+
Box::new(VariantPathRowBuilder {
102+
builder: self,
103+
path,
104+
})
105+
}
106+
}
107+
}
108+
196109
impl<T: VariantShreddingRowBuilder> VariantShreddingRowBuilder for VariantPathRowBuilder<'_, T> {
197110
fn append_null(&mut self) -> Result<()> {
198111
self.builder.append_null()
@@ -276,6 +189,15 @@ struct PrimitiveVariantShreddingRowBuilder<'a, T: ArrowPrimitiveType> {
276189
cast_options: &'a CastOptions<'a>,
277190
}
278191

192+
impl<'a, T: ArrowPrimitiveType> PrimitiveVariantShreddingRowBuilder<'a, T> {
193+
fn new(cast_options: &'a CastOptions<'a>) -> Self {
194+
Self {
195+
builder: PrimitiveBuilder::<T>::new(),
196+
cast_options,
197+
}
198+
}
199+
}
200+
279201
impl<'a, T> VariantShreddingRowBuilder for PrimitiveVariantShreddingRowBuilder<'a, T>
280202
where
281203
T: ArrowPrimitiveType,

0 commit comments

Comments
 (0)