New Parquet-Variant encoding by AdamGS · Pull Request #7130 · vortex-data/vortex

AdamGS · 2026-03-23T15:31:27Z

Summary

This PR introduces a new encoding to support Arrow's canonical extension type, but does not integrate it with anything else.

It does include basic pieces like slice/take/filter, and it can (or at least should) roundtrip with the equivalent arrow array.

Testing

The encoding includes a bunch of basic tests, both for making sure it roundtrips with arrow and for the various nullability cases.

codspeed-hq · 2026-03-23T16:25:59Z

Merging this PR will not alter performance

✅ 1106 untouched benchmarks
⏩ 1522 skipped benchmarks¹

_{Comparing adamg/parquet-variant (04c60a8) with develop (b524763)}

1522 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

encodings/parquet-variant/src/lib.rs

Cargo.toml

a10y · 2026-03-23T21:33:51Z

It looks like this supports one typed_value per column. How would you represent something like this example, which shreds out multiple nested columns from an object?

Physically I think this differs from shredding out a struct column with two fields.

AdamGS · 2026-03-24T10:07:59Z

I think in that case it is a struct, because they are named

AdamGS · 2026-03-24T11:41:23Z

typed_value will be a struct (I think that's "object" in variant terms), but not directly of the event_type and event_ts types, but of ParquetVariant arrays that, but roughly:

Struct {
    "event_ts": Variant {
        ..,
        typed_value: Timestamp
    },
    "event_type": Variant {
        ..,
        typed_value: String
    },
}

## Summary Filling up missing parts of making Variant a canonical array. It still not fully supported but this is a step towards it. These changes started as part of #7130, but I figured they are just noise in that already too big of a PR. ## API Changes Makes variant officially a canonical array and type. --------- Signed-off-by: Adam Gutglick <adam@spiraldb.com>

encodings/parquet-variant/Cargo.toml

encodings/parquet-variant/src/lib.rs

Cargo.toml

.github/workflows/ci.yml

connortsui20

Looks good! I gave it a quick pass but haven't looked at the scalar operations yet

encodings/parquet-variant/src/array.rs

connortsui20 · 2026-03-26T17:40:24Z

encodings/parquet-variant/src/array.rs

+                    Validity::from(BitBuffer::from(nulls.inner().clone()))
+                }
+            })
+            .unwrap_or(Validity::NonNullable);


Is this guaranteed on the arrow side that None implies non-nullable? I feel like I remember running some oddness around this before...

Maybe worth a comment if this is totally fine

Edit: Actually looking down below it seems like both Validity::NonNullable | Validity::AllValid => None so I'm not actually sure what to do here?

Edit again: We have this code in vortex-array/src/arrow/convert.rs, I think we must pass nullability into from_arrow_variant to make this correct?

fn nulls(nulls: Option<&NullBuffer>, nullable: bool) -> Validity { if nullable { nulls .map(|nulls| { if nulls.null_count() == nulls.len() { Validity::AllInvalid } else { Validity::from(BitBuffer::from(nulls.inner().clone())) } }) .unwrap_or_else(|| Validity::AllValid) } else { assert!(nulls.map(|x| x.null_count() == 0).unwrap_or(true)); Validity::NonNullable } }

connortsui20 · 2026-03-26T17:43:42Z

encodings/parquet-variant/src/array.rs

+        let pv =
+            ParquetVariantArray::try_new_with_validity(validity, metadata, value, typed_value)?;


Yeah I think it makes sense to just have try_new that takes validity, and also we could consider having a new_unchecked that does no validation here instead since we trust that the arrow array is well-formed (lengths are correct).

connortsui20 · 2026-03-26T17:45:59Z

encodings/parquet-variant/src/array.rs

+        let nulls = match &self.validity {
+            Validity::NonNullable | Validity::AllValid => None,
+            Validity::AllInvalid => Some(NullBuffer::new_null(len)),
+            Validity::Array(array) => {
+                let mask = array.clone().execute::<Mask>(ctx)?;
+                match mask {
+                    Mask::AllTrue(_) => None,
+                    Mask::AllFalse(l) => Some(NullBuffer::new_null(l)),
+                    Mask::Values(values) => Some(NullBuffer::from(
+                        arrow_buffer::BooleanBuffer::from(values.bit_buffer().clone()),
+                    )),
+                }
+            }
+        };


We have use vortex::array::arrow::executor::validity::to_arrow_null_buffer; for this which is pub(super) right now but arguably should be pub.

will take a look

connortsui20 · 2026-03-26T17:51:06Z

encodings/parquet-variant/src/kernel.rs

+            ParquetVariantArray::try_new_with_validity(validity, metadata, value, typed_value)?
+                .into_array(),


Do we care about an unchecked constructor? If we're just validating lengths it's probably not worth it...

connortsui20 · 2026-03-26T17:53:54Z

encodings/parquet-variant/src/vtable.rs

+        match (&array.value, &other.value) {
+            (Some(a), Some(b)) => {
+                if !a.array_eq(b, precision) {
+                    return false;
+                }
+            }
+            (None, None) => {}
+            _ => return false,
+        }
+        match (&array.typed_value, &other.typed_value) {
+            (Some(a), Some(b)) => a.array_eq(b, precision),
+            (None, None) => true,
+            _ => false,
+        }
+    }


Do we want to do array equality via structural equality or value equality? Since we can have different representations for the same data we might want to just do a loop of scalar_at in here (after the validity and metadata checks).

encodings/parquet-variant/src/operations.rs

connortsui20 · 2026-03-26T17:59:36Z

encodings/parquet-variant/src/array.rs

+    pub(crate) value: Option<ArrayRef>,
+    pub(crate) typed_value: Option<ArrayRef>,


Crazy idea after a quick pass over everything, is it worth having a dedicated enum that enforces at least 1 of these exist? There's a lot of logic that has to check for that everywhere (including in some free standing functions). Maybe that's overkill though and introduces more complexity than it is worth?

Definitely a good idea

I was re-reading the arrow implementation, and I think all four states are actually valid there. Both will be missing when you declare a shredding schema with a field that doesn't exist it the actual data.

They acknowledge that its not a strict adherence to the spec, not sure what to do about this for now.

Signed-off-by: Adam Gutglick <adam@spiraldb.com>

AdamGS added the changelog/feature A new feature label Mar 23, 2026

AdamGS force-pushed the adamg/parquet-variant branch 2 times, most recently from 62280a5 to f266c90 Compare March 23, 2026 16:21

a10y reviewed Mar 23, 2026

View reviewed changes

encodings/parquet-variant/src/lib.rs Outdated Show resolved Hide resolved

a10y reviewed Mar 23, 2026

View reviewed changes

Cargo.toml Outdated Show resolved Hide resolved

AdamGS force-pushed the adamg/parquet-variant branch 3 times, most recently from 3d7ecb4 to ad93dcf Compare March 24, 2026 13:28

AdamGS requested review from a10y, connortsui20 and gatesn and removed request for a10y, connortsui20 and gatesn March 24, 2026 15:33

AdamGS mentioned this pull request Mar 24, 2026

Part 2 of making Variant a canonical array #7143

Merged

gatesn reviewed Mar 24, 2026

View reviewed changes

encodings/parquet-variant/Cargo.toml Show resolved Hide resolved

encodings/parquet-variant/src/lib.rs Show resolved Hide resolved

AdamGS force-pushed the adamg/parquet-variant branch 2 times, most recently from 6e32d0f to fbf4d4e Compare March 25, 2026 12:36

AdamGS commented Mar 25, 2026

View reviewed changes

Cargo.toml Show resolved Hide resolved

AdamGS marked this pull request as ready for review March 25, 2026 14:18

AdamGS force-pushed the adamg/parquet-variant branch from fbf4d4e to 4848c82 Compare March 25, 2026 14:18

AdamGS requested review from a10y, gatesn and robert3005 March 25, 2026 14:19

gatesn reviewed Mar 25, 2026

View reviewed changes

.github/workflows/ci.yml Outdated Show resolved Hide resolved

AdamGS force-pushed the adamg/parquet-variant branch 2 times, most recently from e5a799e to 338af8a Compare March 25, 2026 16:07

AdamGS requested a review from connortsui20 March 26, 2026 14:52

connortsui20 reviewed Mar 26, 2026

View reviewed changes

AdamGS force-pushed the adamg/parquet-variant branch from 338af8a to 004e388 Compare March 27, 2026 13:11

New parquet-variant Vortex encoding

863435c

Signed-off-by: Adam Gutglick <adam@spiraldb.com>

AdamGS force-pushed the adamg/parquet-variant branch from 004e388 to 863435c Compare March 27, 2026 13:19

CR comments

04c60a8

Signed-off-by: Adam Gutglick <adam@spiraldb.com>

		let pv =
		ParquetVariantArray::try_new_with_validity(validity, metadata, value, typed_value)?;

		ParquetVariantArray::try_new_with_validity(validity, metadata, value, typed_value)?
		.into_array(),

		pub(crate) value: Option<ArrayRef>,
		pub(crate) typed_value: Option<ArrayRef>,

Conversation

AdamGS commented Mar 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Testing

Uh oh!

codspeed-hq bot commented Mar 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will not alter performance

Footnotes

Uh oh!

Uh oh!

Uh oh!

a10y commented Mar 23, 2026

Uh oh!

AdamGS commented Mar 24, 2026

Uh oh!

AdamGS commented Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

connortsui20 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

connortsui20 Mar 26, 2026 • 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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AdamGS Mar 27, 2026 • 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

AdamGS commented Mar 23, 2026 •

edited

Loading

codspeed-hq bot commented Mar 23, 2026 •

edited

Loading

AdamGS commented Mar 24, 2026 •

edited

Loading

connortsui20 Mar 26, 2026 •

edited

Loading

AdamGS Mar 27, 2026 •

edited

Loading