Support extracting struct fields as Variant using ExtensionType by codephage2020 · Pull Request #9598 · apache/arrow-rs

codephage2020 · 2026-03-21T01:06:24Z

Which issue does this PR close?

Closes [Variant] Support extracting struct fields as Variant using ExtensionType #9519 .

Rationale for this change

variant_get cannot extract a struct where some fields remain as VariantArrays. Callers must call variant_get per leaf and assemble results manually.

What changes are included in this PR?

Support extracting struct fields as VariantArray via VariantType extension metadata in variant_get, and fix panics on absent fields and silent data loss on non-struct typed values.

Are these changes tested?

Yes — test_struct_extraction_with_variant_fields and test_struct_extraction_missing_variant_field_no_panic.

Are there any user-facing changes?

No breaking changes. Fields marked with VariantType extension metadata are extracted as VariantArrays in struct extraction.

codephage2020 · 2026-03-21T01:21:12Z

@scovich @sdf-jkl @klion26 Feel free to give me some advice when you have time. Thanks.

scovich

Thanks for looking into this. A bunch of questions.

parquet-variant-compute/src/variant_get.rs

codephage2020 · 2026-03-25T03:55:10Z

Thank you for the review. Looks like #9612 needs to be merged first.

parquet-variant-compute/src/variant_get.rs

sdf-jkl · 2026-03-30T18:02:32Z

@codephage2020 #9612 was merged

codephage2020 · 2026-03-31T06:11:15Z

@codephage2020 #9612 was merged

Thanks! I'll pick this up again later tonight.

scovich

This PR is turning out to be more subtle than I initially realized. A couple big points to consider in the comments.

scovich · 2026-03-31T15:10:53Z

parquet-variant-compute/src/variant_get.rs

+                // followed further. Fall back to the value column if present,
+                // otherwise the path is missing.


I don't understand this comment. What value column fallback does it refer to? The only caller of this follow_shredded_path_element function simply creates an all-null array if we return missing_path_step().

Update: I think I finally figured it out... this code is attempting to traverse through a shredded variant object. Say we're following the path x.y.z and the current path step is y:

(L72) The typed_value we start with should ideally be a struct. If it's any other type, then x was not shredded as struct. Two sub-cases:

x.value is present. Some entries may be Variant::Object. In fact, it's possible that every entry is Variant::Object (or Variant::Null), e.g. if the data match the query schema but the wrong shredding schema was applied when writing it out. So it's actually incorrect to blow up here, regardless of JSONpath semantics, because we didn't prove there's a problem yet (have to row-shred x.value and see what it contains).

x.value column is just plain missing. This means the column shredded perfectly as some other type, and we know there are no nulls and no structs. This is currently treated as an error case, but we should really just return missing_path_step() instead (variant_get should follow JSONpath semantics #9606).

(L86) x.typed_value was a struct as expected, but x.typed_value.y doesn't exist => missing_path_step()

(L91) The x.typed_value.y exists, but it's not a struct. This is a malformed data scenario because we're in a shredded variant object and the spec mandates that every field must be a struct (containing value and/or typed_value fields). So the original code was correct to error out upon discovering the problem, regardless of JSONpath semantics (it just failed to explain why it was correct).

(L101) x.typed_value.y is a struct, so we attempt to return a shredding state for its value and/or typed_value fields.

Great analysis, thank you. In a shredded variant object, every field must be a struct containing value and/or typed_value sub-fields per the spec.
A non-struct field here means malformed data, and the original error was correct.

I've reverted to returning an InvalidArgumentError:

let struct_array = field.as_struct_opt().ok_or_else(|| { ArrowError::InvalidArgumentError(format!( "Expected shredded variant struct for field '{}', got {}", name, field.data_type(), )) })?;

The error message now names the field for easier debugging.

I agree with @codephage2020. This is malformed data rather than a JSONPath-handling case.

Spark appears to treat this as malformedVariant as well

What Codex spewed out for me:

Path-extraction route (closest to variant_get path-step behavior):

ParquetRowConverter.end calls assembleVariantStruct(...)
https://github.com/apache/spark/blob/c8695495569e4058a758b111ebc942fc4906494c/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/parquet/ParquetRowConverter.scala#L1036-L1043

assembleVariantStruct calls extractField(...)
https://github.com/apache/spark/blob/c8695495569e4058a758b111ebc942fc4906494c/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/parquet/SparkShreddingUtils.scala#L827-L844

In extractField, a null object-field slot triggers malformed variant
https://github.com/apache/spark/blob/c8695495569e4058a758b111ebc942fc4906494c/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/parquet/SparkShreddingUtils.scala#L795-L799

That throws QueryExecutionErrors.malformedVariant()
https://github.com/apache/spark/blob/c8695495569e4058a758b111ebc942fc4906494c/sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryExecutionErrors.scala#L2995-L2998

Rebuild route (directly via common/variant/.../ShreddingUtils.java):

ParquetRowConverter.end calls assembleVariant(...)
https://github.com/apache/spark/blob/c8695495569e4058a758b111ebc942fc4906494c/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/parquet/ParquetRowConverter.scala#L1039-L1040

assembleVariant calls ShreddingUtils.rebuild(...)
https://github.com/apache/spark/blob/c8695495569e4058a758b111ebc942fc4906494c/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/parquet/SparkShreddingUtils.scala#L821-L823

ShreddingUtils.rebuild throws malformedVariant() on invalid shredded layout
https://github.com/apache/spark/blob/c8695495569e4058a758b111ebc942fc4906494c/common/variant/src/main/java/org/apache/spark/types/variant/ShreddingUtils.java#L49-L63
https://github.com/apache/spark/blob/c8695495569e4058a758b111ebc942fc4906494c/common/variant/src/main/java/org/apache/spark/types/variant/ShreddingUtils.java#L131-L135
https://github.com/apache/spark/blob/c8695495569e4058a758b111ebc942fc4906494c/common/variant/src/main/java/org/apache/spark/types/variant/ShreddingUtils.java#L167-L170

malformedVariant() definition
https://github.com/apache/spark/blob/c8695495569e4058a758b111ebc942fc4906494c/common/variant/src/main/java/org/apache/spark/types/variant/VariantUtil.java#L177-L180

scovich · 2026-04-01T13:48:15Z

parquet-variant-compute/src/variant_get.rs

+            let mut new_field = field
+                .as_ref()
+                .clone()
+                .with_data_type(child.data_type().clone());


Rescuing #9598 (comment) from github oblivion:

Why needed? Doesn't it already have that type?

For example, a VariantArray's internal StructArray has concrete fields like metadata and value, not empty fields. So we need with_data_type to align the field's declared type with the actual child array's type, otherwise StructArray::try_new will reject the mismatch.

I think there are two different scenarios here:

Caller requested a strongly typed struct. Then the input schema and child data type already match. Even for missing columns, the shredding code returns a properly typed array full of nulls, rather than a NullArray. No variant extension type attached.

Caller requested a (possibly shredded) variant. Then the input schema and child data type may not match and we have to re-apply it and re-attach the variant extension type.

Right now, the code treats everything as case 2/, which is extra work but otherwise (mostly?) harmless.

Re "mostly harmless": How do we handle recursive variant shredding? For example, if I construct a shredding schema corresponding to x.y.z::INT, the resulting "happy path" is x.typed_value.y.typed_value.z.typed_value: INT. If I then pass that schema to variant_get, I have specifically requested that specific shredding schema, even if that means shredding or unshred-then-reshredding the underlying data whose own shredding schema doesn't match. But only the top-level column x has the variant extension type. It is incorrect for x.typed_value.y to be annotated as variant. Perhaps we still pass that annotation in order to guide the recursion, but the schema that comes back to the user afterward must not include it.

I'm not sure if we're handling these cases correctly right now?

Good question. VariantType does NOT leak to intermediate fields. Here's why:

The is_variant_field check is driven by the user's requested schema, not by the data's shredding structure. When a user marks field "metadata" with VariantType, we pass as_type = None to shredded_get_path, which returns a VariantArray directly without recursing into the struct branch. So the recursion stops at that level — inner fields of the VariantArray's internal structure never go through the VariantType annotation path.

For case 1 (strongly typed struct), I've also addressed your observation that with_data_type is redundant — strongly typed fields now use the original field directly, since the child data type already matches the requested type.

To make this clearer, I split the handling into two explicit branches:

if is_variant_field { // Update data type + attach VariantType extension // (or build all-null VariantArray when field is absent) } else { // Strongly typed: use original field as-is }

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…riant field handling - Restore InvalidArgumentError for malformed shredded data (non-struct field in shredded variant) - Separate variant vs strongly-typed field handling for clarity - Skip redundant with_data_type for strongly-typed fields - Remove test belonging to upstream PR apache#9599 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

sdf-jkl

Added a few suggestion, but overall LGTM.

parquet-variant-compute/src/variant_get.rs

Add a unit test in parquet-variant-compute/src/variant_get.rs that verifies VariantType field extraction distinguishes between an explicit JSON null (which should yield a present Variant::Null) and a missing field (which should be SQL NULL). Co-Authored-By: Konstantin Tarasov <33369833+sdf-jkl@users.noreply.github.com>

sdf-jkl

LGTM, thanks!

sdf-jkl · 2026-04-09T16:07:36Z

parquet-variant-compute/src/variant_get.rs

-        if target.typed_value_field().is_none() {
+        let has_variant_fields = fields
+            .iter()
+            .any(|f| f.try_extension_type::<VariantType>().is_ok());


A reminder to clean this once #9677 is closed.

scovich

I'm getting all twisted up trying to grok specific requests for shredded or binary variant (vs. "generic" variant that is equivalent to as_field=None). Left a bunch of thoughts but I don't have any good answers at the moment.

scovich · 2026-04-09T14:44:25Z

parquet-variant-compute/src/variant_get.rs

-                // prepared to handle?
                ArrowError::InvalidArgumentError(format!(
-                    "Expected Struct array while following path, got {}",
+                    "Expected shredded variant struct for field '{}', got {}",


Is this perhaps clearer?

Suggested change

"Expected shredded variant struct for field '{}', got {}",

"Physical layout of shredded variant field '{}' should be a struct, got {}",

I'm struggling with the right wording to convey that this particular struct is part of shredding's internal layout, unrelated to the logical data type the user supplied. Similar to how maps are physically represented as structs.

(the code comment is very nice and clear btw -- I'm only worried about this error message)

scovich · 2026-04-09T14:48:48Z

parquet-variant-compute/src/variant_get.rs

-        if target.typed_value_field().is_none() {
+        let has_variant_fields = fields
+            .iter()
+            .any(|f| f.try_extension_type::<VariantType>().is_ok());


This races "silently" with #9677 -- no obvious (physical) merge conflict.
Should we wait for the other to merge and take advantage of the new API?
Or coordinate with @sdf-jkl to merge this PR first and update the other PR to optimize this new call site?

I'm fine either way. Whichever moves first, the other will follow.

scovich · 2026-04-09T15:13:39Z

parquet-variant-compute/src/variant_get.rs

+
+            if is_variant_field {
+                // Variant field: the child's actual data type (VariantArray's internal
+                // StructArray) differs from the user's declared type (empty Struct),


Wait... is it ever (supposed to be) an empty struct?

The extension type blindly accepts any struct, so I guess that does work. And I don't see any code that would help a user create a correct physical variant struct (binary variant, shredded, etc).

Which opens up an interesting design question, because a request for variant_get to return variant could be any of three "flavors":

Generic

"give me back variant, in whatever form it takes"

If the underlying path is already variant, return it unchanged

If strongly typed, there's a very fast path to shred it -- especially if the column contains no nulls

Probably the most common, and also what this PR does (I think)

Unshredded

"give me back binary variant no matter what form the data takes"

If the underlying path is variant, unshred it as needed

If strongly typed, convert to binary variant

Useful for a caller who doesn't want to deal with (or cannot handle) the complexity and physical schema heterogeneity of shredding

Shredded

"give me back shredded variant following the specific shredding schema I provided"

If the underlying path is variant, shred or reshred as needed

If the underlying path is strongly typed, shred it using the provided shredding schema

Useful for a caller who expects a specific format to the data but needs to handle outliers

I guess the best way to handle this is for users to send an empty struct for 1/, with a non-empty struct triggering 2/ and 3/ as appropriate? With the usual recursive pathing to handle each leaf field as efficiently as possible? But we don't currently have a way to pass a shredding schema, so everyone is forced to 1/.

This relates to #9598 (comment), where I was worried that variant extension type would "leak" -- because I had been assuming we covered all three cases above, and that variant extension type was used recursively as the marker to distinguish between strongly typed vs. shredding schemas (which becomes a problem once we recurse into a shredded schema).

In effect, we have several different "flavors" of calls to shredded_path_get:

No specific type requested (return whatever we find)

Strong type requested (parse out the variant values as needed)

Specific variant type requested (provided field is a struct with variant extension type metadata)

Recursing into the shredded field of a specific requested variant type

The problem is, a strongly typed struct (2/) is indistinguishable from a shredded variant field (4/) -- both fields are structs lacking variant extension type metadata.

scovich · 2026-04-09T15:18:00Z

parquet-variant-compute/src/variant_get.rs

+                    for _ in 0..child.len() {
+                        builder.append_null();
+                    }


There's been a recent bunch of PR adding (or expanding) support for bulk-appending NULL values to array builders, e.g. PrimitiveArrayBuilder::append_nulls, StructBuilder::append_nulls, etc. Should we take a follow-on item to add that support to variant array builder as well? (Some builders have append_values methods as well).

Filed here #9684. Should be a quick one.

scovich · 2026-04-09T15:23:34Z

parquet-variant-compute/src/variant_get.rs

+                let new_field = field
+                    .as_ref()
+                    .clone()
+                    .with_data_type(child.data_type().clone())
+                    .with_extension_type(VariantType);
+                updated_fields.push(new_field);


nit: We're cloning and then discarding the field's existing data type.

Suggested change

let new_field = field

.as_ref()

.clone()

.with_data_type(child.data_type().clone())

.with_extension_type(VariantType);

updated_fields.push(new_field);

let new_field = Field::new(

field.name().clone(),

child.data_type().clone(),

field.is_nullable(),

);

updated_fields.push(Arc::new(new_field.with_extension_type(VariantType)));

scovich · 2026-04-09T15:43:40Z

parquet-variant-compute/src/variant_get.rs

+                // StructArray) differs from the user's declared type (empty Struct),
+                // so we must update the field's data type and re-attach VariantType.
+                //
+                // When the field is entirely absent, shredded_get_path returns a


Suggested change

// When the field is entirely absent, shredded_get_path returns a

// Additionally, when the field is entirely absent, shredded_get_path returns a

(it took me a minute to grok that these are two mostly-separate issues)

github-actions bot added the parquet-variant parquet-variant* crates label Mar 21, 2026

scovich reviewed Mar 23, 2026

View reviewed changes

codephage2020 marked this pull request as draft March 25, 2026 06:45

klion26 reviewed Mar 25, 2026

View reviewed changes

parquet-variant-compute/src/variant_get.rs Outdated Show resolved Hide resolved

parquet-variant-compute/src/variant_get.rs Outdated Show resolved Hide resolved

codephage2020 force-pushed the issue-9519 branch from 1439645 to 39bc464 Compare March 31, 2026 11:18

codephage2020 marked this pull request as ready for review March 31, 2026 11:30

codephage2020 force-pushed the issue-9519 branch from 39bc464 to 677ceca Compare March 31, 2026 11:35

github-actions bot added parquet Changes to the parquet crate arrow Changes to the arrow crate arrow-avro arrow-avro crate labels Mar 31, 2026

codephage2020 marked this pull request as draft March 31, 2026 11:38

codephage2020 force-pushed the issue-9519 branch from 677ceca to 97e687e Compare March 31, 2026 11:39

github-actions bot removed parquet Changes to the parquet crate arrow Changes to the arrow crate arrow-avro arrow-avro crate labels Mar 31, 2026

codephage2020 marked this pull request as ready for review March 31, 2026 11:42

scovich reviewed Apr 1, 2026

View reviewed changes

codephage2020 and others added 2 commits April 2, 2026 10:32

Support extracting struct fields as Variant using ExtensionType

b605bfd

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

codephage2020 force-pushed the issue-9519 branch from 52376bc to 9c6ee55 Compare April 2, 2026 02:34

Merge branch 'main' into issue-9519

c0fe077

sdf-jkl suggested changes Apr 8, 2026

View reviewed changes

parquet-variant-compute/src/variant_get.rs Show resolved Hide resolved

parquet-variant-compute/src/variant_get.rs Outdated Show resolved Hide resolved

sdf-jkl mentioned this pull request Apr 8, 2026

[Variant] variant_get should follow JSONPath semantics for Field path element #9676

Open

codephage2020 and others added 2 commits April 9, 2026 14:13

Merge branch 'main' into issue-9519

4e3bbb3

sdf-jkl approved these changes Apr 9, 2026

View reviewed changes

scovich reviewed Apr 9, 2026

View reviewed changes

sdf-jkl mentioned this pull request Apr 9, 2026

[Variant] Add VariantArrayBuilder::append_nulls API #9684

Open

		// followed further. Fall back to the value column if present,
		// otherwise the path is missing.

	"Expected shredded variant struct for field '{}', got {}",
	"Physical layout of shredded variant field '{}' should be a struct, got {}",

	// When the field is entirely absent, shredded_get_path returns a
	// Additionally, when the field is entirely absent, shredded_get_path returns a

Conversation

codephage2020 commented Mar 21, 2026

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Uh oh!

codephage2020 commented Mar 21, 2026

Uh oh!

scovich left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codephage2020 commented Mar 25, 2026

Uh oh!

Uh oh!

Uh oh!

sdf-jkl commented Mar 30, 2026

Uh oh!

codephage2020 commented Mar 31, 2026

Uh oh!

scovich left a comment

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!

codephage2020 Apr 2, 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!

codephage2020 Apr 2, 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!

sdf-jkl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

sdf-jkl left a comment • 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 left a comment

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!

Reviewers

Assignees

codephage2020 Apr 2, 2026 •

edited

Loading

codephage2020 Apr 2, 2026 •

edited

Loading

sdf-jkl left a comment •

edited

Loading