Skip to content

feat(arrow): recognize arrow.parquet.variant on schema import#2840

Open
c-thiel wants to merge 1 commit into
apache:mainfrom
c-thiel:feat/arrow-to-iceberg-variant
Open

feat(arrow): recognize arrow.parquet.variant on schema import#2840
c-thiel wants to merge 1 commit into
apache:mainfrom
c-thiel:feat/arrow-to-iceberg-variant

Conversation

@c-thiel

@c-thiel c-thiel commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator

Teach arrow_schema_to_schema to fold a field tagged with the canonical arrow.parquet.variant extension into an atomic Type::Variant, the inverse of the export path added in #2188. Previously such a schema errored with "Field id not found in metadata" because the variant's metadata/value storage sub-fields carry no field id.

Which issue does this PR close?

PR 1 of 2 for Variant read support.

What changes are included in this PR?

Add a defaulted ArrowSchemaVisitor::variant dispatched via a new visit_field helper at each field-dispatch site. The default re-enters normal traversal, so the reconstructing visitors (Int96Coercion, MetadataStrip) are unchanged; ArrowSchemaConverter overrides it to fold the struct without descending, guarding that the extension sits on Struct storage. Mirrors Java's Parquet reader, which recovers a variant from its group-level annotation without visiting the group's children.

Covers top-level, struct-nested, list-element and map-value positions plus auto-assigned ids; regenerates the iceberg public-api baseline.

Are these changes tested?

unit-tested yes.

Teach arrow_schema_to_schema to fold a field tagged with the canonical
arrow.parquet.variant extension into an atomic Type::Variant, the inverse
of the export path added in apache#2188. Previously such a schema errored with
"Field id not found in metadata" because the variant's metadata/value
storage sub-fields carry no field id.

@CTTY CTTY left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Just one comment

fn variant(&mut self, field: &FieldRef) -> Result<Self::T> {
// The extension may only sit on struct storage (mirrors
// `VariantExtensionType::supports_data_type`).
if !matches!(field.data_type(), DataType::Struct(_)) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we match the nested types here as well (metadata and value)?

/// single logical variant.
///
/// Takes the `&FieldRef` rather than a `&DataType` because the variant signal
/// lives on the field's metadata, not on its data type.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants