codephage2020 commented on code in PR #9598:
URL: https://github.com/apache/arrow-rs/pull/9598#discussion_r3025624955
##########
parquet-variant-compute/src/variant_get.rs:
##########
@@ -86,15 +88,14 @@ pub(crate) fn follow_shredded_path_element<'a>(
return Ok(missing_path_step());
};
- let struct_array = field.as_struct_opt().ok_or_else(|| {
- // TODO: Should we blow up? Or just end the traversal and let
the normal
- // variant pathing code sort out the mess that it must anyway
be
- // prepared to handle?
- ArrowError::InvalidArgumentError(format!(
- "Expected Struct array while following path, got {}",
- field.data_type(),
- ))
- })?;
+ // The field might be a VariantArray (StructArray) if shredded,
+ // or it might be a primitive array. Only proceed if it's a
StructArray.
+ let Some(struct_array) = field.as_struct_opt() else {
+ // Field exists but is not a StructArray, so it cannot be
+ // followed further. Fall back to the value column if present,
+ // otherwise the path is missing.
Review Comment:
Great analysis, thank you. In a shredded variant object,
every field must be a struct containing `value` and/or `typed_value`
sub-fields per the spec.
A non-struct field here means malformed data, and the original error was
correct.
I've reverted to returning an InvalidArgumentError:
let struct_array = field.as_struct_opt().ok_or_else(|| {
ArrowError::InvalidArgumentError(format!(
"Expected shredded variant struct for field '{}', got {}",
name,
field.data_type(),
))
})?;
The error message now names the field for easier debugging.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
