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


##########
parquet-variant/src/builder.rs:
##########
@@ -1499,4 +1564,116 @@ mod tests {
         let valid_result = valid_obj.finish();
         assert!(valid_result.is_ok());
     }
+
+    #[test]
+    fn test_variant_builder_to_list_builder_no_finish() {
+        // Create a list builder but never finish it
+        let mut builder = VariantBuilder::new();
+        let _list_builder = builder.new_list();
+
+        builder.append_value(42i8);
+
+        // The original builder should be unchanged
+        let (metadata, value) = builder.finish();
+        let variant = Variant::try_new(&metadata, &value).unwrap();
+        assert_eq!(variant, Variant::Int8(42));
+    }
+
+    #[test]
+    fn test_variant_builder_to_object_builder_no_finish() {

Review Comment:
   If a builder didn't `finish`, the next append to its parent would finalize 
the orphaned `pending` stuff. For a list, that meant an extra offset (as if 
there were a zero-sized value there, without even a header). For an object, it 
would associate two different fields with the same value bytes.
   
   That's a good question, why the tests pass without these fixes tho?



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