emilk commented on code in PR #10673:
URL: https://github.com/apache/arrow-rs/pull/10673#discussion_r3774901276
##########
arrow-ord/src/comparison.rs:
##########
@@ -60,7 +60,7 @@ where
for j in 0..list.len() {
if list.is_valid(j) && (left.value(i) == list.value(j)) {
bit_util::set_bit(bool_slice, i);
- continue;
+ break;
Review Comment:
let's double-check this. Is this a proper bug-fix, or a regression?
##########
arrow-ord/src/comparison.rs:
##########
@@ -60,7 +60,7 @@ where
for j in 0..list.len() {
if list.is_valid(j) && (left.value(i) == list.value(j)) {
bit_util::set_bit(bool_slice, i);
- continue;
+ break;
Review Comment:
Neither — it is behaviour-preserving, and a small win.
The inner loop body does exactly one thing: `set_bit(bool_slice, i)`. Once a
match is found for row `i`, every later `j` can only set the same bit again, so
the result is identical either way; `continue` just kept scanning the rest of
the list for nothing. `break` stops at the first match.
So the original was not a bug, it was wasted work. If you would rather keep
this PR strictly mechanical, I am happy to make it `{}` (the literal removal
clippy suggests) and leave `break` for a separate PR.
🤖 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]
