alamb commented on code in PR #7914:
URL: https://github.com/apache/arrow-rs/pull/7914#discussion_r2211289684
##########
parquet-variant/src/builder.rs:
##########
@@ -310,6 +376,8 @@ impl MetadataBuilder {
fn upsert_field_name(&mut self, field_name: &str) -> u32 {
let (id, new_entry) =
self.field_names.insert_full(field_name.to_string());
+ dbg!(new_entry);
Review Comment:
I think this is left over
##########
parquet-variant/src/builder.rs:
##########
@@ -239,12 +279,38 @@ impl ValueBuffer {
Variant::Binary(v) => self.append_binary(v),
Variant::String(s) => self.append_string(s),
Variant::ShortString(s) => self.append_short_string(s),
- Variant::Object(_) | Variant::List(_) => {
- unreachable!(
- "Nested values are handled specially by ObjectBuilder and
ListBuilder"
- );
+ Variant::Object(obj) => {
+ let metadata_field_names = metadata_builder
+ .field_names
+ .iter()
+ .enumerate()
+ .map(|(i, f)| (f.clone(), i))
+ .collect::<HashMap<_, _>>();
+
+ let mut object_builder = self.new_object(metadata_builder);
+
+ // first add all object fields that exist in metadata builder
Review Comment:
I see the code here is basically trying to preserve the order so that the
variants compare deeply equal
Rather than trying to preserve the byte-for-byte equality here, I think a
better idea could be to implement a `Variant::eq_value` type method that
compares the Variants for logical equality, rather than byte-for-byte equality.
I am thinking we can merge this PR and then I will make a follow on PR for
that idea
--
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
