This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-11961-cacb42eaf39646c3ca52459c954bba348bc343b1 in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
commit 562983649f933bb7709ce60fe295ff29cf5361ea Author: Raghuvansh Raj <[email protected]> AuthorDate: Thu Oct 29 13:14:05 2026 +0000 fix(parquet): write `specification_version = 1` on the VARIANT logical type (#11961) # Which issue does this PR close? - Closes #11917 # Rationale for this change The Parquet writer annotates Variant columns with `VARIANT` but leaves `specification_version` unset. The Parquet spec requires the version in the declaration ([`LogicalTypes.md` L602-L603](https://github.com/apache/parquet-format/blob/e365c6f7c05eea69c5aa11f1ead6c3db261ca9f9/LogicalTypes.md#L602-L603)), and Spark 4.1 only reads `VARIANT(1)` ([`ParquetSchemaConverter.scala#L395`](https://github.com/apache/spark/blob/branch-4.1/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/parquet/ParquetSchemaConverter.scala#L395)), so every Variant file written by arrow-rs today fails in Spark with `PARQUET_TYPE_NOT_RECOGNIZED`. # What changes are included in this PR? - `logical_type_for_struct` now writes `LogicalType::variant(Some(VARIANT_SPEC_VERSION as i8))`. The constant is the existing header-version constant in `parquet-variant` (`CORRECT_VERSION_VALUE`), renamed to `VARIANT_SPEC_VERSION` and made `pub`, so the annotation and the metadata header share one source of truth. - Existing `write_logical_type` test in `parquet/src/variant.rs` updated to assert the version. - New `arrow_variant_to_parquet_variant` schema-conversion test in `parquet/src/arrow/schema/mod.rs`, next to the UUID and JSON ones. The reader is unchanged: `try_add_extension_type` already matches `LogicalType::Variant(_)`, so files written before this change still read. # Are these changes tested? Yes. Both tests fail on `main` and pass with the change. `cargo test -p parquet --features variant_experimental,arrow,arrow_canonical_extension_types,serde --lib`, `--test variant_integration` and `--test arrow_writer` all pass. # Are there any significant user-facing changes? Variant files written with `variant_experimental` now carry `VARIANT(1)` and are readable by Spark. One public addition: `parquet_variant::VARIANT_SPEC_VERSION` (`pub const u8 = 1`), the version shared by the metadata header and the logical type annotation. # AI Use Prepared with AI assistance; code, tests and links reviewed and run by the author. --- parquet-variant/src/variant.rs | 4 +++- parquet-variant/src/variant/metadata.rs | 16 ++++++++++------ parquet/src/arrow/schema/extension.rs | 10 +++++++++- parquet/src/arrow/schema/mod.rs | 33 +++++++++++++++++++++++++++++++++ parquet/src/variant.rs | 6 ++++-- 5 files changed, 59 insertions(+), 10 deletions(-) diff --git a/parquet-variant/src/variant.rs b/parquet-variant/src/variant.rs index 6e57e0bee5..48f38d662d 100644 --- a/parquet-variant/src/variant.rs +++ b/parquet-variant/src/variant.rs @@ -17,7 +17,9 @@ pub use self::decimal::{VariantDecimal4, VariantDecimal8, VariantDecimal16, VariantDecimalType}; pub use self::list::VariantList; -pub use self::metadata::{EMPTY_VARIANT_METADATA, EMPTY_VARIANT_METADATA_BYTES, VariantMetadata}; +pub use self::metadata::{ + EMPTY_VARIANT_METADATA, EMPTY_VARIANT_METADATA_BYTES, VARIANT_SPEC_VERSION, VariantMetadata, +}; pub use self::object::VariantObject; // Publicly export types used in the API diff --git a/parquet-variant/src/variant/metadata.rs b/parquet-variant/src/variant/metadata.rs index 99de7b6447..da45757729 100644 --- a/parquet-variant/src/variant/metadata.rs +++ b/parquet-variant/src/variant/metadata.rs @@ -32,9 +32,13 @@ pub(crate) struct VariantMetadataHeader { offset_size: OffsetSizeBytes, } -// According to the spec this is currently always = 1, and so we store this const for validation -// purposes and to make that visible. -const CORRECT_VERSION_VALUE: u8 = 1; +/// The version of the Variant specification implemented by this crate. +/// +/// The spec currently defines only version 1. It appears in two places on +/// disk: the 4-bit version field of every metadata header, and the +/// `specification_version` of the Parquet `VARIANT` logical type annotation. +/// Both must carry this value. +pub const VARIANT_SPEC_VERSION: u8 = 1; // The metadata header occupies one byte; use a named constant for readability const NUM_HEADER_BYTES: u32 = 1; @@ -68,9 +72,9 @@ impl VariantMetadataHeader { /// - The actual number of bytes, offset_size, is offset_size_minus_one + 1 pub(crate) fn try_new(header_byte: u8) -> Result<Self, ArrowError> { let version = header_byte & 0x0F; // First four bits - if version != CORRECT_VERSION_VALUE { + if version != VARIANT_SPEC_VERSION { let err_msg = format!( - "The version bytes in the header is not {CORRECT_VERSION_VALUE}, got {version:b}", + "The version bytes in the header is not {VARIANT_SPEC_VERSION}, got {version:b}", ); return Err(ArrowError::InvalidArgumentError(err_msg)); } @@ -170,7 +174,7 @@ pub const EMPTY_VARIANT_METADATA_BYTES: &[u8] = &[1, 0, 0]; pub const EMPTY_VARIANT_METADATA: VariantMetadata = VariantMetadata { bytes: EMPTY_VARIANT_METADATA_BYTES, header: VariantMetadataHeader { - version: CORRECT_VERSION_VALUE, + version: VARIANT_SPEC_VERSION, is_sorted: false, offset_size: OffsetSizeBytes::One, }, diff --git a/parquet/src/arrow/schema/extension.rs b/parquet/src/arrow/schema/extension.rs index 4e1ec75084..11bb97eb59 100644 --- a/parquet/src/arrow/schema/extension.rs +++ b/parquet/src/arrow/schema/extension.rs @@ -123,11 +123,19 @@ pub(crate) fn has_extension_type(parquet_type: &Type) -> bool { } /// Return the Parquet logical type to use for the specified Arrow Struct field, if any. +/// +/// The Parquet specification requires the `VARIANT` logical type annotation to +/// carry the version number (`VARIANT(1)`); readers such as Apache Spark reject +/// files whose annotation does not carry version 1. The value written is +/// [`parquet_variant::VARIANT_SPEC_VERSION`], the same version stamped into +/// every metadata header. #[cfg(feature = "variant_experimental")] pub(crate) fn logical_type_for_struct(field: &Field) -> Option<LogicalType> { + use parquet_variant::VARIANT_SPEC_VERSION; use parquet_variant_compute::VariantType; if field.has_valid_extension_type::<VariantType>() { - Some(LogicalType::variant(None)) + // The 4-bit header version always fits the thrift `i8` field. + Some(LogicalType::variant(Some(VARIANT_SPEC_VERSION as i8))) } else { None } diff --git a/parquet/src/arrow/schema/mod.rs b/parquet/src/arrow/schema/mod.rs index ce993f0872..0c315040ba 100644 --- a/parquet/src/arrow/schema/mod.rs +++ b/parquet/src/arrow/schema/mod.rs @@ -2803,6 +2803,39 @@ mod tests { Ok(()) } + #[test] + #[cfg(feature = "variant_experimental")] + fn arrow_variant_to_parquet_variant() -> Result<()> { + use parquet_variant_compute::VariantType; + let variant_fields = Fields::from(vec![ + Field::new("metadata", DataType::BinaryView, false), + Field::new("value", DataType::BinaryView, true), + ]); + let arrow_schema = Schema::new(vec![ + Field::new("v", DataType::Struct(variant_fields), true) + .with_extension_type(VariantType), + ]); + + let parquet_schema = ArrowSchemaConverter::new().convert(&arrow_schema)?; + + // The Parquet spec requires the VARIANT annotation to carry the + // specification version (`VARIANT(1)`); Spark rejects files without it + let root = parquet_schema.root_schema(); + assert_eq!( + root.get_fields()[0].get_basic_info().logical_type_ref(), + Some(&LogicalType::variant(Some(1))) + ); + + let arrow_schema = parquet_to_arrow_schema(&parquet_schema, None)?; + assert!( + arrow_schema + .field(0) + .has_valid_extension_type::<VariantType>() + ); + + Ok(()) + } + #[test] fn test_parquet_to_arrow_field_levels_with_virtual_rejects_non_virtual() { let message_type = " diff --git a/parquet/src/variant.rs b/parquet/src/variant.rs index 23f45b10ee..967595ad0d 100644 --- a/parquet/src/variant.rs +++ b/parquet/src/variant.rs @@ -247,10 +247,12 @@ mod tests { assert_eq!(fields.len(), 1); let field = &fields[0]; assert_eq!(field.name(), "data"); - // data should have been written with the Variant logical type + // data should have been written with the Variant logical type, with the + // specification version set (required by the Parquet spec and by readers + // such as Spark, which only accept `VARIANT(1)`) assert_eq!( field.get_basic_info().logical_type_ref(), - Some(&crate::basic::LogicalType::variant(None)) + Some(&crate::basic::LogicalType::variant(Some(1))) ); }
