emilk commented on code in PR #10673:
URL: https://github.com/apache/arrow-rs/pull/10673#discussion_r3774992326
##########
arrow-data/src/equal/list_view.rs:
##########
@@ -44,27 +44,28 @@ pub(super) fn list_view_equal<T: ArrowNativeType + Integer>(
return false;
}
- if lhs_null_count == 0 {
Review Comment:
Double-checked, and it needed one more change.
What the commit does: both branches computed the same four sub-slices and
ran the same two length checks, so I hoisted them above the `if`.
What I missed: both length checks are tautological.
`&lhs_sizes[lhs_start..lhs_start + len]` is `len` long by construction (it
panics otherwise), and so are the other three, so `lhs_range_sizes.len() !=
rhs_range_sizes.len()` compares `len` with `len`. They are now gone:
```rust
// All four slices are `len` long
let lhs_range_sizes = &lhs_sizes[lhs_start..lhs_start + len];
let rhs_range_sizes = &rhs_sizes[rhs_start..rhs_start + len];
let lhs_range_offsets = &lhs_offsets[lhs_start..lhs_start + len];
let rhs_range_offsets = &rhs_offsets[rhs_start..rhs_start + len];
```
I also moved the "Sizes can differ if values are null" note into the null
branch, which is what it explains: that branch compares sizes per element
instead of comparing the size slices wholesale. Hoisting it to the top made it
read as a claim about the slices themselves.
The rest is unchanged: the `lhs_range_sizes != rhs_range_sizes` content
check stays in the null-free branch only, and both loops are untouched.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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]
