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 (338af8a) with develop (2383946)}

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

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

connortsui20

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

connortsui20 · 2026-03-26T17:33:04Z

encodings/parquet-variant/src/array.rs

+    }
+
+    /// Creates a Parquet Variant array with explicit outer validity.
+    pub fn try_new_with_validity(


I feel like this should just be try_new? That would be consistent with a lot of our other canonical array constructors. And if you want to be extra consistent you can pull out a validate method, but that's not strictly necessary.

connortsui20 · 2026-03-26T17:37:30Z

encodings/parquet-variant/src/array.rs

+        if let Some(validity_len) = validity.maybe_len() {
+            vortex_ensure!(
+                validity_len == len,
+                "validity length must match metadata length"
+            );
+        }
+        if let Some(ref v) = value {
+            vortex_ensure!(v.len() == len, "value length must match metadata length");
+        }
+        if let Some(ref tv) = typed_value {
+            vortex_ensure!(
+                tv.len() == len,
+                "typed_value length must match metadata length"
+            );
+        }


vortex_ensure_eq!

We really should just plumb through the codebase and replace all instances of these...

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.

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).

connortsui20 · 2026-03-26T17:57:21Z

encodings/parquet-variant/src/operations.rs

+
+fn scalar_from_variant_storage(
+    metadata: &[u8],
+    value: Option<&vortex_array::ArrayRef>,


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

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 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 added 12 commits March 25, 2026 16:04

New Parquet-Variant encoding

a797536

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

fix

26b2f68

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

dt/ts types

40a95b4

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

Split mega lib file

87412e2

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

some more stuff

ef8eaf1

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

More tests and docs

1cf1862

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

Initial work

140be1c

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

more things

a9cc0b1

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

doc fix

1102147

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

Clear up nullability

5a2028b

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

more tests

b4ae19e

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

take/slice/filter kernels

f63e092

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

AdamGS force-pushed the adamg/parquet-variant branch from 510a8c8 to e5a799e Compare March 25, 2026 16:06

try build wasm

338af8a

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

AdamGS force-pushed the adamg/parquet-variant branch 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

		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!

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!

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!

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

connortsui20 Mar 26, 2026 •

edited

Loading