fix: Bug GEQ range query bug (#4557)

The bug was caused by incorrect handling of corner cases,
when a path that should lead to an item was wrongfully cleared, which lead
to empty results for SortedMap::GetRange query.

This PR:
1. fixes the wrong code in bptree_set.h.
2. Adds unit tests for both bptree_set_test and sorted_map_test.

Related to https://github.com/mastodon/mastodon/issues/33805

Signed-off-by: Roman Gershman <roman@dragonflydb.io>
This commit is contained in:
Roman Gershman 2025-02-04 17:06:08 +02:00 committed by GitHub
parent 6c68519f02
commit dc3a91e083
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 53 additions and 4 deletions

View file

@ -489,8 +489,14 @@ template <typename T, typename Policy>
detail::BPTreePath<T> BPTree<T, Policy>::GEQ(KeyT item) const {
BPTreePath path;
if (!Locate(item, &path) && path.Last().second >= path.Last().first->NumItems())
path.Clear();
bool res = Locate(item, &path);
// if we did not find the item and the path does not lead to any key in the node,
// adjust the path to point to the next key in the tree.
// In case we are past all items in the tree, Next() will collapse to the empty path.
if (!res && path.Last().second >= path.Last().first->NumItems()) {
path.Next();
}
return path;
}

View file

@ -88,12 +88,16 @@ class BPTreeSetTest : public ::testing::Test {
static void SetUpTestSuite() {
}
void FillTree(unsigned factor = 1) {
for (unsigned i = 0; i < kNumElems; ++i) {
void FillTree(unsigned start, unsigned factor) {
for (unsigned i = start; i < kNumElems; ++i) {
bptree_.Insert(i * factor);
}
}
void FillTree(unsigned factor = 1) {
FillTree(0, factor);
}
bool Validate();
MiMemoryResource mi_alloc_;
@ -277,6 +281,25 @@ TEST_F(BPTreeSetTest, Ranges) {
EXPECT_TRUE(path.Empty());
}
TEST_F(BPTreeSetTest, HalfRanges) {
FillTree(1, 3); // 3, 6, 9 ...
auto path = bptree_.FromRank(bptree_.Size() - 1);
uint64_t val = path.Terminal();
for (unsigned i = 0; i <= val; ++i) {
path = bptree_.GEQ(i);
ASSERT_FALSE(path.Empty()) << i;
}
path = bptree_.GEQ(val + 1);
ASSERT_TRUE(path.Empty());
for (unsigned i = 3; i <= val + 10; ++i) {
path = bptree_.LEQ(i);
ASSERT_FALSE(path.Empty()) << i;
}
path = bptree_.LEQ(2);
ASSERT_TRUE(path.Empty());
}
TEST_F(BPTreeSetTest, MemoryUsage) {
zskiplist* zsl = zslCreate();
std::vector<sds> sds_vec;

View file

@ -246,6 +246,26 @@ TEST_F(SortedMapTest, DeleteRange) {
EXPECT_EQ(96, sm_.DeleteRangeByLex(lex_range));
}
TEST_F(SortedMapTest, RangeBug) {
constexpr size_t kArrLen = 80;
for (unsigned i = 0; i < kArrLen; i++) {
sds s = sdsempty();
s = sdscatfmt(s, "score%u", i);
sm_.Insert(i, s);
}
for (unsigned i = 0; i < kArrLen; i++) {
zrangespec range;
range.max = HUGE_VAL;
range.min = i;
range.minex = 0;
range.maxex = 0;
auto arr = sm_.GetRange(range, 0, 5, false);
ASSERT_GT(arr.size(), 0) << i;
}
}
// not a real test, just to see how much memory is used by zskiplist.
TEST_F(SortedMapTest, MemoryUsage) {
zskiplist* zsl = zslCreate();