Skip to content

Commit 58fbb17

Browse files
remove panics in unshred variant (#9741)
- Closes #9740
1 parent c5fed03 commit 58fbb17

File tree

1 file changed

+25
-4
lines changed

1 file changed

+25
-4
lines changed

parquet-variant-compute/src/unshred_variant.rs

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ pub fn unshred_variant(array: &VariantArray) -> Result<VariantArray> {
8181
"metadata field must be a binary-like array".to_string(),
8282
)
8383
})?;
84-
let metadata = VariantMetadata::new(metadata_bytes);
84+
let metadata = VariantMetadata::try_new(metadata_bytes)?;
8585
let mut value_builder = value_builder.builder_ext(&metadata);
8686
row_builder.append_row(&mut value_builder, &metadata, i)?;
8787
}
@@ -338,7 +338,7 @@ impl<'a> ValueOnlyUnshredVariantBuilder<'a> {
338338
"value field must be a binary-like array".to_string(),
339339
)
340340
})?;
341-
let variant = Variant::new_with_metadata(metadata.clone(), value_bytes);
341+
let variant = Variant::try_new_with_metadata(metadata.clone(), value_bytes)?;
342342
builder.append_value(variant);
343343
}
344344
Ok(())
@@ -368,7 +368,7 @@ macro_rules! handle_unshredded_case {
368368
v.data_type(),
369369
))
370370
})?;
371-
Result::Ok(Variant::new_with_metadata($metadata.clone(), bytes))
371+
Variant::try_new_with_metadata($metadata.clone(), bytes)
372372
})
373373
.transpose()?;
374374

@@ -628,7 +628,8 @@ impl<'a> StructUnshredVariantBuilder<'a> {
628628
));
629629
};
630630

631-
for (field_name, field_value) in object.iter() {
631+
for entry in object.iter_try() {
632+
let (field_name, field_value) = entry?;
632633
if self.field_unshredders.contains_key(field_name) {
633634
return Err(ArrowError::InvalidArgumentError(format!(
634635
"Field '{field_name}' appears in both typed_value and value",
@@ -795,4 +796,24 @@ mod tests {
795796
assert_eq!(result.value(1), Variant::from(&b"\xff\xaa"[..]));
796797
assert_eq!(result.value(2), Variant::from(&b"\xde\xad\xbe\xef"[..]));
797798
}
799+
800+
#[test]
801+
fn test_unshred_returns_err_on_malformed_metadata() {
802+
// empty metadata bytes fail VariantMetadata's header parse. before this fix the
803+
// call inside unshred_variant used the panicking `VariantMetadata::new`, which
804+
// crashed the thread instead of surfacing the spec violation through the
805+
// documented `Result` return type.
806+
let metadata: ArrayRef = Arc::new(BinaryViewArray::from_iter_values(vec![&b""[..]]));
807+
808+
let typed_value: ArrayRef = Arc::new(StringViewArray::from(vec![Some("hello")]));
809+
810+
let variant_array = VariantArray::from_parts(metadata, None, Some(typed_value), None);
811+
812+
let result = crate::unshred_variant(&variant_array);
813+
814+
assert!(
815+
result.is_err(),
816+
"unshred_variant must return Err on malformed metadata, not panic",
817+
);
818+
}
798819
}

0 commit comments

Comments
 (0)