scovich commented on code in PR #7943:
URL: https://github.com/apache/arrow-rs/pull/7943#discussion_r2216453780
##########
parquet-variant/src/variant/object.rs:
##########
@@ -387,6 +389,38 @@ impl<'m, 'v> VariantObject<'m, 'v> {
}
}
+// Custom implementation of PartialEq for variant objects
+//
+// According to the spec, field values are not required to be in the same
order as the field IDs,
+// to enable flexibility when constructing Variant values
+//
+// Instead of comparing the raw bytes of 2 variant objects, this
implementation recursively
+// checks whether the field values are equal -- regardless of their order
+impl<'m, 'v> PartialEq for VariantObject<'m, 'v> {
+ fn eq(&self, other: &Self) -> bool {
+ let mut is_equal = self.metadata == other.metadata
+ && self.header == other.header
+ && self.num_elements == other.num_elements
+ && self.first_field_offset_byte == other.first_field_offset_byte
+ && self.first_value_byte == other.first_value_byte
+ && self.validated == other.validated;
+
+ // value validation
+ let other_fields: HashMap<&str, Variant> =
HashMap::from_iter(other.iter());
+
+ for (field_name, variant) in self.iter() {
+ match other_fields.get(field_name as &str) {
+ Some(other_variant) => {
+ is_equal = is_equal && variant == *other_variant;
+ }
+ None => return false,
+ }
Review Comment:
post-hoc nit: we should short circuit
```rust
if other_fields.get(field_name as &str).is_none_or(|other|
variant != *other) {
return false;
}
```
... but the check is actually incomplete because it fails to prove the
symmetric difference is empty.
--
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
