parquet-variant-compute: shred_variant panics on an object with duplicate field names

[cetra3]

shred_variant panics on an object with duplicate field names

Describe the bug

shred_variant (and the whole shred_variant_with_options path) panics — rather than
returning Err — when an input object contains two fields that resolve to the same
dictionary name. The panic surfaces deep inside arrow-array, so it is not obvious that
the cause is malformed variant input.

Variant::try_new correctly rejects such an object, but VariantArray::try_new only
performs shallow validation of its elements, so a duplicate-field object read from a file
reaches the shredder unchecked.

To Reproduce

use arrow::array::{ArrayRef, BinaryViewArray, StructArray};
use arrow::datatypes::{DataType, Field, Fields};
use parquet_variant_compute::{shred_variant, VariantArray};
use std::sync::Arc;

// Metadata dictionary with a single entry: ["a"]
let metadata: &[u8] = &[0x01, 0x01, 0x00, 0x01, b'a'];

// Object with TWO fields, both field-id 0 -> both named "a"
let value: &[u8] = &[
    0x02, 0x02,       // object header, 2 elements
    0x00, 0x00,       // field ids: [0, 0]
    0x00, 0x02, 0x04, // value offsets
    12, 0,            // Int8(0)
    12, 1,            // Int8(1)
];

// Variant::try_new rejects this...
assert!(parquet_variant::Variant::try_new(metadata, value).is_err());

// ...but VariantArray::try_new only shallow-validates, so it reaches the shredder.
let meta_col: ArrayRef = Arc::new(BinaryViewArray::from(vec![Some(metadata)]));
let value_col: ArrayRef = Arc::new(BinaryViewArray::from(vec![Some(value)]));
let sa = StructArray::try_new(
    Fields::from(vec![
        Field::new("metadata", DataType::BinaryView, false),
        Field::new("value", DataType::BinaryView, true),
    ]),
    vec![meta_col, value_col],
    None,
).unwrap();
let array = VariantArray::try_new(&sa).unwrap();

let as_type = DataType::Struct(Fields::from(vec![Field::new("a", DataType::Int8, true)]));
let _ = shred_variant(&array, &as_type); // panics

Panic:

thread '...' panicked at arrow-array/src/array/struct_array.rs:91:
called `Result::unwrap()` on an `Err` value: InvalidArgumentError(
    "Incorrect array length for StructArray field \"typed_value\", expected 1 got 2")

Root cause

In VariantToShreddedObjectVariantRowBuilder::append_value (shred_variant.rs, ~L418), each
object field is routed by name into a per-field builder:

for (field_name, value) in obj.iter() {
    match self.typed_value_builders.get_mut(field_name) {
        Some(typed_value_builder) => {
            typed_value_builder.append_value(value)?; // called once PER matching field
            seen.insert(field_name);
        }
        None => {
            object_builder.insert_bytes(field_name, value);
            partially_shredded = true;
        }
    }
}

Two fields named "a" cause append_value to run twice on that child builder, while the row's
null buffers (typed_value_nulls, nulls) advance only once at the end. The "a" child ends
up with 2 rows against 1 → the length check in StructArray::try_new (called from finish)
fails and is unwrap-ed into a panic.

Expected behavior

shred_variant should return an InvalidArgumentError, consistent with Variant::try_new
and with the existing collision guard in unshred_variant (unshred_variant.rs:627-631,
"Field '...' appears in both typed_value and value").

Proposed fix

The builder already maintains a seen set (currently only used for the missing-fields pass).
Promote it to reject duplicates before the append:

let mut seen = std::collections::HashSet::new();
let mut partially_shredded = false;
for (field_name, value) in obj.iter() {
    // A field name must be unique within an object. A duplicate would append twice to one
    // typed_value builder while the row's null buffers advance only once, leaving the child
    // arrays with mismatched lengths and panicking in `finish`. Reject it as invalid input,
    // mirroring the collision check in `unshred_variant`.
    if !seen.insert(field_name) {
        return Err(ArrowError::InvalidArgumentError(format!(
            "duplicate field name '{field_name}' in variant object",
        )));
    }
    match self.typed_value_builders.get_mut(field_name) {
        Some(typed_value_builder) => {
            typed_value_builder.append_value(value)?;
        }
        None => {
            object_builder.insert_bytes(field_name, value);
            partially_shredded = true;
        }
    }
}

The guard runs before the match, so a duplicate in the unshredded branch is caught too. The
missing-fields pass below (!seen.contains(...)) is unchanged and remains correct. Verified:
the repro then returns Err("duplicate field name 'a' in variant object"), the existing suite
passes, and reverting the guard reproduces the exact struct_array.rs:91 panic.

(Deliberately scoped narrowly to shred_variant. Whether VariantArray::try_new should fully
validate its elements — which would close a broader class of "bad object reaches a downstream
kernel" issues — is a separate perf/design question.)

Additional context

Found while adding a proptest fuzzing harness to parquet-variant in #10352; this defect is
downstream of that PR's scope and not fixed by it. Investigation was assisted by an
agent-driven proptest harness.