Support reading/writing `VariantArray` to parquet with Variant LogicalType #8408

alamb · 2025-09-22T19:13:31Z

Which issue does this PR close?

Closes [Variant] Support reading/writing Parquet Variant LogicalType #8370
Closes Path Finding: Support reading/writing VariantArray to parquet with Variant LogicalType #8365

Rationale for this change

Parquet has logical types, which is how other writers signal what columns contain Variant values.

What changes are included in this PR?

Add mapping from Parquet LogicalType to/from Arrow ExtensionType added in Add Arrow Variant Extension Type, remove Array impl for VariantArray and ShreddedVariantFieldArray #8392
Documentation and tests showing reading/writing Parquet files with the Variant logical annotation

Are these changes tested?

Yes, new unit tests and doc examples

Are there any user-facing changes?

You can now read/write Variant columns to VariantArray

alamb · 2025-09-22T19:28:11Z

parquet/src/arrow/schema/complex.rs

                        .with_nullable(false),
                );
-                let value_field = Arc::new(convert_field(map_value, &value, arrow_value));
+                let value_field = Arc::new(convert_field(map_value, &mut value, arrow_value));


This logic is somewhat confusing to me in that the arrow type is encoded twice -- once on a ParquetField (which has an arrow Field in it) and once in this ValueField.

Any help simplifing this would be most appreciated

Sorry, I don't think I understand this part of the code well enough to suggest anything :(

alamb · 2025-09-22T19:29:25Z

parquet/src/arrow/schema/extension.rs

+// specific language governing permissions and limitations
+// under the License.
+
+//! Arrow Extension Type Support for Parquet


I plan to consolidate the rest of the extension type handling in this module, to try and improve the current situation where #cfg(..) is sprinkled over the type conversion logic

Update, see:

Support parquet canonical extension type roundtrip #8409

alamb · 2025-09-22T19:31:19Z

parquet/src/arrow/schema/extension.rs

+#[cfg(feature = "variant_experimental")]
+pub(crate) fn logical_type_for_struct(field: &Field) -> Option<LogicalType> {
+    use parquet_variant_compute::VariantType;
+    if field.extension_type_name()? == VariantType::NAME {


I was worried here about testing for the extension type using try_extension_type and then discarding any error via ok() -- creating an ArrowError requires allocating a string, so that pattern can be expensive (allocate and format a string just to throw it away)

@scovich also noticed this in the pathfinding PR here:

Path Finding: Support reading/writing VariantArray to parquet with Variant LogicalType #8365 (comment)

Can we do both? Check the name (= quick and cheap) and only try_extension_type if the name matches? (if they asked for Variant and provided an incorrect data type, they shouldn't be surprised at the cost of allocating an error message)

This is an excellent call -- done in 68ffd32

alamb · 2025-09-22T19:32:14Z

parquet/src/variant.rs

+//! ```
 pub use parquet_variant::*;
-pub use parquet_variant_compute as compute;
+pub use parquet_variant_compute::*;


I also started importing everything directly into parquet::variant rather than also having a parquet::variant::compute which I think makes it easier to work with this crate

alamb · 2025-09-22T19:35:14Z

parquet/src/variant.rs

+    use std::path::PathBuf;
+    use std::sync::Arc;
+
+    #[test]


here are end to end cases showing how to read/write the files to disk

alamb · 2025-09-22T19:38:32Z

parquet/src/variant.rs

+    /// Ensure a file with Variant LogicalType, written by another writer in
+    /// parquet-testing, can be read as a VariantArray
+    #[test]
+    fn read_logical_type() {


@scovich noted in following comment of the pathfinding PR that this is very similar to the doc test above.

Path Finding: Support reading/writing VariantArray to parquet with Variant LogicalType #8365 (comment)

I re-reviewed it and while I agree this is redundant with the doc test above, I feel that putting the test in both places makes it easier to understand that there is test coverage for reading logical types, and thus I would like to keep it here.

alamb · 2025-09-24T18:29:02Z

@scovich / @paleolimbot / @mbrobbel I wonder if you might have some time to review this PR?

It consists of the final connection code to read/write variant LogicalTypes.

I have a follow on in

Support parquet canonical extension type roundtrip #8409
Also @paleolimbot has a great suggestion about making some sort of ExtensionType registry which I plan to file as a separate issue

scovich · 2025-09-24T18:57:30Z

@alamb -- PR description seems a bit... unpopulated?

scovich

Looks like a step forward

scovich · 2025-09-24T19:03:23Z

parquet/src/arrow/schema/extension.rs

+#[cfg(feature = "variant_experimental")]
+pub(crate) fn logical_type_for_struct(field: &Field) -> Option<LogicalType> {
+    use parquet_variant_compute::VariantType;
+    if field.extension_type_name()? == VariantType::NAME {


Can we do both? Check the name (= quick and cheap) and only try_extension_type if the name matches? (if they asked for Variant and provided an incorrect data type, they shouldn't be surprised at the cost of allocating an error message)

alamb · 2025-09-24T19:15:05Z

@alamb -- PR description seems a bit... unpopulated?

Good call -- I updated it

…_type

paleolimbot

I'm very new to this part of the code but I've added context from Arrow C++ where I thought it might fit!

parquet/src/arrow/schema/extension.rs

paleolimbot · 2025-09-24T20:16:02Z

parquet/src/arrow/schema/extension.rs

+    // Check the name (= quick and cheap) and only try_extension_type if the name matches
+    // to avoid unnecessary String allocations in ArrowError
+    if field.extension_type_name()? != VariantType::NAME {
+        return None;
+    }
+    match field.try_extension_type::<VariantType>() {
+        Ok(VariantType) => Some(LogicalType::Variant),
+        // Given check above, this should not error, but if it does ignore
+        Err(_e) => None,
+    }


I know you're working to make this more generic, but as a pattern this is also how Arrow C++ does the conversion (except in Arrow C++ we can just look at the DataType and here you have to poke the field metadata and parse it. Arrow C++ just hard-codes a few names/storage types too (but no compile time flag on the Parquet library...just a static extension registry that would control whether an extension type shows up as a DataType or a field with metadata).

Yeah, I think it is a fine line to decide how much of the canonical extension types end up in the core parquet/Arrow API

Including them all by default makes the initial developer experience nicer perhaps and keeps the code cleaner, but it makes it harder to customize the binary size / feature set

In my mind I am trying to follow the existing pattern / guidelines (set by @tustvold I think) and I do think it makes sense, but I do understand there are tradeoffs involved

Ah sorry, I think I was reflecting in my comment that you're doing exactly what Arrow C++ is doing here (canonical extension type support there is also controlled by compile-time flags, they're just on by default and it just manifests as missing field metadata).

mbrobbel

Thanks @alamb, looking forward to #8409

parquet/src/arrow/schema/extension.rs

parquet/src/variant.rs

alamb · 2025-09-25T21:29:28Z

I ran out of time today, but I plan to address comments on this PR tomorrow and merge it. I also want to close out the initial Variant epic and do some housecleaning ticket-wise

Co-authored-by: Matthijs Brobbel <[email protected]>

alamb · 2025-09-26T16:15:03Z

Ok, I think I have resolved all the comments on this PR and plan to merge it in when it passes. I'l then polish up #8409 and clean up the variant epic

alamb · 2025-09-26T16:21:20Z

woohoo! That feels pretty good -- we can read/write Parquet files with Variant logical types now!

# Which issue does this PR close? - Follow on to #8408 - Closes #7063 # Rationale for this change I was trying to consolidate the parquet extension type code after #8408, and in so doing I believe I actually found (and fixed) the root cause of #7063 (I will point it out inline) # What changes are included in this PR? 1. Consolidate parquet<-->arrow extension type metadata mapping in one module 2. Enable tests # Are these changes tested? Yes # Are there any user-facing changes? When reading parquet that is annotated with Json or UUID logical types, the resulting Arrow arrays will also have the canonical types attached.

github-actions bot added parquet Changes to the parquet crate parquet-variant parquet-variant* crates labels Sep 22, 2025

Support reading/writing VariantArray to parquet with Variant LogicalType

8f16381

alamb force-pushed the alamb/variant_logical_type branch from 04b2e37 to 8f16381 Compare September 22, 2025 19:39

alamb marked this pull request as ready for review September 22, 2025 19:58

alamb commented Sep 22, 2025

View reviewed changes

This was referenced Sep 22, 2025

Support mapping canonical extension types to Parquet LogicalTypes #7063

Closed

Path Finding: Support reading/writing VariantArray to parquet with Variant LogicalType #8365

Closed

Support parquet canonical extension type roundtrip #8409

Merged

Merge branch 'main' into alamb/variant_logical_type

c35d758

alamb requested a review from scovich September 24, 2025 18:26

scovich approved these changes Sep 24, 2025

View reviewed changes

alamb changed the title ~~Support reading/writing VariantArray to parquet with Variant LogicalType~~ Support reading/writing VariantArray to parquet with Variant LogicalType Sep 24, 2025

alamb added 2 commits September 24, 2025 15:15

Merge remote-tracking branch 'apache/main' into alamb/variant_logical…

e40d275

…_type

use try_extension_type after faster name check

68ffd32

paleolimbot reviewed Sep 24, 2025

View reviewed changes

mbrobbel approved these changes Sep 25, 2025

View reviewed changes

parquet/src/arrow/schema/extension.rs Outdated Show resolved Hide resolved

parquet/src/arrow/schema/extension.rs Show resolved Hide resolved

parquet/src/variant.rs Show resolved Hide resolved

alamb and others added 4 commits September 26, 2025 08:20

Merge branch 'main' into alamb/variant_logical_type

d19e72f

Update parquet/src/arrow/schema/extension.rs

ddd5fb8

Co-authored-by: Matthijs Brobbel <[email protected]>

fix whitespace in examples

a75e4ca

Do not panic on extension type mismatch

e6cacd8

alamb mentioned this pull request Sep 26, 2025

[Arrow] Consider faster way to check if a Field has an extension type #8474

Open

fmt

77f0d93

alamb merged commit d6a29ec into apache:main Sep 26, 2025
20 checks passed

alamb mentioned this pull request Oct 17, 2025

[Variant] Support reading/writing Parquet Variant LogicalType #8370

Closed

paleolimbot mentioned this pull request Oct 30, 2025

[geometry] Wire up arrow reader/writer for GEOMETRY and GEOGRAPHY #8717

Open

Support reading/writing VariantArray to parquet with Variant LogicalType #8408

Support reading/writing VariantArray to parquet with Variant LogicalType #8408

Uh oh!

Conversation

alamb commented Sep 22, 2025 • edited 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 these changes tested?

Are there any user-facing changes?

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!

alamb commented Sep 24, 2025

Uh oh!

scovich commented Sep 24, 2025

Uh oh!

scovich left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alamb commented Sep 24, 2025

Uh oh!

paleolimbot 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!

mbrobbel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alamb commented Sep 25, 2025

Uh oh!

alamb commented Sep 26, 2025

Uh oh!

Uh oh!

alamb commented Sep 26, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Support reading/writing `VariantArray` to parquet with Variant LogicalType #8408

Support reading/writing `VariantArray` to parquet with Variant LogicalType #8408

alamb commented Sep 22, 2025 •

edited

Loading