alamb commented on code in PR #7888:
URL: https://github.com/apache/arrow-rs/pull/7888#discussion_r2198111683


##########
parquet-variant/src/variant/list.rs:
##########
@@ -123,11 +123,16 @@ pub struct VariantList<'m, 'v> {
     pub metadata: VariantMetadata<'m>,
     pub value: &'v [u8],
     header: VariantListHeader,
-    num_elements: usize,
-    first_value_byte: usize,
+    num_elements: u32,
+    first_value_byte: u32,
     validated: bool,
 }
 
+#[cfg(test)]
+const _: () = if std::mem::size_of::<VariantList>() != 64 {
+    panic!("VariantList changed size");

Review Comment:
   ```suggestion
       // to ensure fast performance, we want to avoid increasing the size of 
VariantList
       panic!("VariantList changed size");
   ```



##########
parquet-variant/src/variant/list.rs:
##########
@@ -186,10 +191,10 @@ impl<'m, 'v> VariantList<'m, 'v> {
 
         // Use the last offset to upper-bound the value buffer
         let last_offset = new_self
-            .get_offset(num_elements)?
+            .get_offset(num_elements as _)?

Review Comment:
   I agree this is fine



##########
parquet-variant/src/variant/metadata.rs:
##########
@@ -128,15 +128,20 @@ impl VariantMetadataHeader {
 ///
 /// [`Variant`]: crate::Variant
 /// [Variant Spec]: 
https://github.com/apache/parquet-format/blob/master/VariantEncoding.md#metadata-encoding
-#[derive(Debug, Clone, Copy, PartialEq)]
+#[derive(Debug, Clone, PartialEq)]
 pub struct VariantMetadata<'m> {
     bytes: &'m [u8],
     header: VariantMetadataHeader,
-    dictionary_size: usize,
-    first_value_byte: usize,
+    dictionary_size: u32,
+    first_value_byte: u32,
     validated: bool,
 }
 
+#[cfg(test)]
+const _: () = if std::mem::size_of::<VariantMetadata>() != 32 {
+    panic!("VariantMetadata changed size, which will impact VariantList and 
VariantObject");

Review Comment:
   It might help to give more context here
   ```suggestion
       // to ensure fast performance, we want to avoid increasing the size of 
VariantMetadata 
       panic!("VariantMetadata changed size, which will impact VariantList and 
VariantObject.");
   ```



-- 
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