HaoYang670 commented on code in PR #1818:
URL: https://github.com/apache/arrow-rs/pull/1818#discussion_r892062160
##########
arrow/src/array/equal/list.rs:
##########
@@ -73,11 +73,19 @@ pub(super) fn list_equal<T: OffsetSizeTrait>(
// however, one is more likely to slice into a list array and get a region
that has 0
// child values.
// The test that triggered this behaviour had [4, 4] as a slice of 1 value
slot.
Review Comment:
nit: We should also update the docs. Because we add an edge case for
zero-length array
##########
arrow/src/array/equal/mod.rs:
##########
@@ -629,6 +629,39 @@ mod tests {
test_equal(&a, &b, false);
}
+ #[test]
Review Comment:
Should we also test that 2 zero-length arrays with different (offset,
null-buffer, childdata) are equal?
##########
arrow/src/array/equal/list.rs:
##########
@@ -73,11 +73,19 @@ pub(super) fn list_equal<T: OffsetSizeTrait>(
// however, one is more likely to slice into a list array and get a region
that has 0
// child values.
// The test that triggered this behaviour had [4, 4] as a slice of 1 value
slot.
- let lhs_child_length = lhs_offsets[lhs_start + len].to_usize().unwrap()
- - lhs_offsets[lhs_start].to_usize().unwrap();
+ let lhs_child_length = if len == 0 {
Review Comment:
What about just add
```rust
// Zero length list arrays are always equal
if len == 0 {
return true;
}
```
at the beginning of the function?
--
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]
