Initial Builder API for Creating Variant Values #7653

PinkCrow007 · 2025-06-13T06:37:57Z

Which issue does this PR close?

Rationale for this change

This PR introduces a basic builder API for creating Variant values, building on the foundation laid by @mkarbo. The builder provides a user-friendly nested API while maintaining performance through a single-buffer design. The design was shaped with huge help from @alamb, @scovich and @Weijun-H ’s feedback, and draws much inspiration from the excellent work by @zeroshade

This is an initial version and does not yet support nested values, metadata key sorting, and so on

What changes are included in this PR?

Adds VariantBuilder, ObjectBuilder, ArrayBuilder

Are there any user-facing changes?

The new API's added in parquet-variant will be user facing.

alamb · 2025-06-13T17:04:35Z

It also looks like there are some CI failures on this PR:

RAT means you have to put the apache license at the start of the file
Clippy means run cargo clippy -p parquet-variant
The test seems to be failing when you run cargo test -p parquet-variant

alamb

Thank you @PinkCrow007 -- this is looking like a great start

Overall

I tried out the APIs and wrote some docs and I think this API works well.

Suggested next steps

In my opinion, since we are early in this project we should "move fast and bias to action" -- thus, what I would suggest is:

Get this PR so it passes the CI tests (so we can merge it in)
Address any comments you have tome to
Merge it in

Then we can iterate / add additional tests / features / coverage as needed.

Suggestions for Docs:

One thing that helps me with these early days of API design / development is to try and write examples out of common things people want to do with the builders.

That often helps clarify API design points and has the added bonus of better code.

I took a shot at creating an example in the following PR (that targets your branch, so merging it will update this PR):

PinkCrow007#1

We can also merge this PR without the change and I'll make a new PR targeting this repo

FYI @scovich @mkarbo @superserious-dev

parquet-variant/src/builder.rs

alamb · 2025-06-13T16:43:07Z

parquet-variant/tests/variant_interop.rs

 }

+#[test]
+fn variant_primitive_builder() -> Result<(), ArrowError> {


this is neat

alamb · 2025-06-13T16:45:07Z

parquet-variant/src/variant.rs


+impl<'m, 'v> From<()> for Variant<'m, 'v> {
+    fn from(_: ()) -> Self {
+        Variant::Null


Ideally this should not return Null but the actual variant value

I think with the change in this PR you could implement this API directly

[Variant] Simplify creation of Variants from metadata and value #7663

parquet-variant/src/builder.rs

Add documentation and examples to VariantBuilder

scovich

Initial pass

parquet-variant/src/builder.rs

scovich · 2025-06-14T16:22:22Z

parquet-variant/src/builder.rs

+/// object_builder.append_value("last_name", "Li");
+/// object_builder.finish();
+/// // Finish the builder to get the metadata and value
+/// let (metadata, value) = builder.finish();


In order to deal with nested data efficiently, we will need a separate metadata builder that multiple object builders can make use of. Otherwise, we would need to merge multiple metadata dictionaries, which would also require rewriting the corresponding field ids. Very complex and error-prone.

We should also give some thought into how to build sorted metadata dictionaries. It probably would require making two passes -- one to identify the string values and sort them into a variant metadata; and another to build the variant value that references the string ids?

Here is the solution that @zeroshade suggested:

Initial Builder API for Creating Variant Values #7653 (comment)

Supporting nested data is tracked here:

[Variant] Support Nested Data in VariantBuilder #7696

parquet-variant/src/builder.rs

scovich · 2025-06-14T16:33:39Z

parquet-variant/src/builder.rs

+    fn append_decimal4(&mut self, integer: i32, scale: u8) {
+        self.buffer
+            .push(primitive_header(VariantPrimitiveType::Decimal4));
+        self.buffer.push(scale);


Should we validate the scale before pushing it?

We probably should (or at least have optional checking 🤔 )

scovich · 2025-06-14T16:34:33Z

parquet-variant/src/builder.rs

+        self.buffer.extend_from_slice(&micros.to_le_bytes());
+    }
+
+    fn append_decimal4(&mut self, integer: i32, scale: u8) {


nit, to match the variant spec:

Suggested change

fn append_decimal4(&mut self, integer: i32, scale: u8) {

fn append_decimal4(&mut self, unscaled_value: i32, scale: u8) {

(more below)

scovich · 2025-06-14T16:39:24Z

parquet-variant/src/builder.rs

+            Entry::Vacant(entry) => {
+                let id = self.dict_keys.len() as u32;
+                entry.insert(id);
+                self.dict_keys.push(key.to_string());


It's a bit unfortunate we have to double-allocate strings. Ideally the dictionary could track &str backed by dict_keys, but I don't know how to manage mutability and lifetimes to achieve that...

Maybe the dictionary could be a map to u32 (an index into the relevant entry in the dict_keys Vec) instead of a String

That could work. Also, we ultimately need a single packed array of utf8 bytes; so we might want to consider storing the same kinds of offsets that the actual variant metadata stores, where each string's bytes are found in the range offset[i]..offset[i+1]. That shouldn't be any more expensive to access (sort) vs. an array of (heap-allocated) strings?

Storing offset pairs directly would also work really nicely if we decide not to sort+dedup the metadata field entries (**), because then the Vec<u8> can simply be copied as-is to its final home.

(**) Sort+dedup is actually quite some work, because it requires finding and rewriting all field ids scattered through the value bytes. So it would almost certainly be a second step (either a preliminary pass before we actually generate the variant value bytes, or as a fix-up pass afterward to rewrite field ids)

(**) Sort+dedup is actually quite some work, because it requires finding and rewriting all field ids scattered through the value bytes. So it would almost certainly be a second step (either a preliminary pass before we actually generate the variant value bytes, or as a fix-up pass afterward to rewrite field ids)

@zeroshade described a pretty elegant solution in the Go variant builder which was to write the dictionary fields in the order they are encountered., and then decides if the dictionary is sorted when writing it out. The builder has an additional API that you can use to directly add a dictionary key

One benefit of this approach is that different writers can decide if they care about sorted dictionaries and if they do they can do a pre-pass to figure them out and add them directly to the builder

Tracking with

[Variant] Improve VariantBuilder when creating field name dictionaries / sorted dictionaries #7698

scovich · 2025-06-14T19:08:17Z

parquet-variant/src/builder.rs

+        (metadata, self.buffer)
+    }
+
+    pub fn append_value<T: Into<Variant<'static, 'static>>>(&mut self, value: T) {


Can we adjust lifetimes, so normal strings can be appended and not just string constants?

Suggested change

pub fn append_value<T: Into<Variant<'static, 'static>>>(&mut self, value: T) {

pub fn append_value<'v, T: Into<Variant<'_, 'v>>>(&mut self, value: T) {

I did so in d7d1449

scovich · 2025-06-14T19:09:27Z

parquet-variant/src/builder.rs

+    pub fn new_array(&mut self) -> ArrayBuilder {
+        ArrayBuilder::new(self)
+    }
+
+    /// Create an [`ObjectBuilder`] for creating [`Variant::Object`] values.
+    ///
+    /// See the examples on [`VariantBuilder`] for usage.
+    pub fn new_object(&mut self) -> ObjectBuilder {


Arrays and objects are tricky to build incrementally. The value offset and field id arrays require either knowing the number of elements/fields to be created in advance (and then worrying about what happens if the caller builds too many/few entries afterward), or building the arrays in separate storage and then moving an arbitrarily large number of buffered bytes to make room for the them after the fact.

I guess this PR takes the latter approach, but for deeply nested data that will lead to something that looks like quadratic cost -- O(num_bytes * depth) -- as the deepest fields get moved repeatedly while finishing each of the parent builders in turn.

Not sure the best solution here...

Maybe the builder can leave room for the list length and then append the values, and then go back and update the length when the list is finsihed

This would get tricky for building "large" lists as the length field may not be known upfront.

we could also introduce potentially a functon like new_large_object() or something for callers to hint up front their object has many fields, and if they use new_object but push too many values fallback to copying

Tracking in

[Variant] Test and implement efficient building for "large" Arrays #7699

scovich · 2025-06-15T00:43:42Z

parquet-variant/src/builder.rs

+        ObjectBuilder::new(self)
+    }
+
+    pub fn finish(self) -> (Vec<u8>, Vec<u8>) {


I don't think this approach can be easily extended to allow nested variants?

Update: Never mind -- I missed the fact that VariantBuilder is the truly top-level builder, and the array/object builders both nest around it. There's still the problem that variant can't impl From arrays/objects, but I guess we could add a call to create a new object/array builder from an existing one, in order to achieve nesting?

There's still the problem that variant can't impl From arrays/objects, but I guess we could add a call to create a new object/array builder from an existing one, in order to achieve nesting?

I am not sure we will be able to implement a general purpose From impl for native rust slices or objects. We could potentially offer a serde style thing (like serde_json but serde_variant 🤔 )

scovich · 2025-06-15T00:45:51Z

parquet-variant/src/builder.rs

+            Variant::Float(v) => self.append_float(v),
+            Variant::Double(v) => self.append_double(v),
+            Variant::Binary(v) => self.append_binary(v),
+            Variant::String(s) | Variant::ShortString(s) => self.append_string(s),


this will make three times deciding between short vs. long string? one to make the input, another here, and a third to consume it? Any way we could make this more direct/efficient?

I think it would be nice to have a self.append_short_string and self.append_string and then call the appropriate ones. We could have self.append_short_string return an error potentially as well if the string was too long

Tracking with

[Variant] More efficient determination of String vs ShortString #7700

alamb

I took the liberty of merging up from main to resolve conflicts and updating the API t get the CI passing

alamb

I went through this PR and comments again and I think it looks good enough to merge

There are several outstanding questions, that I think we can address as follow on PRs (I will file tickets)

Supporting nested objects / lists
Error checking for variants that have restricted values (e.g. binary that is longer than u32::MAX, valid scale for decimals, etc)
improving performance of dictionary creation / handling sorted dictionaries

But all in all I think this is a great step forward if I do say so my self.

Thank you @scovich and @PinkCrow007

🚀

alamb · 2025-06-17T20:20:38Z

parquet-variant/src/builder.rs

+            .extend_from_slice(&(value.len() as u32).to_le_bytes());
+        self.buffer.extend_from_slice(value);


I think we should throw an error if value.len() > u32::max

alamb · 2025-06-17T20:24:03Z

parquet-variant/src/builder.rs

+    pub fn new_array(&mut self) -> ArrayBuilder {
+        ArrayBuilder::new(self)
+    }
+
+    /// Create an [`ObjectBuilder`] for creating [`Variant::Object`] values.
+    ///
+    /// See the examples on [`VariantBuilder`] for usage.
+    pub fn new_object(&mut self) -> ObjectBuilder {


Maybe the builder can leave room for the list length and then append the values, and then go back and update the length when the list is finsihed

This would get tricky for building "large" lists as the length field may not be known upfront.

we could also introduce potentially a functon like new_large_object() or something for callers to hint up front their object has many fields, and if they use new_object but push too many values fallback to copying

alamb · 2025-06-17T20:27:21Z

parquet-variant/src/builder.rs

+    fn append_decimal4(&mut self, integer: i32, scale: u8) {
+        self.buffer
+            .push(primitive_header(VariantPrimitiveType::Decimal4));
+        self.buffer.push(scale);


We probably should (or at least have optional checking 🤔 )

alamb · 2025-06-17T20:34:09Z

parquet-variant/src/builder.rs

+        ObjectBuilder::new(self)
+    }
+
+    pub fn finish(self) -> (Vec<u8>, Vec<u8>) {


There's still the problem that variant can't impl From arrays/objects, but I guess we could add a call to create a new object/array builder from an existing one, in order to achieve nesting?

I am not sure we will be able to implement a general purpose From impl for native rust slices or objects. We could potentially offer a serde style thing (like serde_json but serde_variant 🤔 )

alamb · 2025-06-17T20:36:51Z

parquet-variant/src/builder.rs

+            Variant::Binary(v) => self.append_binary(v),
+            Variant::String(s) | Variant::ShortString(s) => self.append_string(s),
+            Variant::Object(_) | Variant::List(_) => {
+                unreachable!("Object and Array variants cannot be created through Into<Variant>")


FWIW I think you can append a variant object. I will write a test / file a ticket

Filed [Variant] Panic when appending Object or List to VariantBuilder #7701

alamb · 2025-06-17T20:39:50Z

parquet-variant/src/builder.rs

+            Variant::Float(v) => self.append_float(v),
+            Variant::Double(v) => self.append_double(v),
+            Variant::Binary(v) => self.append_binary(v),
+            Variant::String(s) | Variant::ShortString(s) => self.append_string(s),


I think it would be nice to have a self.append_short_string and self.append_string and then call the appropriate ones. We could have self.append_short_string return an error potentially as well if the string was too long

alamb · 2025-06-17T20:41:22Z

parquet-variant/src/builder.rs

+        (metadata, self.buffer)
+    }
+
+    pub fn append_value<T: Into<Variant<'static, 'static>>>(&mut self, value: T) {


I did so in d7d1449

alamb · 2025-06-17T20:58:27Z

I also updated the naming to match the change to rename Variant::Array to Variant::List

The more I see that the less I like it -- I will file a separate ticket

alamb · 2025-06-17T22:03:07Z

All right -- I have filed a bunch of follow on tickets, and attached them to

[EPIC] [Parquet] Implement Variant type support in Parquet #6736

Once I get the CI to pass I'll plan to merge this PR

Thanks again @PinkCrow007 and @scovich

PinkCrow007 · 2025-06-18T06:10:27Z

Huge thanks @alamb @scovich — I’m honestly really touched by all the care and thought you put into this. I learned so much from your improvements and comments. Really appreciate you helping push it forward! I’ll keep working on the follow-ups and keep learning from you!

PinkCrow007 added 4 commits June 12, 2025 08:45

init

2a8bdbc

correct non-primitive

a03baa0

unify append

c4ffce2

refine

fecca4e

github-actions bot added the parquet Changes to the parquet crate label Jun 13, 2025

PinkCrow007 and others added 2 commits June 13, 2025 08:16

minor fix

ff2f23f

Add documentation and examples to VariantBuilder

37cecc5

alamb reviewed Jun 13, 2025

View reviewed changes

PinkCrow007 and others added 5 commits June 13, 2025 17:24

Merge pull request #1 from alamb/alamb/variant_builder_docs

044e4f5

Add documentation and examples to VariantBuilder

Merge; Apply suggestions

87773c1

Merge branch 'my-recovered-builder' into variant-builder

e4f80f0

fix

c58df81

minor

9625962

scovich reviewed Jun 15, 2025

View reviewed changes

This was referenced Jun 15, 2025

[EPIC] [Parquet] Implement Variant type support in Parquet #6736

Closed

[Variant] Implement read support for remaining primitive types #7644

Merged

minor fix

968486e

alamb mentioned this pull request Jun 16, 2025

Add documentation and examples to VariantBuilder PinkCrow007/arrow-rs#1

Merged

alamb added 3 commits June 17, 2025 16:07

Fix logical and textual conflicts

59ee7cb

update examples

39aa887

Fix test

18ba160

alamb reviewed Jun 17, 2025

View reviewed changes

Fix lifetimes

d7d1449

alamb approved these changes Jun 17, 2025

View reviewed changes

Fix docs and update naming

2235cb8

This was referenced Jun 17, 2025

[Variant] Support Nested Data in VariantBuilder #7696

Closed

[Variant] Add input validation in VariantBuilder #7697

Closed

[Variant] Improve VariantBuilder when creating field name dictionaries / sorted dictionaries #7698

Closed

This was referenced Jun 17, 2025

[Variant] Test and implement efficient building for "large" Arrays #7699

Closed

[Variant] More efficient determination of String vs ShortString #7700

Closed

alamb added 2 commits June 17, 2025 17:59

fix example

eba876f

Fix msrv

b11dc53

alamb merged commit 56ac4dc into apache:main Jun 17, 2025
13 checks passed

This was referenced Jun 18, 2025

Make variant iterators safely infallible #7704

Merged

Variant: Write Variant Values as JSON #7670

Merged

[Variant]: Rust API to Create Variant Values #7424

Closed

	fn append_decimal4(&mut self, integer: i32, scale: u8) {
	fn append_decimal4(&mut self, unscaled_value: i32, scale: u8) {

	pub fn append_value<T: Into<Variant<'static, 'static>>>(&mut self, value: T) {
	pub fn append_value<'v, T: Into<Variant<'_, 'v>>>(&mut self, value: T) {

		.extend_from_slice(&(value.len() as u32).to_le_bytes());
		self.buffer.extend_from_slice(value);

Initial Builder API for Creating Variant Values #7653

Initial Builder API for Creating Variant Values #7653

Conversation

PinkCrow007 commented Jun 13, 2025 • edited by alamb Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are there any user-facing changes?

Uh oh!

alamb commented Jun 13, 2025

Uh oh!

alamb left a comment

Choose a reason for hiding this comment

Overall

Suggested next steps

Suggestions for Docs:

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

scovich left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alamb Jun 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

scovich Jun 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PinkCrow007 commented Jun 13, 2025 •

edited by alamb

Loading

alamb Jun 16, 2025 •

edited

Loading

scovich Jun 16, 2025 •

edited

Loading