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


##########
parquet-variant/src/builder.rs:
##########
@@ -350,14 +378,59 @@ impl<S: AsRef<str>> FromIterator<S> for MetadataBuilder {
     }
 }
 
-impl<S: AsRef<str>> Extend<S> for MetadataBuilder {
+impl<S: AsRef<str>> Extend<S> for DefaultMetadataBuilder {
     fn extend<T: IntoIterator<Item = S>>(&mut self, iter: T) {
         for field_name in iter {
             self.upsert_field_name(field_name.as_ref());
         }
     }
 }
 
+/// Read-only metadata builder that validates field names against an existing 
metadata dictionary

Review Comment:
   Second observation: How helpful is it, to have the 
`Variant::ShreddedObject::value` member? Could we just extract its fields 
directly into the map instead? Then we just have to sort the map and write them 
out again. We might also want to have the map key off field ids instead of 
`&str`, using lookups into the top-level `VariantMetadata` object. That way, 
the final to-bytes transformation doesn't need a metadata builder at all.



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