(arrow-rs) branch main updated: [Variant] Make sure ObjectBuilder and ListBuilder to be finalized before its parent builder (#7843)

[alamb]

This is an automated email from the ASF dual-hosted git repository.

alamb pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-rs.git


The following commit(s) were added to refs/heads/main by this push:
     new 13d79b3588 [Variant] Make sure ObjectBuilder and ListBuilder to be 
finalized before its parent builder (#7843)
13d79b3588 is described below

commit 13d79b35884bf1fb2b761dc8e70b39bb24ae6c6b
Author: Liang-Chi Hsieh <vii...@gmail.com>
AuthorDate: Fri Jul 4 11:55:39 2025 -0700

    [Variant] Make sure ObjectBuilder and ListBuilder to be finalized before 
its parent builder (#7843)
    
    # Which issue does this PR close?
    
    
    - Closes #7780.
    
    
    # Rationale for this change
    
    This minor patch adds some comments to ObjectBuilder and ListBuilder and
    fixes one existing test.
    
    This patch makes sure ObjectBuilder and ListBuilder to be finalized
    before its parent builder is finalized. If `finish` is forgotten to be
    called on them, the compiler will show error.
    
    
    # What changes are included in this PR?
    
    
    # Are these changes tested?
    
    Updated tests.
    
    # Are there any user-facing changes?
    
    No
    
    ---------
    
    Co-authored-by: Liang-Chi Hsieh <liangchi.hs...@databricks.com>
    Co-authored-by: Andrew Lamb <and...@nerdnetworks.org>
---
 parquet-variant/src/builder.rs | 35 ++++++++++++++++++++++++++++++++---
 1 file changed, 32 insertions(+), 3 deletions(-)

diff --git a/parquet-variant/src/builder.rs b/parquet-variant/src/builder.rs
index fe3090f70b..cb3a373cb8 100644
--- a/parquet-variant/src/builder.rs
+++ b/parquet-variant/src/builder.rs
@@ -450,6 +450,7 @@ impl MetadataBuilder {
 /// obj.insert("a", 1);
 /// obj.insert("a", 2); // duplicate field
 ///
+/// // When validation is enabled, finish will return an error
 /// let result = obj.finish(); // returns Err
 /// assert!(result.is_err());
 /// ```
@@ -495,10 +496,20 @@ impl VariantBuilder {
             .with_validate_unique_fields(self.validate_unique_fields)
     }
 
+    /// Append a non-nested value to the builder.
+    ///
+    /// # Example
+    /// ```
+    /// # use parquet_variant::{Variant, VariantBuilder};
+    /// let mut builder = VariantBuilder::new();
+    /// // most primitive types can be appended directly as they implement 
`Into<Variant>`
+    /// builder.append_value(42i8);
+    /// ```
     pub fn append_value<'m, 'd, T: Into<Variant<'m, 'd>>>(&mut self, value: T) 
{
         self.buffer.append_non_nested_value(value);
     }
 
+    /// Finish the builder and return the metadata and value buffers.
     pub fn finish(self) -> (Vec<u8>, Vec<u8>) {
         (self.metadata_builder.finish(), self.buffer.into_inner())
     }
@@ -577,6 +588,7 @@ impl<'a> ListBuilder<'a> {
         self.offsets.push(element_end);
     }
 
+    /// Finish the list, writing it to the parent buffer and consuming self.
     pub fn finish(mut self) {
         self.check_new_offset();
 
@@ -603,6 +615,14 @@ impl<'a> ListBuilder<'a> {
     }
 }
 
+/// Drop implementation for ListBuilder does nothing
+/// as the `finish` method must be called to finalize the list.
+/// This is to ensure that the list is always finalized before its parent 
builder
+/// is finalized.
+impl Drop for ListBuilder<'_> {
+    fn drop(&mut self) {}
+}
+
 /// A builder for creating [`Variant::Object`] values.
 ///
 /// See the examples on [`VariantBuilder`] for usage.
@@ -694,9 +714,7 @@ impl<'a, 'b> ObjectBuilder<'a, 'b> {
         list_builder
     }
 
-    /// Finalize object
-    ///
-    /// This consumes self and writes the object to the parent buffer.
+    /// Finalize the object, writing it to the parent buffer and consuming 
self.
     pub fn finish(mut self) -> Result<(), ArrowError> {
         self.check_pending_field();
 
@@ -756,6 +774,14 @@ impl<'a, 'b> ObjectBuilder<'a, 'b> {
     }
 }
 
+/// Drop implementation for ObjectBuilder does nothing
+/// as the `finish` method must be called to finalize the object.
+/// This is to ensure that the object is always finalized before its parent 
builder
+/// is finalized.
+impl Drop for ObjectBuilder<'_, '_> {
+    fn drop(&mut self) {}
+}
+
 /// Trait that abstracts functionality from Variant fconstruction 
implementations, namely
 /// `VariantBuilder`, `ListBuilder` and `ObjectFieldBuilder` to minimize code 
duplication.
 pub(crate) trait VariantBuilderExt<'m, 'v> {
@@ -1490,6 +1516,9 @@ mod tests {
             "Invalid argument error: Duplicate field keys detected: [x]"
         );
 
+        inner_list.finish();
+        outer_list.finish();
+
         // Valid object should succeed
         let mut list = builder.new_list();
         let mut valid_obj = list.new_object();