Skip to content

[Variant] Accept Dictionary and RunEndEncoded metadata in VariantArray #10802

Description

@peterxcli

Describe the bug

The canonical arrow.parquet.variant extension permits its non-nullable metadata child to be dictionary-encoded or run-end-encoded. However, VariantArray::try_new calls a validator whose validation and row access accept only Binary, LargeBinary, and BinaryView.

This affects arrow-rs 59.2.0 and current main:

  • Canonical extension definition: https://arrow.apache.org/docs/format/CanonicalExtensions.html#parquet-variant
  • VariantArray::try_new documents Dictionary metadata as permissible, but rejects it: https://github.com/apache/arrow-rs/blob/59.2.0/parquet-variant-compute/src/variant_array.rs#L285-L320
  • Byte access and validation only handle unencoded binary arrays: https://github.com/apache/arrow-rs/blob/59.2.0/parquet-variant-compute/src/variant_array.rs#L43-L83
  • Current main has the same limitation:
    /// Returns the raw bytes at the given index from a binary-like array, return `None` if the array isn't binary-like.
    pub(crate) fn binary_array_value(array: &dyn Array, index: usize) -> Option<&[u8]> {
    match array.data_type() {
    DataType::Binary => Some(array.as_binary::<i32>().value(index)),
    DataType::LargeBinary => Some(array.as_binary::<i64>().value(index)),
    DataType::BinaryView => Some(array.as_binary_view().value(index)),
    _ => None,
    }
    }
    /// Returns a [`Variant`] from a `metadata` and `value` byte arrays, returns `None`
    /// if one of them is of invalid type.
    pub(crate) fn variant_from_arrays_at<'m, 'v>(
    metadata: &'m dyn Array,
    value: &'v dyn Array,
    index: usize,
    ) -> Option<Variant<'m, 'v>> {
    let metadata = binary_array_value(metadata, index)?;
    let value = binary_array_value(value, index)?;
    Some(Variant::new(metadata, value))
    }
    /// Returns an all-null binary `value` column of the given length.
    ///
    /// The shredding spec requires the `value` column to always be present in the
    /// schema, so producers that have no unshredded values to store must still
    /// emit an all-null column. See <https://github.com/apache/arrow-rs/issues/10306>.
    pub(crate) fn all_null_value_column(len: usize) -> ArrayRef {
    new_null_array(&DataType::BinaryView, len)
    }
    /// Validates that an array has a binary-like data type.
    pub(crate) fn validate_binary_array(array: &dyn Array, field_name: &str) -> Result<()> {
    match array.data_type() {
    DataType::Binary | DataType::LargeBinary | DataType::BinaryView => Ok(()),
    _ => Err(ArrowError::InvalidArgumentError(format!(
    "VariantArray '{field_name}' field must be Binary, LargeBinary, or BinaryView, got {}",
    array.data_type()
    ))),
    }

This is specifically about encoding the Variant metadata child. It is not about shredded typed_value layouts or Dictionary/REE output from variant_to_arrow (#10013 / #10014).

To Reproduce

A conforming StructArray with this storage schema fails construction:

Struct<
  metadata: Dictionary<Int8, Binary> not null,
  value: Binary not null
>

For example, using an empty Variant metadata dictionary and an Int8 Variant value:

let metadata_values = Arc::new(BinaryArray::from(vec![
    Some(&[0x01, 0x00, 0x00][..]),
]));
let metadata = Arc::new(
    DictionaryArray::<Int8Type>::try_new(
        Int8Array::from(vec![0]),
        metadata_values,
    )
    .unwrap(),
);
let value = Arc::new(BinaryArray::from(vec![Some(&[12, 1][..])]));

let array = StructArray::try_new(
    Fields::from(vec![
        Field::new("metadata", metadata.data_type().clone(), false),
        Field::new("value", DataType::Binary, false),
    ]),
    vec![metadata, value],
    None,
)
.unwrap();

VariantArray::try_new(&array).unwrap();

The final call currently fails with:

VariantArray 'metadata' field must be Binary, LargeBinary, or BinaryView, got Dictionary(Int8, Binary)

Run-end-encoded metadata is rejected for the same reason.

Expected behavior

VariantArray should accept Dictionary- and RunEndEncoded metadata whose logical values are Binary, LargeBinary, or BinaryView.

All paths that consume metadata bytes—including row access and compute paths such as variant_get and unshred_variant—must resolve the logical metadata value correctly; relaxing constructor validation alone would not be sufficient.

Please add focused coverage for both Dictionary and RunEndEncoded metadata, including null/sliced arrays and at least one downstream compute path.

Additional context

Downstream consumers currently have to decode the metadata child eagerly before constructing VariantArray. This is a follow-up to the raw binary layout support in #8387 / #9610.

Activity

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

Metadata

Metadata

Assignees

Labels

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions