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


##########
parquet-variant/src/builder.rs:
##########
@@ -626,50 +712,35 @@ impl Drop for ListBuilder<'_> {
 /// A builder for creating [`Variant::Object`] values.
 ///
 /// See the examples on [`VariantBuilder`] for usage.
-pub struct ObjectBuilder<'a, 'b> {
-    parent_buffer: &'a mut ValueBuffer,
-    metadata_builder: &'a mut MetadataBuilder,
+pub struct ObjectBuilder<'a> {
+    parent_state: ParentState<'a>,
     fields: IndexMap<u32, usize>, // (field_id, offset)
     buffer: ValueBuffer,
-    /// Is there a pending list or object that needs to be finalized?
-    pending: Option<(&'b str, usize)>,
     validate_unique_fields: bool,
     /// Set of duplicate fields to report for errors
     duplicate_fields: HashSet<u32>,
 }
 
-impl<'a, 'b> ObjectBuilder<'a, 'b> {
-    fn new(parent_buffer: &'a mut ValueBuffer, metadata_builder: &'a mut 
MetadataBuilder) -> Self {
+impl<'a> ObjectBuilder<'a> {
+    fn new(parent_state: ParentState<'a>, validate_unique_fields: bool) -> 
Self {
         Self {
-            parent_buffer,
-            metadata_builder,
+            parent_state,
             fields: IndexMap::new(),
             buffer: ValueBuffer::default(),
-            pending: None,
-            validate_unique_fields: false,
+            validate_unique_fields,
             duplicate_fields: HashSet::new(),
         }
     }
 
-    fn check_pending_field(&mut self) {
-        let Some(&(field_name, field_start)) = self.pending.as_ref() else {
-            return;
-        };
-
-        let field_id = self.metadata_builder.upsert_field_name(field_name);
-        self.fields.insert(field_id, field_start);
-
-        self.pending = None;
-    }
-
     /// Add a field with key and value to the object
     ///
     /// Note: when inserting duplicate keys, the new value overwrites the 
previous mapping,
     /// but the old value remains in the buffer, resulting in a larger variant
     pub fn insert<'m, 'd, T: Into<Variant<'m, 'd>>>(&mut self, key: &str, 
value: T) {
-        self.check_pending_field();
+        // Get metadata_builder from parent state
+        let metadata_builder = self.parent_state.metadata_builder();

Review Comment:
   Hmm, so modification to metadata builder from an un-finalized builder won't 
affect the parent builder in the end?



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