viirya commented on code in PR #7935: URL: https://github.com/apache/arrow-rs/pull/7935#discussion_r2217931769
########## parquet-variant/src/builder.rs: ########## @@ -1134,39 +1197,86 @@ impl<'a> ObjectBuilder<'a> { ))); } - let data_size = self.buffer.offset(); - let num_fields = self.fields.len(); - let is_large = num_fields > u8::MAX as usize; + let metadata_builder = self.parent_state.metadata_builder(); self.fields.sort_by(|&field_a_id, _, &field_b_id, _| { - let key_a = &metadata_builder.field_name(field_a_id as usize); - let key_b = &metadata_builder.field_name(field_b_id as usize); - key_a.cmp(key_b) + let field_a_name = metadata_builder.field_name(field_a_id as usize); + let field_b_name = metadata_builder.field_name(field_b_id as usize); + field_a_name.cmp(field_b_name) }); - let max_id = self.fields.iter().map(|(i, _)| *i).max().unwrap_or(0); + // the length of the metadata's field names is a very cheap to compute the upper bound. + // it will almost always be a tight upper bound as well -- it would take a pretty + // carefully crafted object to use only the early field ids of a large dictionary. + let max_id = metadata_builder.field_names.len(); Review Comment: Hmm, honestly I didn't get what this comment trying to say. And the `max_id` is same as the length of `field_names` exactly? -- 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