Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make BITPOS in Bitmap handling stop_given #2085

Merged
merged 4 commits into from
Feb 2, 2024

Conversation

mapleFU
Copy link
Member

@mapleFU mapleFU commented Feb 1, 2024

See #2084

}
bitmap_->BitPos(key_, false, 0, 0, /*stop_given=*/true, &pos);
EXPECT_EQ(-1, pos);
bitmap_->BitPos(key_, false, 0, 1, /*stop_given=*/false, &pos);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If test:

bitmap_->BitPos(key_, false, 0, 1, /*stop_given=*/false, &pos);

The result is undefined, sigh...

@@ -285,6 +285,10 @@ rocksdb::Status Bitmap::BitPos(const Slice &user_key, bool bit, int64_t start, i
std::tie(start, stop) = BitmapString::NormalizeRange(start, stop, static_cast<int64_t>(metadata.size));
auto u_start = static_cast<uint32_t>(start);
auto u_stop = static_cast<uint32_t>(stop);
if (u_start > u_stop) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This unifying the responds of BitmapString

}
// There're two cases that `pin_value.size() < kBitmapSegmentBytes`:
// 1. If it's the last segment, we've done searching in the above loop.
// 2. If it's not the last segment, we can check if the segment is all 0.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic here is a bit hacking:

  1. if bit, we don't search the value greater than searching area
  2. if i == stop_index, making the logic outside the loop to decide the response
  3. Otherwise, searching setting the pos

@mapleFU
Copy link
Member Author

mapleFU commented Feb 1, 2024

cc @PragmaTwice @git-hulk

Copy link
Member

@git-hulk git-hulk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link

sonarcloud bot commented Feb 2, 2024

Quality Gate Passed Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

1 New issue
0 Security Hotspots
60.2% Coverage on New Code
1.5% Duplication on New Code

See analysis details on SonarCloud

@mapleFU mapleFU merged commit 4d85538 into apache:unstable Feb 2, 2024
30 checks passed
@mapleFU mapleFU deleted the bitpos-re-fix branch February 2, 2024 10:24
JoverZhang pushed a commit to JoverZhang/kvrocks that referenced this pull request Feb 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants