Skip to content

Commit 18e0214

Browse files
author
Bryan Thompson
committed
fix: scan_from backtrack across sibling subtrees (#850)
Remove incorrect pop() before backtrack loop in seek(). When gte_key_byte() or lte_key_byte() returns nothing, the current node was never pushed onto the stack, so the stack top is already the first ancestor to check for a sibling. The spurious pop() discarded that ancestor, causing seek to return end instead of finding the next subtree. Fixes both FWD and REV paths in art.hpp and olc_art.hpp. Adds regression test: scanFromBacktrackAcrossSiblingSubtrees.
1 parent 9f22403 commit 18e0214

3 files changed

Lines changed: 58 additions & 8 deletions

File tree

art.hpp

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2339,7 +2339,10 @@ typename db<Key, Value>::iterator& db<Key, Value>::iterator::seek(
23392339
// do a left-most descent under that right-sibling. If there
23402340
// is no such parent, we will wind up with an empty stack
23412341
// (iterator at the end) and return that state.
2342-
if (!empty()) pop();
2342+
//
2343+
// Note: The current node (where gte_key_byte failed) was
2344+
// never pushed, so the stack top is already the first
2345+
// ancestor to check for a right-sibling.
23432346
while (!empty()) {
23442347
const auto& centry = top();
23452348
const auto cnode{centry.node}; // possible parent from the stack
@@ -2368,11 +2371,14 @@ typename db<Key, Value>::iterator& db<Key, Value>::iterator::seek(
23682371
// immediate predecessor of the desired key in the data.
23692372
auto nxt = inode->lte_key_byte(node_type, remaining_key[0]);
23702373
if (!nxt) {
2371-
// Pop off the current entry until we find one with a
2372-
// left-sibling and then do a right-most descent under that
2373-
// left-sibling. In the extreme case there is no such
2374-
// previous entry and we will wind up with an empty stack.
2375-
if (!empty()) pop();
2374+
// Pop off entries until we find one with a left-sibling and
2375+
// then do a right-most descent under that left-sibling. In
2376+
// the extreme case there is no such previous entry and we
2377+
// will wind up with an empty stack.
2378+
//
2379+
// Note: The current node (where lte_key_byte failed) was
2380+
// never pushed, so the stack top is already the first
2381+
// ancestor to check for a left-sibling.
23762382
while (!empty()) {
23772383
const auto& centry = top();
23782384
const auto cnode{centry.node}; // possible parent from stack

olc_art.hpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3423,7 +3423,9 @@ bool olc_db<Key, Value>::iterator::try_seek(art_key_type search_key,
34233423
if (UNODB_DETAIL_UNLIKELY(
34243424
!node_critical_section.try_read_unlock())) // unlock node
34253425
return false; // LCOV_EXCL_LINE
3426-
if (!empty()) pop();
3426+
// Note: The current node (where gte_key_byte failed) was
3427+
// never pushed, so the stack top is already the first
3428+
// ancestor to check for a right-sibling.
34273429
while (!empty()) {
34283430
const auto& centry = top();
34293431
const auto cnode{centry.node}; // a possible parent from the stack.
@@ -3500,7 +3502,9 @@ bool olc_db<Key, Value>::iterator::try_seek(art_key_type search_key,
35003502
if (UNODB_DETAIL_UNLIKELY(
35013503
!node_critical_section.try_read_unlock())) // unlock node
35023504
return false; // LCOV_EXCL_LINE
3503-
if (!empty()) pop();
3505+
// Note: The current node (where lte_key_byte failed) was
3506+
// never pushed, so the stack top is already the first
3507+
// ancestor to check for a left-sibling.
35043508
while (!empty()) {
35053509
const auto& centry = top();
35063510
const auto cnode{centry.node}; // a possible parent from stack

test/test_art_scan.cpp

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1083,4 +1083,44 @@ UNODB_TYPED_TEST(ARTScanTest, scanRangeC143) {
10831083
do_scan_range_test<TypeParam>(823, 247, 999);
10841084
}
10851085

1086+
// Regression test for T-5bec51c4: scan_from fails to backtrack across
1087+
// sibling subtrees when the seek key exceeds all children in a subtree.
1088+
// Keys {100,200,300,400,500} encoded as 8-byte BE branch at byte 6:
1089+
// child 0x00 → {100,200}, child 0x01 → {300,400,500}.
1090+
// seek(250) enters child 0x00, finds no child >= 0xFA at byte 7,
1091+
// must backtrack to byte 6 and descend child 0x01 to find 300.
1092+
UNODB_TYPED_TEST(ARTScanTest, scanFromBacktrackAcrossSiblingSubtrees) {
1093+
unodb::test::tree_verifier<TypeParam> verifier;
1094+
TypeParam& db = verifier.get_db();
1095+
for (const std::uint64_t k : {100U, 200U, 300U, 400U, 500U})
1096+
verifier.insert(k, unodb::test::test_values[0]);
1097+
// FWD: scan_from(250) should find 300, 400, 500.
1098+
{
1099+
std::vector<std::uint64_t> results;
1100+
db.scan_from(
1101+
250, [&results](const unodb::visitor<typename TypeParam::iterator>& v) {
1102+
results.push_back(decode(v.get_key()));
1103+
return false;
1104+
});
1105+
UNODB_ASSERT_EQ(3U, results.size());
1106+
UNODB_EXPECT_EQ(300U, results[0]);
1107+
UNODB_EXPECT_EQ(400U, results[1]);
1108+
UNODB_EXPECT_EQ(500U, results[2]);
1109+
}
1110+
// REV: scan_from(250, fn, false) should find 200, 100.
1111+
{
1112+
std::vector<std::uint64_t> results;
1113+
db.scan_from(
1114+
250,
1115+
[&results](const unodb::visitor<typename TypeParam::iterator>& v) {
1116+
results.push_back(decode(v.get_key()));
1117+
return false;
1118+
},
1119+
false /*fwd*/);
1120+
UNODB_ASSERT_EQ(2U, results.size());
1121+
UNODB_EXPECT_EQ(200U, results[0]);
1122+
UNODB_EXPECT_EQ(100U, results[1]);
1123+
}
1124+
}
1125+
10861126
} // namespace

0 commit comments

Comments
 (0)