Skip to content

Commit a695fbd

Browse files
committed
Erro handling
Signed-off-by: Adam Gutglick <adam@spiraldb.com>
1 parent cd0e72b commit a695fbd

File tree

3 files changed

+52
-26
lines changed

3 files changed

+52
-26
lines changed

parquet-variant-compute/src/shred_variant.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -932,7 +932,7 @@ mod tests {
932932
);
933933
}
934934
None => {
935-
assert!(fallbacks.0.is_null(idx));
935+
assert!(fallback_value.is_null(idx));
936936
}
937937
}
938938
}

parquet-variant-compute/src/unshred_variant.rs

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,11 @@ pub fn unshred_variant(array: &VariantArray) -> Result<VariantArray> {
7676
if array.is_null(i) {
7777
value_builder.append_null();
7878
} else {
79-
let metadata_bytes = binary_array_value(metadata.as_ref(), i)
80-
.expect("metadata field must be a binary-like array");
79+
let metadata_bytes = binary_array_value(metadata.as_ref(), i).ok_or_else(|| {
80+
ArrowError::InvalidArgumentError(
81+
"metadata field must be a binary-like array".to_string(),
82+
)
83+
})?;
8184
let metadata = VariantMetadata::new(metadata_bytes);
8285
let mut value_builder = value_builder.builder_ext(&metadata);
8386
row_builder.append_row(&mut value_builder, &metadata, i)?;
@@ -330,8 +333,11 @@ impl<'a> ValueOnlyUnshredVariantBuilder<'a> {
330333
if self.value.is_null(index) {
331334
builder.append_null();
332335
} else {
333-
let value_bytes = binary_array_value(self.value.as_ref(), index)
334-
.expect("value field must be a binary-like array");
336+
let value_bytes = binary_array_value(self.value.as_ref(), index).ok_or_else(|| {
337+
ArrowError::InvalidArgumentError(
338+
"value field must be a binary-like array".to_string(),
339+
)
340+
})?;
335341
let variant = Variant::new_with_metadata(metadata.clone(), value_bytes);
336342
builder.append_value(variant);
337343
}
@@ -354,11 +360,17 @@ trait AppendToVariantBuilder: Array {
354360
macro_rules! handle_unshredded_case {
355361
($self:expr, $builder:expr, $metadata:expr, $index:expr, $partial_shredding:expr) => {{
356362
let value = $self.value.as_ref().filter(|v| v.is_valid($index));
357-
let value = value.map(|v| {
358-
let bytes = binary_array_value(v.as_ref(), $index)
359-
.expect("value field must be a binary-like array");
360-
Variant::new_with_metadata($metadata.clone(), bytes)
361-
});
363+
let value = value
364+
.map(|v| {
365+
let bytes = binary_array_value(v.as_ref(), $index).ok_or_else(|| {
366+
ArrowError::InvalidArgumentError(format!(
367+
"value field must be a binary-like array, instead got {}",
368+
v.data_type(),
369+
))
370+
})?;
371+
Result::Ok(Variant::new_with_metadata($metadata.clone(), bytes))
372+
})
373+
.transpose()?;
362374

363375
// If typed_value is null, handle unshredded case and return early
364376
if $self.typed_value.is_null($index) {

parquet-variant-compute/src/variant_array.rs

Lines changed: 30 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -51,17 +51,12 @@ pub(crate) fn binary_array_value(array: &dyn Array, index: usize) -> Option<&[u8
5151
}
5252
}
5353

54-
/// Returns `true` if the data type is `Binary`, `LargeBinary`, or `BinaryView`.
55-
fn is_binary_like(dt: &DataType) -> bool {
56-
matches!(
57-
dt,
58-
DataType::Binary | DataType::LargeBinary | DataType::BinaryView
59-
)
60-
}
61-
6254
/// Validates that an array has a binary-like data type.
6355
fn validate_binary_array(array: &dyn Array, field_name: &str) -> Result<()> {
64-
if is_binary_like(array.data_type()) {
56+
if matches!(
57+
array.data_type(),
58+
DataType::Binary | DataType::LargeBinary | DataType::BinaryView
59+
) {
6560
Ok(())
6661
} else {
6762
Err(ArrowError::InvalidArgumentError(format!(
@@ -397,10 +392,19 @@ impl VariantArray {
397392
}
398393
// Otherwise fall back to value, if available
399394
(_, Some(value)) if value.is_valid(index) => {
400-
let metadata = binary_array_value(self.metadata.as_ref(), index)
401-
.expect("metadata field must be a binary-like array");
402-
let value = binary_array_value(value.as_ref(), index)
403-
.expect("value field must be a binary-like array");
395+
let metadata =
396+
binary_array_value(self.metadata.as_ref(), index).ok_or_else(|| {
397+
ArrowError::InvalidArgumentError(format!(
398+
"metadata field must be a binary-like array, instead got {}",
399+
self.metadata.data_type(),
400+
))
401+
})?;
402+
let value = binary_array_value(value.as_ref(), index).ok_or_else(|| {
403+
ArrowError::InvalidArgumentError(format!(
404+
"value field must be a binary-like array, instead got {}",
405+
value.data_type(),
406+
))
407+
})?;
404408
Ok(Variant::new(metadata, value))
405409
}
406410
// It is technically invalid for neither value nor typed_value fields to be available,
@@ -983,9 +987,19 @@ fn typed_value_to_variant<'a>(
983987
let value = array.value(index);
984988
Ok(Uuid::from_slice(value).unwrap().into()) // unwrap is safe: slice is always 16 bytes
985989
}
986-
DataType::Binary | DataType::LargeBinary | DataType::BinaryView => {
987-
let value = binary_array_value(typed_value.as_ref(), index)
988-
.expect("match arm guarantees the array is binary-like");
990+
DataType::Binary => {
991+
let array = typed_value.as_binary::<i32>();
992+
let value = array.value(index);
993+
Ok(Variant::from(value))
994+
}
995+
DataType::LargeBinary => {
996+
let array = typed_value.as_binary::<i64>();
997+
let value = array.value(index);
998+
Ok(Variant::from(value))
999+
}
1000+
DataType::BinaryView => {
1001+
let array = typed_value.as_binary_view();
1002+
let value = array.value(index);
9891003
Ok(Variant::from(value))
9901004
}
9911005
DataType::Utf8 => {

0 commit comments

Comments
 (0)