Skip to content

[Variant] make value mandatory field for VariantArray/ShreddingState #10318

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
alamb merged 5 commits into from
Jul 23, 2026
Merged

[Variant] make value mandatory field for VariantArray/ShreddingState #10318

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ce56d07
make `value` mandatory field for `VariantArray`/`ShreddingState`
sdf-jkl Jul 10, 2026
b631527
address @klion26's comments
sdf-jkl Jul 13, 2026
7030d2c
Merge branch 'main' of https://github.com/apache/arrow-rs into shredd…
sdf-jkl Jul 13, 2026
2738018
make VariantArray lenient on read - synthesize all-Null value col if …
sdf-jkl Jul 20, 2026
34fc7bb
Merge branch 'main' of https://github.com/apache/arrow-rs into shredd…
sdf-jkl Jul 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions parquet-variant-compute/src/from_json.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ mod test {
let variant_array = json_to_variant(&array_ref).unwrap();

let metadata_array = variant_array.metadata_column();
let value_array = variant_array.value_column().expect("value column");
let value_array = variant_array.value_column();

// Compare row 0
assert!(!variant_array.is_null(0));
Expand Down Expand Up @@ -134,7 +134,7 @@ mod test {
let variant_array = json_to_variant(&array_ref).unwrap();

let metadata_array = variant_array.metadata_column();
let value_array = variant_array.value_column().expect("value column");
let value_array = variant_array.value_column();

// Compare row 0
assert!(!variant_array.is_null(0));
Expand Down Expand Up @@ -183,7 +183,7 @@ mod test {
let variant_array = json_to_variant(&array_ref).unwrap();

let metadata_array = variant_array.metadata_column();
let value_array = variant_array.value_column().expect("value column");
let value_array = variant_array.value_column();

// Compare row 0
assert!(!variant_array.is_null(0));
Expand Down
90 changes: 41 additions & 49 deletions parquet-variant-compute/src/shred_variant.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,6 @@ pub(crate) fn shred_variant_with_options(
));
}

if array.value_column().is_none() {
// all-null case -- nothing to do.
return Ok(array.clone());
};

let mut builder = make_variant_to_shredded_variant_arrow_row_builder(
as_type,
cast_options,
Expand All @@ -103,7 +98,7 @@ pub(crate) fn shred_variant_with_options(
let (value, typed_value, nulls) = builder.finish()?;
Ok(VariantArray::from_parts(
array.metadata_column().clone(),
Some(Arc::new(value)),
Arc::new(value),
Some(typed_value),
nulls,
))
Expand Down Expand Up @@ -452,11 +447,8 @@ impl<'a> VariantToShreddedObjectVariantRowBuilder<'a> {
let mut builder = StructArrayBuilder::new();
for (field_name, typed_value_builder) in self.typed_value_builders {
let (value, typed_value, nulls) = typed_value_builder.finish()?;
let array = ShreddedVariantFieldArray::from_parts(
Some(Arc::new(value)),
Some(typed_value),
nulls,
);
let array =
ShreddedVariantFieldArray::from_parts(Arc::new(value), Some(typed_value), nulls);
builder = builder.with_field(field_name, ArrayRef::from(array), false);
}
if let Some(nulls) = self.typed_value_nulls.finish() {
Expand Down Expand Up @@ -701,7 +693,7 @@ impl VariantSchemaNode {
mod tests {
use super::*;
use crate::VariantArrayBuilder;
use crate::variant_array::{binary_array_value, variant_from_arrays_at};
use crate::variant_array::{all_null_value_column, binary_array_value, variant_from_arrays_at};
use arrow::array::{
Array, BinaryViewArray, FixedSizeBinaryArray, FixedSizeListArray, Float64Array,
GenericListArray, GenericListViewArray, Int64Array, LargeBinaryArray, LargeStringArray,
Expand Down Expand Up @@ -880,7 +872,7 @@ mod tests {
) {
assert_eq!(array.len(), expected_len);

let fallback_value = array.value_column().unwrap();
let fallback_value = array.value_column();
let fallback_metadata = array.metadata_column();
let array = downcast_list_like_array::<O>(array);

Expand Down Expand Up @@ -995,7 +987,7 @@ mod tests {
}

// Validate fallback variants for list elements that could not be shredded
let element_fallbacks = element_array.value_column().unwrap();
let element_fallbacks = element_array.value_column();
assert_eq!(element_fallbacks.len(), expected_fallbacks.len());
for (idx, expected_fallback) in expected_fallbacks.iter().enumerate() {
match expected_fallback {
Expand Down Expand Up @@ -1123,7 +1115,7 @@ mod tests {
typed_struct.column_by_name(field_name).unwrap(),
)
.unwrap();
assert!(field.value_column().unwrap().is_null(0));
assert!(field.value_column().is_null(0));
assert!(field.typed_value_column().unwrap().is_null(0));
}
}
Expand All @@ -1135,11 +1127,10 @@ mod tests {
// First create a valid VariantArray, then extract its parts to construct a shredded one
let temp_array = VariantArray::from_iter(vec![Some(Variant::from("test"))]);
let metadata = temp_array.metadata_column().clone();
let value = temp_array.value_column().unwrap().clone();
let value = temp_array.value_column().clone();
let typed_value = Arc::new(Int64Array::from(vec![42])) as ArrayRef;

let shredded_array =
VariantArray::from_parts(metadata, Some(value), Some(typed_value), None);
let shredded_array = VariantArray::from_parts(metadata, value, Some(typed_value), None);

let result = shred_variant(&shredded_array, &DataType::Int64);
assert!(matches!(
Expand All @@ -1150,14 +1141,18 @@ mod tests {

#[test]
fn test_all_null_input() {
// Create VariantArray with no value field (all null case)
let metadata = Arc::new(BinaryViewArray::from_iter_values([&[1u8, 0u8]])); // minimal valid metadata
let all_null_array = VariantArray::from_parts(metadata, None, None, None);
// Create VariantArray whose value column is entirely null
let metadata = Arc::new(BinaryViewArray::from_iter_values([
EMPTY_VARIANT_METADATA_BYTES,
]));
let all_null_array =
VariantArray::from_parts(metadata, all_null_value_column(1), None, None);
let result = shred_variant(&all_null_array, &DataType::Int64).unwrap();

// Should return array with no value/typed_value fields
assert!(result.value_column().is_none());
assert!(result.typed_value_column().is_none());
// The row is valid but has no value, so it shreds to an explicit Variant::Null
// stored in the value column, with a null typed_value
assert!(result.typed_value_column().unwrap().is_null(0));
assert_eq!(result.value(0), Variant::Null);
}

#[test]
Expand Down Expand Up @@ -1259,7 +1254,7 @@ mod tests {

// Verify structure
let metadata_field = result.metadata_column();
let value_field = result.value_column().unwrap();
let value_field = result.value_column();
let typed_value_field = result
.typed_value_column()
.unwrap()
Expand Down Expand Up @@ -1354,7 +1349,7 @@ mod tests {

let result = shred_variant(&input, &DataType::LargeUtf8).unwrap();
let metadata = result.metadata_column();
let value = result.value_column().unwrap();
let value = result.value_column();
let typed_value = result
.typed_value_column()
.unwrap()
Expand Down Expand Up @@ -1410,7 +1405,7 @@ mod tests {

let result = shred_variant(&input, &DataType::LargeBinary).unwrap();
let metadata = result.metadata_column();
let value = result.value_column().unwrap();
let value = result.value_column();
let typed_value = result
.typed_value_column()
.unwrap()
Expand Down Expand Up @@ -1660,20 +1655,20 @@ mod tests {
// The first row should be shredded, so the `value` field should be null and the
// `typed_value` field should contain the list
assert!(result.is_valid(0));
assert!(result.value_column().unwrap().is_null(0));
assert!(result.value_column().is_null(0));
assert!(result.typed_value_column().unwrap().is_valid(0));

// The second row should not be shredded because the provided schema for shredding did not
// match. Hence, the `value` field should contain the raw value and the `typed_value` field
// should be null.
assert!(result.is_valid(1));
assert!(result.value_column().unwrap().is_valid(1));
assert!(result.value_column().is_valid(1));
assert!(result.typed_value_column().unwrap().is_null(1));

// The third row should be shredded, so the `value` field should be null and the
// `typed_value` field should contain the list
assert!(result.is_valid(2));
assert!(result.value_column().unwrap().is_null(2));
assert!(result.value_column().is_null(2));
assert!(result.typed_value_column().unwrap().is_valid(2));

let typed_value = result.typed_value_column().unwrap();
Expand Down Expand Up @@ -1781,15 +1776,15 @@ mod tests {
.as_any()
.downcast_ref::<ListArray>()
.unwrap();
let outer_fallbacks = outer_elements.value_column().unwrap();
let outer_fallbacks = outer_elements.value_column();

let outer_metadata = Arc::new(BinaryViewArray::from_iter_values(std::iter::repeat_n(
EMPTY_VARIANT_METADATA_BYTES,
outer_elements.len(),
)));
let outer_variant = VariantArray::from_parts(
outer_metadata,
Some(outer_fallbacks.clone()),
outer_fallbacks.clone(),
Some(Arc::new(outer_values.clone())),
None,
);
Expand Down Expand Up @@ -1881,7 +1876,7 @@ mod tests {
let id_field =
ShreddedVariantFieldArray::try_new(element_objects.column_by_name("id").unwrap())
.unwrap();
let id_values = id_field.value_column().unwrap();
let id_values = id_field.value_column();
let id_typed_values = id_field
.typed_value_column()
.unwrap()
Expand All @@ -1905,7 +1900,7 @@ mod tests {
let name_field =
ShreddedVariantFieldArray::try_new(element_objects.column_by_name("name").unwrap())
.unwrap();
let name_values = name_field.value_column().unwrap();
let name_values = name_field.value_column();
let name_typed_values = name_field
.typed_value_column()
.unwrap()
Expand Down Expand Up @@ -1965,12 +1960,11 @@ mod tests {
let result = shred_variant(&input, &target_schema).unwrap();

// Verify structure
assert!(result.value_column().is_some());
assert!(result.typed_value_column().is_some());
assert_eq!(result.len(), 9);

let metadata = result.metadata_column();
let value = result.value_column().unwrap();
let value = result.value_column();
let typed_value = result
.typed_value_column()
.unwrap()
Expand All @@ -1985,14 +1979,14 @@ mod tests {
let age_field =
ShreddedVariantFieldArray::try_new(typed_value.column_by_name("age").unwrap()).unwrap();

let score_value = score_field.value_column().unwrap();
let score_value = score_field.value_column();
let score_typed_value = score_field
.typed_value_column()
.unwrap()
.as_any()
.downcast_ref::<Float64Array>()
.unwrap();
let age_value = age_field.value_column().unwrap();
let age_value = age_field.value_column();
let age_typed_value = age_field
.typed_value_column()
.unwrap()
Expand Down Expand Up @@ -2295,7 +2289,7 @@ mod tests {
assert_eq!(result.len(), 5);

// Access base value/typed_value columns
let value_field = result.value_column().unwrap();
let value_field = result.value_column();
let typed_struct = result
.typed_value_column()
.unwrap()
Expand Down Expand Up @@ -2331,7 +2325,7 @@ mod tests {
EMPTY_VARIANT_METADATA_BYTES,
scores_field.len(),
))),
Some(scores_field.value_column().unwrap().clone()),
scores_field.value_column().clone(),
Some(scores_field.typed_value_column().unwrap().clone()),
None,
),
Expand Down Expand Up @@ -2360,7 +2354,7 @@ mod tests {
.with_path("id", &DataType::Int32)?
.build();
let result1 = shred_variant(&input, &schema1).unwrap();
let value_field1 = result1.value_column().unwrap();
let value_field1 = result1.value_column();
assert!(!value_field1.is_null(0)); // should contain {"age": 25, "score": 95.5}

// Test with schema containing id and age fields
Expand All @@ -2369,7 +2363,7 @@ mod tests {
.with_path("age", &DataType::Int64)?
.build();
let result2 = shred_variant(&input, &schema2).unwrap();
let value_field2 = result2.value_column().unwrap();
let value_field2 = result2.value_column();
assert!(!value_field2.is_null(0)); // should contain {"score": 95.5}

// Test with schema containing all fields
Expand All @@ -2379,7 +2373,7 @@ mod tests {
.with_path("score", &DataType::Float64)?
.build();
let result3 = shred_variant(&input, &schema3).unwrap();
let value_field3 = result3.value_column().unwrap();
let value_field3 = result3.value_column();
assert!(value_field3.is_null(0)); // fully shredded, no remaining fields

Ok(())
Expand Down Expand Up @@ -2426,12 +2420,11 @@ mod tests {

let result = shred_variant(&input, &target_schema).unwrap();

assert!(result.value_column().is_some());
assert!(result.typed_value_column().is_some());
assert_eq!(result.len(), 6);

let metadata = result.metadata_column();
let value = result.value_column().unwrap();
let value = result.value_column();
let typed_value = result
.typed_value_column()
.unwrap()
Expand All @@ -2446,14 +2439,14 @@ mod tests {
ShreddedVariantFieldArray::try_new(typed_value.column_by_name("session_id").unwrap())
.unwrap();

let id_value = id_field.value_column().unwrap();
let id_value = id_field.value_column();
let id_typed_value = id_field
.typed_value_column()
.unwrap()
.as_any()
.downcast_ref::<FixedSizeBinaryArray>()
.unwrap();
let session_id_value = session_id_field.value_column().unwrap();
let session_id_value = session_id_field.value_column();
let session_id_typed_value = session_id_field
.typed_value_column()
.unwrap()
Expand Down Expand Up @@ -2572,12 +2565,11 @@ mod tests {

// Test output structure correctness
assert_eq!(result.len(), input.len());
assert!(result.value_column().is_some());
assert!(result.typed_value_column().is_some());

// For primitive shredding, verify that value and typed_value are never both non-null
// (This rule applies to primitives; for objects, both can be non-null for partial shredding)
let value_field = result.value_column().unwrap();
let value_field = result.value_column();
let typed_value_field = result
.typed_value_column()
.unwrap()
Expand Down
Loading
Loading