HaoYang670 commented on code in PR #2345:
URL: https://github.com/apache/arrow-rs/pull/2345#discussion_r939640567


##########
arrow/src/datatypes/field.rs:
##########
@@ -698,41 +684,25 @@ impl Field {
     /// * self.metadata is a superset of other.metadata
     /// * all other fields are equal

Review Comment:
   I am not sure whether this is reasonable. For example,
   ```rust
   let child_field1 = ...;
   let child_field2 = ...;
   let field1 = Struct(&child_field1);
   let mut field2 = Struct(&child_field2);
   field2.try_merge(field1).unwrap();
   assert_eq!(false, field2.contains(field1))
   ```
   As `field2` has merged `field1` into itself, I expect the `field2` to 
contain `field1`.
   However, you will find that the answer if `false` because `field1` contains 
`child_field1`
   but `field2` contains `child_field1` and `child_field2`.



##########
arrow/src/datatypes/field.rs:
##########
@@ -698,41 +684,25 @@ impl Field {
     /// * self.metadata is a superset of other.metadata
     /// * all other fields are equal

Review Comment:
   I am not sure whether this is reasonable. For example,
   ```rust
   let child_field1 = ...;
   let child_field2 = ...;
   let field1 = Struct(&child_field1);
   let mut field2 = Struct(&child_field2);
   field2.try_merge(field1).unwrap();
   assert_eq!(false, field2.contains(field1))
   ```
   As `field2` has merged `field1` into itself, I expect the `field2` to 
contain `field1`.
   However, you will find that the answer is `false` because `field1` contains 
`child_field1`
   but `field2` contains `child_field1` and `child_field2`.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to