scovich commented on code in PR #7807:
URL: https://github.com/apache/arrow-rs/pull/7807#discussion_r2176145986


##########
parquet-variant/src/variant/list.rs:
##########
@@ -119,54 +226,51 @@ impl<'m, 'v> VariantList<'m, 'v> {
         self.len() == 0
     }
 
-    /// Returns element by index in `0..self.len()`, if any
+    /// Returns element by index in `0..self.len()`, if any. May panic if this 
list is [invalid].
+    ///
+    /// [invalid]: Self#Validation
     pub fn get(&self, index: usize) -> Option<Variant<'m, 'v>> {
-        if index >= self.num_elements {
-            return None;
-        }
-
-        match self.try_get(index) {
-            Ok(variant) => Some(variant),
-            Err(err) => panic!("validation error: {err}"),
-        }
+        (index < self.num_elements).then(|| {
+            self.try_get_impl(index)
+                .and_then(Variant::validate)
+                .expect("Invalid variant array element")
+        })
     }
 
     /// Fallible version of `get`. Returns element by index, capturing 
validation errors
-    fn try_get(&self, index: usize) -> Result<Variant<'m, 'v>, ArrowError> {
-        if index >= self.num_elements {
-            return Err(ArrowError::InvalidArgumentError(format!(
-                "Index {} out of bounds for list of length {}",
-                index, self.num_elements,
-            )));
-        }
-
-        // Skip header and num_elements bytes to read the offsets
-        let unpack = |i| {
-            self.header
-                .offset_size
-                .unpack_usize(self.value, self.first_offset_byte, i)
-        };
+    pub fn try_get(&self, index: usize) -> Result<Variant<'m, 'v>, ArrowError> 
{
+        self.try_get_impl(index)?.validate()
+    }
 
-        // Read the value bytes from the offsets
-        let variant_value_bytes = slice_from_slice_at_offset(
-            self.value,
-            self.first_value_byte,
-            unpack(index)?..unpack(index + 1)?,
-        )?;
-        let variant = Variant::try_new_with_metadata(self.metadata, 
variant_value_bytes)?;
-        Ok(variant)
+    /// Fallible iteration over the elements of this list.
+    pub fn iter_try(&self) -> impl Iterator<Item = Result<Variant<'m, 'v>, 
ArrowError>> + '_ {
+        (0..self.len()).map(move |i| self.try_get_impl(i))

Review Comment:
   Fixed.



-- 
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: github-unsubscr...@arrow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to