friendlymatthew commented on code in PR #7720:
URL: https://github.com/apache/arrow-rs/pull/7720#discussion_r2159790437


##########
parquet-variant/src/builder.rs:
##########
@@ -540,7 +535,8 @@ impl<'a> ObjectBuilder<'a> {
         }
 
         // Write field offsets
-        for &(_, offset) in &self.fields {
+        for id in self.parent.dict.values() {
+            let &offset = self.fields.get(id).unwrap();

Review Comment:
   Hi, yes. I figured since the current `VariantBuilder` can only build 1 
object as of now, it would be ok to assume the field names of the current 
object maps 1:1 with the field names in the `dict` metadata dictionary. 
   
   I pushed up 22789798797b5b42950569ef6fdb720b1a256a68, which filters by the 
relative field ids within the current object. I thought it would make sense to 
update this logic when thinking about nested lists and objects. 



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