scovich commented on code in PR #7871:
URL: https://github.com/apache/arrow-rs/pull/7871#discussion_r2187685092
##########
parquet-variant/src/variant/list.rs:
##########
@@ -232,19 +232,20 @@ impl<'m, 'v> VariantList<'m, 'v> {
/// [invalid]: Self#Validation
pub fn get(&self, index: usize) -> Option<Variant<'m, 'v>> {
(index < self.num_elements).then(|| {
- self.try_get_impl(index)
- .and_then(Variant::validate)
+ self.try_get_with_shallow_validation(index)
+ .and_then(Variant::with_full_validation)
Review Comment:
Do we actually want full validation here? I would expect `get` to perform
shallow validation only, and panic if that failed -- matching the behavior of
`iter` below.
##########
parquet-variant/src/variant/object.rs:
##########
@@ -281,26 +285,31 @@ impl<'m, 'v> VariantObject<'m, 'v> {
/// Returns an iterator of (name, value) pairs over the fields of this
object.
pub fn iter(&self) -> impl Iterator<Item = (&'m str, Variant<'m, 'v>)> +
'_ {
- self.iter_try_impl()
+ self.iter_try_with_shallow_validation()
.map(|result| result.expect("Invalid variant object field value"))
}
/// Fallible iteration over the fields of this object.
pub fn iter_try(
&self,
) -> impl Iterator<Item = Result<(&'m str, Variant<'m, 'v>), ArrowError>>
+ '_ {
- self.iter_try_impl().map(|result| {
+ self.iter_try_with_shallow_validation().map(|result| {
let (name, value) = result?;
- Ok((name, value.validate()?))
+ Ok((name, value.with_full_validation()?))
})
}
// Fallible iteration over the fields of this object that performs only
shallow (constant-cost)
// validation of field values.
- fn iter_try_impl(
+ fn iter_try_with_shallow_validation(
&self,
) -> impl Iterator<Item = Result<(&'m str, Variant<'m, 'v>), ArrowError>>
+ '_ {
- (0..self.num_elements).map(move |i| Ok((self.try_field_name(i)?,
self.try_field(i)?)))
+ (0..self.num_elements).map(move |i| {
+ Ok((
+ self.try_field_name(i)?,
+ self.try_field_with_shallow_validation(i)?,
+ ))
Review Comment:
```suggestion
let field = self.try_field_with_shallow_validation(i)?;
Ok((self.try_field_name(i)?, field))
```
--
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]
