This is an automated email from the ASF dual-hosted git repository.
alamb pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
The following commit(s) were added to refs/heads/main by this push:
new b46e7334b3 [Variant] make `value` mandatory field for
`VariantArray`/`ShreddingState` (#10318)
b46e7334b3 is described below
commit b46e7334b3d4233b35868d7d22b0807d3c38e147
Author: Konstantin Tarasov <[email protected]>
AuthorDate: Thu Jul 23 14:56:18 2026 -0400
[Variant] make `value` mandatory field for `VariantArray`/`ShreddingState`
(#10318)
# Which issue does this PR close?
<!--
We generally require a GitHub issue to be filed for all bug fixes and
enhancements and this helps us generate change logs for our releases.
You can link an issue to this PR using the GitHub syntax.
-->
- Closes #10306.
# Rationale for this change
arrow-rs doesn't follow the Variant spec by making `value` field
optional.
<!--
Why are you proposing this change? If this is already explained clearly
in the issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand
your changes and offer better suggestions for fixes.
-->
# What changes are included in this PR?
- Make `value` field mandatory for `VariantArray` and `ShreddingState`.
- Fix the related APIs and tests
- Some DRY thrown in there for tests
<!--
There is no need to duplicate the description in the issue here but it
is sometimes worth providing a summary of the individual changes in this
PR.
-->
# Are these changes tested?
- Yes, fixed the unit tests
<!--
We typically require tests for all PRs in order to:
1. Prevent the code from being accidentally broken by subsequent changes
2. Serve as another way to document the expected behavior of the code
If tests are not included in your PR, please explain why (for example,
are they covered by existing tests)?
If this PR claims a performance improvement, please include evidence
such as benchmark results.
-->
# Are there any user-facing changes?
- Yes, `value` field is now mandatory for Variant
<!--
If there are user-facing changes then we may require documentation to be
updated before approving the PR.
If there are any breaking changes to public APIs, please call them out.
-->
---
parquet-variant-compute/src/from_json.rs | 6 +-
parquet-variant-compute/src/shred_variant.rs | 90 +++---
parquet-variant-compute/src/unshred_variant.rs | 14 +-
parquet-variant-compute/src/variant_array.rs | 348 ++++++++++-----------
.../src/variant_array_builder.rs | 4 +-
parquet-variant-compute/src/variant_get.rs | 182 +++++------
parquet-variant-compute/src/variant_to_arrow.rs | 4 +-
parquet/tests/variant_integration.rs | 8 +
8 files changed, 307 insertions(+), 349 deletions(-)
diff --git a/parquet-variant-compute/src/from_json.rs
b/parquet-variant-compute/src/from_json.rs
index 8e8c0ea9cf..93df1b281d 100644
--- a/parquet-variant-compute/src/from_json.rs
+++ b/parquet-variant-compute/src/from_json.rs
@@ -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));
@@ -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));
@@ -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));
diff --git a/parquet-variant-compute/src/shred_variant.rs
b/parquet-variant-compute/src/shred_variant.rs
index 7753fce0f6..495728b9a4 100644
--- a/parquet-variant-compute/src/shred_variant.rs
+++ b/parquet-variant-compute/src/shred_variant.rs
@@ -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,
@@ -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,
))
@@ -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() {
@@ -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,
@@ -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);
@@ -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 {
@@ -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));
}
}
@@ -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!(
@@ -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]
@@ -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()
@@ -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()
@@ -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()
@@ -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();
@@ -1781,7 +1776,7 @@ 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,
@@ -1789,7 +1784,7 @@ mod tests {
)));
let outer_variant = VariantArray::from_parts(
outer_metadata,
- Some(outer_fallbacks.clone()),
+ outer_fallbacks.clone(),
Some(Arc::new(outer_values.clone())),
None,
);
@@ -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()
@@ -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()
@@ -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()
@@ -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()
@@ -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()
@@ -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,
),
@@ -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
@@ -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
@@ -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(())
@@ -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()
@@ -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()
@@ -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()
diff --git a/parquet-variant-compute/src/unshred_variant.rs
b/parquet-variant-compute/src/unshred_variant.rs
index 30f861ce1e..08c58fe0f5 100644
--- a/parquet-variant-compute/src/unshred_variant.rs
+++ b/parquet-variant-compute/src/unshred_variant.rs
@@ -59,7 +59,7 @@ use uuid::Uuid;
/// - If unsupported data types are encountered in typed_value columns
pub fn unshred_variant(array: &VariantArray) -> Result<VariantArray> {
// Check if already unshredded (optimization for common case)
- if array.typed_value_column().is_none() && array.value_column().is_some() {
+ if array.typed_value_column().is_none() {
return Ok(array.clone());
}
@@ -89,7 +89,7 @@ pub fn unshred_variant(array: &VariantArray) ->
Result<VariantArray> {
let value = value_builder.build()?;
Ok(VariantArray::from_parts(
metadata.clone(),
- Some(Arc::new(value)),
+ Arc::new(value),
None,
nulls.cloned(),
))
@@ -715,7 +715,7 @@ mod tests {
Some("world"),
]));
- let variant_array = VariantArray::from_parts(metadata, None,
Some(typed_value), None);
+ let variant_array = VariantArray::perfectly_shredded(metadata,
typed_value, None);
let result = crate::unshred_variant(&variant_array).unwrap();
@@ -737,7 +737,7 @@ mod tests {
Some("world"),
]));
- let variant_array = VariantArray::from_parts(metadata, None,
Some(typed_value), None);
+ let variant_array = VariantArray::perfectly_shredded(metadata,
typed_value, None);
let result = crate::unshred_variant(&variant_array).unwrap();
@@ -759,7 +759,7 @@ mod tests {
&b"\xde\xad\xbe\xef"[..],
]));
- let variant_array = VariantArray::from_parts(metadata, None,
Some(typed_value), None);
+ let variant_array = VariantArray::perfectly_shredded(metadata,
typed_value, None);
let result = crate::unshred_variant(&variant_array).unwrap();
@@ -781,7 +781,7 @@ mod tests {
&b"\xde\xad\xbe\xef"[..],
]));
- let variant_array = VariantArray::from_parts(metadata, None,
Some(typed_value), None);
+ let variant_array = VariantArray::perfectly_shredded(metadata,
typed_value, None);
let result = crate::unshred_variant(&variant_array).unwrap();
@@ -801,7 +801,7 @@ mod tests {
let typed_value: ArrayRef =
Arc::new(StringViewArray::from(vec![Some("hello")]));
- let variant_array = VariantArray::from_parts(metadata, None,
Some(typed_value), None);
+ let variant_array = VariantArray::perfectly_shredded(metadata,
typed_value, None);
let result = crate::unshred_variant(&variant_array);
diff --git a/parquet-variant-compute/src/variant_array.rs
b/parquet-variant-compute/src/variant_array.rs
index 82cda7867f..ef72359ed7 100644
--- a/parquet-variant-compute/src/variant_array.rs
+++ b/parquet-variant-compute/src/variant_array.rs
@@ -22,7 +22,7 @@ use crate::type_conversion::{
generic_conversion_single_value,
generic_conversion_single_value_with_result,
primitive_conversion_single_value,
};
-use arrow::array::{Array, ArrayRef, AsArray, StructArray};
+use arrow::array::{Array, ArrayRef, AsArray, StructArray, new_null_array};
use arrow::buffer::NullBuffer;
use arrow::compute::cast;
use arrow::datatypes::{
@@ -63,6 +63,15 @@ pub(crate) fn variant_from_arrays_at<'m, 'v>(
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() {
@@ -278,7 +287,7 @@ impl VariantArray {
/// 1. A required field named `metadata` which is binary, large_binary, or
/// binary_view
///
- /// 2. An optional field named `value` that is binary, large_binary, or
+ /// 2. A required field named `value` that is binary, large_binary, or
/// binary_view
///
/// 3. An optional field named `typed_value` which can be any primitive
type
@@ -308,24 +317,36 @@ impl VariantArray {
};
validate_binary_array(metadata_col.as_ref(), "metadata")?;
+ let shredding_state = ShreddingState::try_from(inner)?;
+
+ // `try_from` synthesizes an all-null `value` when the input omits it.
Rebuild the inner
+ // struct so `inner()` and write-back carry the column too.
+ if inner.column_by_name("value").is_none() {
+ return Ok(Self::from_parts(
+ metadata_col.clone(),
+ shredding_state.value_column().clone(),
+ shredding_state.typed_value_column().cloned(),
+ inner.nulls().cloned(),
+ ));
+ }
+
// Note these clones are cheap, they just bump the ref count
Ok(Self {
inner: inner.clone(),
metadata: metadata_col.clone(),
- shredding_state: ShreddingState::try_from(inner)?,
+ shredding_state,
})
}
pub(crate) fn from_parts(
metadata: ArrayRef,
- value: Option<ArrayRef>,
+ value: ArrayRef,
typed_value: Option<ArrayRef>,
nulls: Option<NullBuffer>,
) -> Self {
- let mut builder = StructArrayBuilder::new().with_field("metadata",
metadata.clone(), false);
- if let Some(value) = value.clone() {
- builder = builder.with_field("value", value, true);
- }
+ let mut builder = StructArrayBuilder::new()
+ .with_field("metadata", metadata.clone(), false)
+ .with_field("value", value.clone(), true);
if let Some(typed_value) = typed_value.clone() {
builder = builder.with_field_ref(typed_value_field(&typed_value),
typed_value);
}
@@ -396,25 +417,26 @@ impl VariantArray {
/// Note: Does not do deep validation of the [`Variant`], so it is up to
the
/// caller to ensure that the metadata and value were constructed
correctly.
pub fn try_value(&self, index: usize) -> Result<Variant<'_, '_>> {
- match (self.typed_value_column(), self.value_column()) {
+ let value = self.value_column();
+ match self.typed_value_column() {
// Always prefer typed_value, if available
- (Some(typed_value), value) if typed_value.is_valid(index) => {
- typed_value_to_variant(typed_value, value, index)
+ Some(typed_value) if typed_value.is_valid(index) => {
+ if !matches!(typed_value.data_type(), DataType::Struct(_)) &&
value.is_valid(index) {
+ // Only a partially shredded struct is allowed to have
values for both columns
+ panic!("Invalid variant, conflicting value and
typed_value");
+ }
+ typed_value_to_variant(typed_value, index)
}
// Otherwise fall back to value, if available
- (_, Some(value)) if value.is_valid(index) =>
variant_from_arrays_at(
- &self.metadata,
- value,
- index,
- )
- .ok_or_else(|| {
- ArrowError::InvalidArgumentError(format!(
- "metadata and value fields must be binary-like arrays,
instead got {} and {}",
- self.metadata.data_type(),
- value.data_type()
- ))
- }),
- // It is technically invalid for neither value nor typed_value
fields to be available,
+ _ if value.is_valid(index) =>
variant_from_arrays_at(&self.metadata, value, index)
+ .ok_or_else(|| {
+ ArrowError::InvalidArgumentError(format!(
+ "metadata and value fields must be binary-like arrays,
instead got {} and {}",
+ self.metadata.data_type(),
+ value.data_type()
+ ))
+ }),
+ // It is technically invalid for both value and typed_value to be
null,
// but the spec specifically requires readers to return
Variant::Null in this case.
_ => Ok(Variant::Null),
}
@@ -425,8 +447,8 @@ impl VariantArray {
&self.metadata
}
- /// Return a reference to the `value` column of the [`StructArray`], if
present
- pub fn value_column(&self) -> Option<&ArrayRef> {
+ /// Return a reference to the `value` column of the [`StructArray`]
+ pub fn value_column(&self) -> &ArrayRef {
self.shredding_state.value_column()
}
@@ -663,7 +685,7 @@ impl ShreddedVariantFieldArray {
///
/// # Requirements of the `StructArray`
///
- /// 1. An optional field named `value` that is binary, large_binary, or
+ /// 1. A required field named `value` that is binary, large_binary, or
/// binary_view
///
/// 2. An optional field named `typed_value` which can be any primitive
type
@@ -676,10 +698,22 @@ impl ShreddedVariantFieldArray {
));
};
+ let shredding_state = ShreddingState::try_from(inner_struct)?;
+
+ // `try_from` synthesizes an all-null `value` when the input omits it.
Rebuild the inner
+ // struct so `inner()` and write-back carry the column too (see
`VariantArray::try_new`).
+ if inner_struct.column_by_name("value").is_none() {
+ return Ok(Self::from_parts(
+ shredding_state.value_column().clone(),
+ shredding_state.typed_value_column().cloned(),
+ inner_struct.nulls().cloned(),
+ ));
+ }
+
// Note this clone is cheap, it just bumps the ref count
Ok(Self {
inner: inner_struct.clone(),
- shredding_state: ShreddingState::try_from(inner_struct)?,
+ shredding_state,
})
}
@@ -688,8 +722,8 @@ impl ShreddedVariantFieldArray {
&self.shredding_state
}
- /// Return a reference to the `value` column of the [`StructArray`], if
present
- pub fn value_column(&self) -> Option<&ArrayRef> {
+ /// Return a reference to the `value` column of the [`StructArray`]
+ pub fn value_column(&self) -> &ArrayRef {
self.shredding_state.value_column()
}
@@ -704,14 +738,11 @@ impl ShreddedVariantFieldArray {
}
pub(crate) fn from_parts(
- value: Option<ArrayRef>,
+ value: ArrayRef,
typed_value: Option<ArrayRef>,
nulls: Option<NullBuffer>,
) -> Self {
- let mut builder = StructArrayBuilder::new();
- if let Some(value) = value.clone() {
- builder = builder.with_field("value", value, true);
- }
+ let mut builder = StructArrayBuilder::new().with_field("value",
value.clone(), true);
if let Some(typed_value) = typed_value.clone() {
builder = builder.with_field_ref(typed_value_field(&typed_value),
typed_value);
}
@@ -781,9 +812,10 @@ impl From<ShreddedVariantFieldArray> for StructArray {
/// Shredding Spec]. Shredding means that the actual value is stored in a typed
/// `typed_field` instead of the generic `value` field.
///
-/// Both value and typed_value are optional fields used together to encode a
-/// single value. Values in the two fields must be interpreted according to the
-/// following table (see [Parquet Variant Shredding Spec] for more details):
+/// The `value` column is always present (the spec requires writers to emit
+/// it); `typed_value` is optional. Values in the two columns must be
+/// interpreted according to the following table (see [Parquet Variant
+/// Shredding Spec] for more details):
///
/// | value | typed_value | Meaning |
/// |----------|--------------|---------|
@@ -797,10 +829,12 @@ impl From<ShreddedVariantFieldArray> for StructArray {
///
/// | value | typed_value | Meaning |
/// |--------|-------------|---------|
-/// | -- | -- | **Missing**: The value is always missing; only
valid for shredded object fields |
/// | exists | -- | **Unshredded**: If present, the value may be any
type, including [`Variant::Null`]
-/// | -- | exists | **Perfectly shredded**: If present, the value is
always the shredded type |
-/// | exists | exists | **Imperfectly shredded**: The value might (not)
be present and might (not) be the shredded type |
+/// | exists | exists | **Shredded**: perfectly if `value` is all-null,
otherwise imperfectly |
+///
+/// Note the spec requires the `value` column to always be present in the
+/// schema; structs without one are rejected
+/// (see <https://github.com/apache/arrow-rs/issues/10306>).
///
/// NOTE: Partial shredding is a row-wise situation that can arise under
imperfect shredding (a
/// column-wise situation): When both columns exist (imperfect shredding) and
the typed_value column
@@ -810,7 +844,7 @@ impl From<ShreddedVariantFieldArray> for StructArray {
/// [Parquet Variant Shredding Spec]:
https://github.com/apache/parquet-format/blob/master/VariantShredding.md#value-shredding
#[derive(Debug, Clone)]
pub struct ShreddingState {
- value: Option<ArrayRef>,
+ value: ArrayRef,
typed_value: Option<ArrayRef>,
}
@@ -829,13 +863,13 @@ impl ShreddingState {
/// let struct_array: StructArray = get_struct_array();
/// let shredding_state = ShreddingState::try_from(&struct_array).unwrap();
/// ```
- pub fn new(value: Option<ArrayRef>, typed_value: Option<ArrayRef>) -> Self
{
+ pub fn new(value: ArrayRef, typed_value: Option<ArrayRef>) -> Self {
Self { value, typed_value }
}
- /// Return a reference to the `value` column, if present
- pub fn value_column(&self) -> Option<&ArrayRef> {
- self.value.as_ref()
+ /// Return a reference to the `value` column
+ pub fn value_column(&self) -> &ArrayRef {
+ &self.value
}
/// Return a reference to the `typed_value` column, if present
@@ -846,7 +880,7 @@ impl ShreddingState {
/// Slice all the underlying arrays
pub fn slice(&self, offset: usize, length: usize) -> Self {
Self {
- value: self.value.as_ref().map(|v| v.slice(offset, length)),
+ value: self.value.slice(offset, length),
typed_value: self.typed_value.as_ref().map(|tv| tv.slice(offset,
length)),
}
}
@@ -856,14 +890,21 @@ impl TryFrom<&StructArray> for ShreddingState {
type Error = ArrowError;
fn try_from(inner_struct: &StructArray) -> Result<Self> {
- // The `value` column need not exist, but if it does it must be a
binary type.
- let value = if let Some(value_col) =
inner_struct.column_by_name("value") {
- validate_binary_array(value_col.as_ref(), "value")?;
- Some(value_col.clone())
- } else {
- None
- };
let typed_value = inner_struct.column_by_name("typed_value").cloned();
+ let value = match inner_struct.column_by_name("value") {
+ Some(value) => {
+ validate_binary_array(value.as_ref(), "value")?;
+ value.clone()
+ }
+ // Lenient read: a shredded group may omit the spec-required
`value`. Synthesize an
+ // all-null one so the model always has a `value`.
+ None if typed_value.is_some() =>
all_null_value_column(inner_struct.len()),
+ None => {
+ return Err(ArrowError::InvalidArgumentError(
+ "Invalid VariantArray: StructArray must contain a 'value'
field".to_string(),
+ ));
+ }
+ };
Ok(ShreddingState::new(value, typed_value))
}
}
@@ -931,16 +972,8 @@ impl StructArrayBuilder {
}
/// returns the non-null element at index as a Variant
-fn typed_value_to_variant<'a>(
- typed_value: &'a ArrayRef,
- value: Option<&'a ArrayRef>,
- index: usize,
-) -> Result<Variant<'a, 'a>> {
+fn typed_value_to_variant(typed_value: &ArrayRef, index: usize) ->
Result<Variant<'_, '_>> {
let data_type = typed_value.data_type();
- if value.is_some_and(|v| !matches!(data_type, DataType::Struct(_)) &&
v.is_valid(index)) {
- // Only a partially shredded struct is allowed to have values for both
columns
- panic!("Invalid variant, conflicting value and typed_value");
- }
match data_type {
DataType::Null => Ok(Variant::Null),
DataType::Boolean => {
@@ -1244,6 +1277,28 @@ fn canonicalize_and_verify_field(field: &Arc<Field>) ->
Result<Cow<'_, Arc<Field
Ok(Cow::Owned(Arc::new(new_field)))
}
+/// Test-only constructors for perfectly shredded arrays, where every value
lives in
+/// `typed_value` and the required `value` column is all-null.
+#[cfg(test)]
+impl VariantArray {
+ pub(crate) fn perfectly_shredded(
+ metadata: ArrayRef,
+ typed_value: ArrayRef,
+ nulls: Option<NullBuffer>,
+ ) -> Self {
+ let value = all_null_value_column(typed_value.len());
+ Self::from_parts(metadata, value, Some(typed_value), nulls)
+ }
+}
+
+#[cfg(test)]
+impl ShreddedVariantFieldArray {
+ pub(crate) fn perfectly_shredded(typed_value: ArrayRef) -> Self {
+ let value = all_null_value_column(typed_value.len());
+ Self::from_parts(value, Some(typed_value), None)
+ }
+}
+
#[cfg(test)]
mod test {
use crate::VariantArrayBuilder;
@@ -1283,30 +1338,36 @@ mod test {
}
#[test]
- fn all_null_missing_value_and_typed_value() {
- let fields = Fields::from(vec![Field::new("metadata",
DataType::BinaryView, false)]);
- let array = StructArray::new(fields, vec![make_binary_view_array()],
None);
+ fn read_missing_value_column() {
+ // With `typed_value` present, a missing `value` is read leniently: an
all-null `value` is
+ // synthesized and normalized into the inner struct (so a write-back
emits it), and values
+ // still decode from `typed_value`.
+ let typed_value = Arc::new(Int64Array::from(vec![Some(1), None,
Some(3)])) as ArrayRef;
+ let metadata =
+
BinaryViewArray::from_iter_values(std::iter::repeat_n(EMPTY_VARIANT_METADATA_BYTES,
3));
+ let struct_array = StructArrayBuilder::new()
+ .with_field("metadata", Arc::new(metadata), false)
+ .with_field("typed_value", typed_value, true)
+ .build();
+ assert!(struct_array.column_by_name("value").is_none());
- // NOTE: By strict spec interpretation, this case (top-level variant
with null/null)
- // should be invalid, but we currently allow it and treat it as
Variant::Null.
- // This is a pragmatic decision to handle missing data gracefully.
- let variant_array = VariantArray::try_new(&array).unwrap();
-
- // Verify the shredding state is AllNull
- assert!(matches!(
- variant_array.shredding_state(),
- ShreddingState {
- value: None,
- typed_value: None
- }
- ));
+ let variant_array = VariantArray::try_new(&struct_array).unwrap();
+ assert_eq!(variant_array.value_column().len(), 3);
+ assert_eq!(variant_array.value_column().null_count(), 3);
+ assert!(variant_array.inner().column_by_name("value").is_some());
+ assert!(variant_array.typed_value_column().is_some());
+ assert_eq!(variant_array.value(0), Variant::from(1i64));
+ assert_eq!(variant_array.value(1), Variant::Null);
+ assert_eq!(variant_array.value(2), Variant::from(3i64));
- // Verify that value() returns Variant::Null (compensating for spec
violation)
- for i in 0..variant_array.len() {
- if variant_array.is_valid(i) {
- assert_eq!(variant_array.value(i),
parquet_variant::Variant::Null);
- }
- }
+ // With no `typed_value` either, there is nothing to read, so `value`
stays required.
+ let fields = Fields::from(vec![Field::new("metadata",
DataType::BinaryView, false)]);
+ let metadata_only = StructArray::new(fields,
vec![make_binary_view_array()], None);
+ let err = VariantArray::try_new(&metadata_only);
+ assert_eq!(
+ err.unwrap_err().to_string(),
+ "Invalid argument error: Invalid VariantArray: StructArray must
contain a 'value' field"
+ );
}
#[test]
@@ -1358,8 +1419,10 @@ mod test {
EMPTY_VARIANT_METADATA_BYTES,
typed_value.len(),
));
+ let value = new_null_array(&DataType::BinaryView, typed_value.len());
StructArrayBuilder::new()
.with_field("metadata", Arc::new(metadata), false)
+ .with_field("value", value, true)
.with_field("typed_value", typed_value, true)
.build()
}
@@ -1400,88 +1463,27 @@ mod test {
}
#[test]
- fn all_null_shredding_state() {
- // Verify the shredding state is AllNull
- assert!(matches!(
- ShreddingState::new(None, None),
- ShreddingState {
- value: None,
- typed_value: None
- }
- ));
- }
-
- #[test]
- fn all_null_variant_array_construction() {
+ fn all_null_value_column_is_valid_and_unshredded() {
+ // An all-null `value` column is accepted (only a *missing* column is
+ // rejected) and the array is unshredded (no typed_value column)
let metadata = BinaryViewArray::from(vec![b"test" as &[u8]; 3]);
- let nulls = NullBuffer::from(vec![false, false, false]); // all null
-
- let fields = Fields::from(vec![Field::new("metadata",
DataType::BinaryView, false)]);
- let struct_array = StructArray::new(fields, vec![Arc::new(metadata)],
Some(nulls));
-
- let variant_array = VariantArray::try_new(&struct_array).unwrap();
-
- // Verify the shredding state is AllNull
- assert!(matches!(
- variant_array.shredding_state(),
- ShreddingState {
- value: None,
- typed_value: None
- }
- ));
-
- // Verify all values are null
- assert_eq!(variant_array.len(), 3);
- assert!(!variant_array.is_valid(0));
- assert!(!variant_array.is_valid(1));
- assert!(!variant_array.is_valid(2));
-
- // Verify that value() returns Variant::Null for all indices
- for i in 0..variant_array.len() {
- assert!(
- !variant_array.is_valid(i),
- "Expected value at index {i} to be null"
- );
- }
- }
-
- #[test]
- fn value_field_present_but_all_null_should_be_unshredded() {
- // This test demonstrates the issue: when a value field exists in
schema
- // but all its values are null, it should remain Unshredded, not
AllNull
- let metadata = BinaryViewArray::from(vec![b"test" as &[u8]; 3]);
-
- // Create a value field with all null values
- let value_nulls = NullBuffer::from(vec![false, false, false]); // all
null
- let value_array = BinaryViewArray::from_iter_values(vec![""; 3]);
- let value_data = value_array
- .to_data()
- .into_builder()
- .nulls(Some(value_nulls))
- .build()
- .unwrap();
- let value = BinaryViewArray::from(value_data);
+ let value = new_null_array(&DataType::BinaryView, 3);
let fields = Fields::from(vec![
Field::new("metadata", DataType::BinaryView, false),
- Field::new("value", DataType::BinaryView, true), // Field exists
in schema
+ Field::new("value", DataType::BinaryView, true),
]);
- let struct_array = StructArray::new(
- fields,
- vec![Arc::new(metadata), Arc::new(value)],
- None, // struct itself is not null, just the value field is all
null
- );
+ let struct_array = StructArray::new(fields, vec![Arc::new(metadata),
value], None);
let variant_array = VariantArray::try_new(&struct_array).unwrap();
+ assert!(variant_array.typed_value_column().is_none());
- // This should be Unshredded, not AllNull, because value field exists
in schema
- assert!(matches!(
- variant_array.shredding_state(),
- ShreddingState {
- value: Some(_),
- typed_value: None
- }
- ));
+ // The rows are valid but have neither value nor typed_value; the spec
+ // requires readers to return Variant::Null in this case
+ for i in 0..variant_array.len() {
+ assert!(variant_array.is_valid(i));
+ assert_eq!(variant_array.value(i), Variant::Null);
+ }
}
#[test]
@@ -1715,15 +1717,8 @@ mod test {
#[test]
fn binary_typed_value_roundtrips() {
// Verify that a shredded variant with Binary typed_value can be read
back
- let metadata: ArrayRef = Arc::new(BinaryViewArray::from_iter_values([
- EMPTY_VARIANT_METADATA_BYTES,
- ]));
let typed_value: ArrayRef = Arc::new(BinaryArray::from(vec![b"hello"
as &[u8]]));
-
- let struct_array = StructArrayBuilder::new()
- .with_field("metadata", metadata, false)
- .with_field("typed_value", typed_value, true)
- .build();
+ let struct_array = make_variant_struct_with_typed_value(typed_value);
let variant_array = VariantArray::try_new(&struct_array).unwrap();
assert_eq!(variant_array.value(0), Variant::from(b"hello" as &[u8]));
@@ -1732,15 +1727,8 @@ mod test {
#[test]
fn large_binary_typed_value_roundtrips() {
// Verify that a shredded variant with LargeBinary typed_value can be
read back
- let metadata: ArrayRef = Arc::new(BinaryViewArray::from_iter_values([
- EMPTY_VARIANT_METADATA_BYTES,
- ]));
let typed_value: ArrayRef =
Arc::new(LargeBinaryArray::from(vec![b"world" as &[u8]]));
-
- let struct_array = StructArrayBuilder::new()
- .with_field("metadata", metadata, false)
- .with_field("typed_value", typed_value, true)
- .build();
+ let struct_array = make_variant_struct_with_typed_value(typed_value);
let variant_array = VariantArray::try_new(&struct_array).unwrap();
assert_eq!(variant_array.value(0), Variant::from(b"world" as &[u8]));
@@ -1750,16 +1738,10 @@ mod test {
($fn_name: ident, $invalid_typed_value: expr, $error_msg: literal) => {
#[test]
fn $fn_name() {
- let metadata =
BinaryViewArray::from_iter_values(std::iter::repeat_n(
- EMPTY_VARIANT_METADATA_BYTES,
- 1,
- ));
let invalid_typed_value = $invalid_typed_value;
- let struct_array = StructArrayBuilder::new()
- .with_field("metadata", Arc::new(metadata), false)
- .with_field("typed_value", Arc::new(invalid_typed_value),
true)
- .build();
+ let struct_array =
+
make_variant_struct_with_typed_value(Arc::new(invalid_typed_value));
let array: VariantArray = VariantArray::try_new(&struct_array)
.expect("should create variant array")
diff --git a/parquet-variant-compute/src/variant_array_builder.rs
b/parquet-variant-compute/src/variant_array_builder.rs
index f43e34d0cd..bc2d7d370c 100644
--- a/parquet-variant-compute/src/variant_array_builder.rs
+++ b/parquet-variant-compute/src/variant_array_builder.rs
@@ -501,7 +501,7 @@ mod test {
// the metadata and value fields of non shredded variants should not
be null
assert!(variant_array.metadata_column().nulls().is_none());
- assert!(variant_array.value_column().unwrap().nulls().is_none());
+ assert!(variant_array.value_column().nulls().is_none());
let DataType::Struct(fields) = variant_array.data_type() else {
panic!("Expected VariantArray to have Struct data type");
};
@@ -647,7 +647,7 @@ mod test {
let array2 = VariantArray::from_parts(
array.metadata_column().clone(),
- Some(Arc::new(value_builder.build().unwrap())),
+ Arc::new(value_builder.build().unwrap()),
None,
None,
);
diff --git a/parquet-variant-compute/src/variant_get.rs
b/parquet-variant-compute/src/variant_get.rs
index 59407c617c..bd28f5c8b7 100644
--- a/parquet-variant-compute/src/variant_get.rs
+++ b/parquet-variant-compute/src/variant_get.rs
@@ -28,6 +28,7 @@ use arrow_schema::{ArrowError, DataType, FieldRef};
use parquet_variant::{VariantPath, VariantPathElement};
use crate::ShreddingState;
+use crate::variant_array::all_null_value_column;
use crate::variant_to_arrow::make_variant_to_arrow_row_builder;
use crate::{VariantArray, VariantType, unshred_variant};
@@ -37,8 +38,8 @@ use std::sync::Arc;
pub(crate) enum ShreddedPathStep {
/// Path step succeeded, return the new shredding state
Success(ShreddingState),
- /// The path element is not present in the `typed_value` column and there
is no `value` column,
- /// so we know it does not exist. It, and all paths under it, are all-NULL.
+ /// The path element is not present in the `typed_value` column and the
`value` column is
+ /// all-null, so we know it does not exist. It, and all paths under it,
are all-NULL.
Missing,
/// The path element is not present in the `typed_value` column and must
be retrieved from the `value`
/// column instead. The caller should be prepared to handle any value,
including the requested
@@ -70,8 +71,8 @@ fn take_list_like_index_as_shredding_state<L: ListLikeArray +
'static>(
let value_array = shredding_state.value_column();
let typed_array = shredding_state.typed_value_column();
- // If list elements have neither typed nor fallback value, this path step
is missing.
- if value_array.is_none() && typed_array.is_none() {
+ // If list elements have neither typed nor fallback values, this path step
is missing.
+ if typed_array.is_none() && value_array.null_count() == value_array.len() {
return Ok(None);
}
@@ -85,9 +86,7 @@ fn take_list_like_index_as_shredding_state<L: ListLikeArray +
'static>(
let index_array = UInt64Array::from(take_indices);
// Gather both typed and fallback values at the requested element index.
- let taken_value = value_array
- .map(|value| take(value, &index_array, None))
- .transpose()?;
+ let taken_value = take(value_array, &index_array, None)?;
let taken_typed = typed_array
.map(|typed| take(typed, &index_array, None))
.transpose()?;
@@ -115,11 +114,15 @@ pub(crate) fn follow_shredded_path_element(
path_element: &VariantPathElement<'_>,
_cast_options: &CastOptions,
) -> Result<ShreddedPathStep> {
- // If the requested path element is not present in `typed_value`, and
`value` is missing, then
+ // If the requested path element is not present in `typed_value`, and
`value` is all-null, then
// we know it does not exist; it, and all paths under it, are all-NULL.
- let missing_path_step = || match shredding_state.value_column() {
- Some(_) => ShreddedPathStep::NotShredded,
- None => ShreddedPathStep::Missing,
+ let missing_path_step = || {
+ let value = shredding_state.value_column();
+ if value.null_count() == value.len() {
+ ShreddedPathStep::Missing
+ } else {
+ ShreddedPathStep::NotShredded
+ }
};
let Some(typed_value) = shredding_state.typed_value_column() else {
@@ -194,9 +197,7 @@ fn shredded_get_path(
// Helper that creates a new VariantArray from the given nested value and
typed_value columns,
// properly accounting for accumulated nulls from path traversal
let make_target_variant =
- |value: Option<ArrayRef>,
- typed_value: Option<ArrayRef>,
- accumulated_nulls: Option<NullBuffer>| {
+ |value: ArrayRef, typed_value: Option<ArrayRef>, accumulated_nulls:
Option<NullBuffer>| {
let metadata = input.metadata_column().clone();
VariantArray::from_parts(metadata, value, typed_value,
accumulated_nulls)
};
@@ -291,7 +292,12 @@ fn shredded_get_path(
// Propagating metadata is not necessary for an all-NULL
array, but is cheaper than constructing
// a new empty metadata array. (n * 3 bytes vs Arc bump)
let metadata = input.metadata_column().clone();
- let arr = VariantArray::from_parts(metadata, None, None,
all_nulls);
+ let arr = VariantArray::from_parts(
+ metadata,
+ all_null_value_column(num_rows),
+ None,
+ all_nulls,
+ );
return Ok(ArrayRef::from(arr));
}
let arr = match as_field.map(|f| f.data_type()) {
@@ -302,7 +308,7 @@ fn shredded_get_path(
}
ShreddedPathStep::NotShredded => {
let target = make_target_variant(
- shredding_state.value_column().cloned(),
+ shredding_state.value_column().clone(),
None,
accumulated_nulls,
);
@@ -313,7 +319,7 @@ fn shredded_get_path(
// Path exhausted! Create a new `VariantArray` for the location we landed
on.
let target = make_target_variant(
- shredding_state.value_column().cloned(),
+ shredding_state.value_column().clone(),
shredding_state.typed_value_column().cloned(),
accumulated_nulls,
);
@@ -381,14 +387,11 @@ fn try_perfect_shredding(variant_array: &VariantArray,
as_field: &Field) -> Opti
}
let typed_value = variant_array.typed_value_column()?;
- if typed_value.data_type() == as_field.data_type()
- && variant_array
- .value_column()
- .is_none_or(|v| v.null_count() == v.len())
- {
- // Here we need to gate against the case where the `typed_value` is
null but data is in the `value` column.
- // 1. If the `value` column is null, or
- // 2. If every row in the `value` column is null
+ let value = variant_array.value_column();
+ if typed_value.data_type() == as_field.data_type() && value.null_count()
== value.len() {
+ // Here we need to gate against the case where the `typed_value` is
null
+ // but data is in the `value` column: only an all-null `value` column
+ // qualifies as perfect shredding.
// This is a perfect shredding, where the value is entirely shredded
out,
// so we can just return the typed value after merging the accumulated
nulls.
@@ -484,7 +487,9 @@ mod test {
use std::sync::Arc;
use super::{GetOptions, requested_field_is_shredded, variant_get};
- use crate::variant_array::{ShreddedVariantFieldArray, StructArrayBuilder};
+ use crate::variant_array::{
+ ShreddedVariantFieldArray, StructArrayBuilder, all_null_value_column,
+ };
use crate::{
ShreddedSchemaBuilder, VariantArray, VariantArrayBuilder,
cast_to_variant, json_to_variant,
shred_variant,
@@ -1198,7 +1203,7 @@ mod test {
EMPTY_VARIANT_METADATA_BYTES,
typed_value.len(),
));
- VariantArray::from_parts(Arc::new(metadata), None,
Some(typed_value), None).into()
+ VariantArray::perfectly_shredded(Arc::new(metadata),
typed_value, None).into()
}
};
}
@@ -1868,7 +1873,7 @@ mod test {
ArrayRef::from(VariantArray::from_parts(
Arc::new(metadata),
- None,
+ all_null_value_column(3),
None,
Some(nulls),
))
@@ -2161,11 +2166,9 @@ mod test {
let x_field_typed_value = Int32Array::from(vec![Some(1), Some(42)]);
// For perfect shredding of the x field, no "value" column, only
typed_value
- let x_field_shredded = ShreddedVariantFieldArray::from_parts(
- None,
- Some(Arc::new(x_field_typed_value) as ArrayRef),
- None,
- );
+ let x_field_shredded =
ShreddedVariantFieldArray::perfectly_shredded(Arc::new(
+ x_field_typed_value,
+ ) as ArrayRef);
// Create the main typed_value as a struct containing the "x" field
let typed_value_fields = Fields::from(vec![Field::new(
@@ -2183,7 +2186,7 @@ mod test {
// Create the main VariantArray
ArrayRef::from(VariantArray::from_parts(
Arc::new(metadata_array),
- Some(Arc::new(value_array)),
+ Arc::new(value_array),
Some(Arc::new(typed_value_struct)),
None,
))
@@ -2454,7 +2457,7 @@ mod test {
let result_variant = VariantArray::try_new(&result).unwrap();
assert!(result_variant.typed_value_column().is_none());
- assert!(result_variant.value_column().is_some());
+ assert!(result_variant.value_column().null_count() <
result_variant.len());
let expected_json: ArrayRef = Arc::new(StringArray::from(vec![
Some(r#"{"k":100000}"#),
@@ -2618,11 +2621,9 @@ mod test {
let x_field_typed_value = Int32Array::from(vec![Some(42), None]);
// For the x field, only typed_value (perfect shredding when possible)
- let x_field_shredded = ShreddedVariantFieldArray::from_parts(
- None,
- Some(Arc::new(x_field_typed_value) as ArrayRef),
- None,
- );
+ let x_field_shredded =
ShreddedVariantFieldArray::perfectly_shredded(Arc::new(
+ x_field_typed_value,
+ ) as ArrayRef);
// Create the main typed_value as a struct containing the "x" field
let typed_value_fields = Fields::from(vec![Field::new(
@@ -2640,7 +2641,7 @@ mod test {
// Build final VariantArray
ArrayRef::from(VariantArray::from_parts(
Arc::new(metadata_array),
- Some(Arc::new(value_array)),
+ Arc::new(value_array),
Some(Arc::new(typed_value_struct)),
None,
))
@@ -2698,11 +2699,8 @@ mod test {
// Create the nested shredded structure
// Level 2: x field (the deepest level)
let x_typed_value = Int32Array::from(vec![Some(55), None]);
- let x_field_shredded = ShreddedVariantFieldArray::from_parts(
- None,
- Some(Arc::new(x_typed_value) as ArrayRef),
- None,
- );
+ let x_field_shredded =
+
ShreddedVariantFieldArray::perfectly_shredded(Arc::new(x_typed_value) as
ArrayRef);
// Level 1: a field containing x field + value field for fallbacks
// The "a" field needs both typed_value (for shredded x) and value
(for fallback cases)
@@ -2730,7 +2728,7 @@ mod test {
.unwrap(),
) as ArrayRef;
let a_field_shredded = ShreddedVariantFieldArray::from_parts(
- Some(Arc::new(a_value_array)),
+ Arc::new(a_value_array),
Some(a_inner_typed_value),
None,
);
@@ -2751,7 +2749,7 @@ mod test {
// Build final VariantArray
ArrayRef::from(VariantArray::from_parts(
Arc::new(metadata_array),
- Some(Arc::new(value_array)),
+ Arc::new(value_array),
Some(Arc::new(typed_value_struct)),
None,
))
@@ -2802,11 +2800,8 @@ mod test {
// Level 3: x field (deepest level)
let x_typed_value = Int32Array::from(vec![Some(100), None, None]);
- let x_field_shredded = ShreddedVariantFieldArray::from_parts(
- None,
- Some(Arc::new(x_typed_value) as ArrayRef),
- None,
- );
+ let x_field_shredded =
+
ShreddedVariantFieldArray::perfectly_shredded(Arc::new(x_typed_value) as
ArrayRef);
// Level 2: b field containing x field + value field
let b_value_data = {
@@ -2832,7 +2827,7 @@ mod test {
.unwrap(),
) as ArrayRef;
let b_field_shredded = ShreddedVariantFieldArray::from_parts(
- Some(Arc::new(b_value_array)),
+ Arc::new(b_value_array),
Some(b_inner_typed_value),
None,
);
@@ -2861,7 +2856,7 @@ mod test {
.unwrap(),
) as ArrayRef;
let a_field_shredded = ShreddedVariantFieldArray::from_parts(
- Some(Arc::new(a_value_array)),
+ Arc::new(a_value_array),
Some(a_inner_typed_value),
None,
);
@@ -2882,7 +2877,7 @@ mod test {
// Build final VariantArray
ArrayRef::from(VariantArray::from_parts(
Arc::new(metadata_array),
- Some(Arc::new(value_array)),
+ Arc::new(value_array),
Some(Arc::new(typed_value_struct)),
None,
))
@@ -3634,27 +3629,21 @@ mod test {
// Create shredded fields with different null patterns
// Field "a": present in rows 0,3 (missing in rows 1,2,4)
let a_field_typed_value = Int32Array::from(vec![Some(1), None, None,
Some(1), None]);
- let a_field_shredded = ShreddedVariantFieldArray::from_parts(
- None,
- Some(Arc::new(a_field_typed_value) as ArrayRef),
- None,
- );
+ let a_field_shredded =
ShreddedVariantFieldArray::perfectly_shredded(Arc::new(
+ a_field_typed_value,
+ ) as ArrayRef);
// Field "b": present in rows 0,2 (missing in rows 1,3,4)
let b_field_typed_value = Int32Array::from(vec![Some(2), None,
Some(2), None, None]);
- let b_field_shredded = ShreddedVariantFieldArray::from_parts(
- None,
- Some(Arc::new(b_field_typed_value) as ArrayRef),
- None,
- );
+ let b_field_shredded =
ShreddedVariantFieldArray::perfectly_shredded(Arc::new(
+ b_field_typed_value,
+ ) as ArrayRef);
// Field "c": present in row 0 only (missing in all other rows)
let c_field_typed_value = Int32Array::from(vec![Some(3), None, None,
None, None]);
- let c_field_shredded = ShreddedVariantFieldArray::from_parts(
- None,
- Some(Arc::new(c_field_typed_value) as ArrayRef),
- None,
- );
+ let c_field_shredded =
ShreddedVariantFieldArray::perfectly_shredded(Arc::new(
+ c_field_typed_value,
+ ) as ArrayRef);
// Create main typed_value struct
let typed_value_fields = Fields::from(vec![
@@ -3674,10 +3663,9 @@ mod test {
.unwrap();
// Build final VariantArray with top-level nulls
- ArrayRef::from(VariantArray::from_parts(
+ ArrayRef::from(VariantArray::perfectly_shredded(
Arc::new(metadata_array),
- None,
- Some(Arc::new(typed_value_struct)),
+ Arc::new(typed_value_struct),
Some(nulls),
))
}
@@ -3690,11 +3678,8 @@ mod test {
// Create the inner level: contains typed_value with Int32 values
// Row 0: has value 42, Row 1: inner null, Row 2: outer null, Row 3:
top-level null
let inner_typed_value = Int32Array::from(vec![Some(42), None, None,
None]); // dummy value for row 2
- let inner = ShreddedVariantFieldArray::from_parts(
- None,
- Some(Arc::new(inner_typed_value) as ArrayRef),
- None,
- );
+ let inner =
+
ShreddedVariantFieldArray::perfectly_shredded(Arc::new(inner_typed_value) as
ArrayRef);
let outer_typed_value_nulls = NullBuffer::from(vec![
true, // row 0: inner struct exists with typed_value=42
@@ -3707,11 +3692,8 @@ mod test {
.with_nulls(outer_typed_value_nulls)
.build();
- let outer = ShreddedVariantFieldArray::from_parts(
- None,
- Some(Arc::new(outer_typed_value) as ArrayRef),
- None,
- );
+ let outer =
+
ShreddedVariantFieldArray::perfectly_shredded(Arc::new(outer_typed_value) as
ArrayRef);
let typed_value_nulls = NullBuffer::from(vec![
true, // row 0: inner struct exists with typed_value=42
@@ -3733,10 +3715,9 @@ mod test {
true, // row 2: outer field NULL
false, // row 3: top-level NULL
]);
- ArrayRef::from(VariantArray::from_parts(
+ ArrayRef::from(VariantArray::perfectly_shredded(
Arc::new(metadata_array),
- None,
- Some(Arc::new(typed_value)),
+ Arc::new(typed_value),
Some(nulls),
))
}
@@ -3788,11 +3769,9 @@ mod test {
// Create shredded field "x" (globally shredded - never appears in
value field)
// For top-level null row, the field still needs valid content (not
null)
let x_field_typed_value = Int32Array::from(vec![Some(1), Some(2),
Some(3), Some(0)]);
- let x_field_shredded = ShreddedVariantFieldArray::from_parts(
- None,
- Some(Arc::new(x_field_typed_value) as ArrayRef),
- None,
- );
+ let x_field_shredded =
ShreddedVariantFieldArray::perfectly_shredded(Arc::new(
+ x_field_typed_value,
+ ) as ArrayRef);
// Create main typed_value struct (only contains shredded fields)
let typed_value_struct = StructArrayBuilder::new()
@@ -3804,7 +3783,7 @@ mod test {
let variant_nulls = NullBuffer::from(vec![true, true, true, false]);
// Row 3 is top-level null
ArrayRef::from(VariantArray::from_parts(
Arc::new(metadata_array),
- Some(Arc::new(value_array)),
+ Arc::new(value_array),
Some(Arc::new(typed_value_struct)),
Some(variant_nulls),
))
@@ -4665,7 +4644,7 @@ mod test {
]));
let all_nulls_erased: ArrayRef = all_nulls_values.clone();
let all_nulls_field =
- ShreddedVariantFieldArray::from_parts(None,
Some(all_nulls_erased.clone()), None);
+
ShreddedVariantFieldArray::perfectly_shredded(all_nulls_erased.clone());
let all_nulls_type = all_nulls_field.data_type().clone();
let all_nulls_struct: ArrayRef = ArrayRef::from(all_nulls_field);
@@ -4674,7 +4653,7 @@ mod test {
Arc::new(Int32Array::from(vec![Some(10), None, Some(30)]));
let some_nulls_erased: ArrayRef = some_nulls_values.clone();
let some_nulls_field =
- ShreddedVariantFieldArray::from_parts(None,
Some(some_nulls_erased.clone()), None);
+
ShreddedVariantFieldArray::perfectly_shredded(some_nulls_erased.clone());
let some_nulls_type = some_nulls_field.data_type().clone();
let some_nulls_struct: ArrayRef = ArrayRef::from(some_nulls_field);
@@ -4682,8 +4661,7 @@ mod test {
let inner_values: Arc<Int32Array> =
Arc::new(Int32Array::from(vec![Some(111), None, Some(333)]));
let inner_erased: ArrayRef = inner_values.clone();
- let inner_field =
- ShreddedVariantFieldArray::from_parts(None,
Some(inner_erased.clone()), None);
+ let inner_field =
ShreddedVariantFieldArray::perfectly_shredded(inner_erased.clone());
let inner_field_type = inner_field.data_type().clone();
let inner_struct_array: ArrayRef = ArrayRef::from(inner_field);
@@ -4697,7 +4675,7 @@ mod test {
);
let nested_struct_erased: ArrayRef = nested_struct.clone();
let struct_field =
- ShreddedVariantFieldArray::from_parts(None,
Some(nested_struct_erased.clone()), None);
+
ShreddedVariantFieldArray::perfectly_shredded(nested_struct_erased.clone());
let struct_field_type = struct_field.data_type().clone();
let struct_field_struct: ArrayRef = ArrayRef::from(struct_field);
@@ -4717,10 +4695,9 @@ mod test {
EMPTY_VARIANT_METADATA_BYTES,
all_nulls_values.len(),
));
- let variant_array: ArrayRef = VariantArray::from_parts(
+ let variant_array: ArrayRef = VariantArray::perfectly_shredded(
Arc::new(metadata),
- None,
- Some(Arc::new(typed_value_struct)),
+ Arc::new(typed_value_struct),
None,
)
.into();
@@ -5084,8 +5061,7 @@ mod test {
)));
let typed_value: ArrayRef = Arc::new($typed_value);
let variant_array: ArrayRef =
- VariantArray::from_parts(metadata, None,
Some(typed_value), $parent_nulls)
- .into();
+ VariantArray::perfectly_shredded(metadata, typed_value,
$parent_nulls).into();
let result = variant_get(
&variant_array,
diff --git a/parquet-variant-compute/src/variant_to_arrow.rs
b/parquet-variant-compute/src/variant_to_arrow.rs
index 8022ded25a..d7c61b7653 100644
--- a/parquet-variant-compute/src/variant_to_arrow.rs
+++ b/parquet-variant-compute/src/variant_to_arrow.rs
@@ -1152,7 +1152,7 @@ impl<'a> ListElementBuilder<'a> {
Self::Shredded(b) => {
let (value, typed_value, nulls) = b.finish()?;
Ok(ArrayRef::from(ShreddedVariantFieldArray::from_parts(
- Some(Arc::new(value)),
+ Arc::new(value),
Some(typed_value),
nulls,
)))
@@ -1404,7 +1404,7 @@ impl VariantToBinaryVariantArrowRowBuilder {
fn finish(mut self) -> Result<ArrayRef> {
let variant_array = VariantArray::from_parts(
self.metadata,
- Some(Arc::new(self.builder.build()?)),
+ Arc::new(self.builder.build()?),
None, // no typed_value column
self.nulls.finish(),
);
diff --git a/parquet/tests/variant_integration.rs
b/parquet/tests/variant_integration.rs
index f4525185f6..e06426a7a8 100644
--- a/parquet/tests/variant_integration.rs
+++ b/parquet/tests/variant_integration.rs
@@ -55,6 +55,8 @@ macro_rules! variant_test_case {
// Notes
// - case 3 is empty in cases.json for some reason
// - cases 40, 42, 87, 127 and 128 are expected to fail always (they include
invalid variants)
+// - cases 41, 131 and 138 omit the spec-required `value` column. They are
read leniently by
+// synthesizing an all-null `value`
(https://github.com/apache/arrow-rs/issues/10306)
// - the remaining cases are expected to (eventually) pass
variant_test_case!(1);
@@ -104,6 +106,8 @@ variant_test_case!(38);
variant_test_case!(39);
// Is an error case (should be failing as the expected error message indicates)
variant_test_case!(40, "both value and typed_value are non-null");
+// Schema omits the spec-required `value` column. Read leniently by
synthesizing an
+// all-null `value` (testArrayMissingValueColumn,
https://github.com/apache/arrow-rs/issues/10306)
variant_test_case!(41);
// Is an error case (should be failing as the expected error message indicates)
variant_test_case!(42, "both value and typed_value are non-null");
@@ -201,6 +205,8 @@ variant_test_case!(127, "Illegal shredded value type:
UInt32");
variant_test_case!(128, "Expected object in value field");
variant_test_case!(129);
variant_test_case!(130);
+// Schema omits the spec-required `value` column. Read leniently by
synthesizing an
+// all-null `value` (testMissingValueColumn,
https://github.com/apache/arrow-rs/issues/10306)
variant_test_case!(131);
variant_test_case!(132);
variant_test_case!(133);
@@ -209,6 +215,8 @@ variant_test_case!(135);
variant_test_case!(136);
// Is an error case (should be failing as the expected error message indicates)
variant_test_case!(137, "Illegal shredded value type: FixedSizeBinary(4)");
+// Schema omits the spec-required `value` column. Read leniently by
synthesizing an
+// all-null `value` (testShreddedObjectMissingValueColumn,
https://github.com/apache/arrow-rs/issues/10306)
variant_test_case!(138);
/// Test case definition structure matching the format from
