Skip to content

Commit 613896a

Browse files
Copilotbowling233
andcommitted
Fix node_16::grow() missing 128 offset in node_48 index population
node_16::grow() populates node_48::indexes_[] without the required 128 offset, causing child lookups to fail after growth. All other node_48 operations use indexes_[128 + partial_key] for consistent byte-to-index mapping. Changes: - include/art/node_16.hpp: Add 128 offset when populating node_48::indexes_[] during growth - test/node_16.cpp: Add test case that grows node_16 to node_48 and verifies child accessibility and iterator navigation - .gitignore: Exclude _codeql_detected_source_root Impact: Without this fix: - Children inserted before growth become unreachable - Iterator navigation returns incorrect keys - Data effectively corrupted after any node_16→node_48 transition Related to PR #20. Co-authored-by: bowling233 <zhubaolin228@gmail.com>
1 parent 49494b0 commit 613896a

File tree

3 files changed

+83
-1
lines changed

3 files changed

+83
-1
lines changed

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,3 +10,4 @@ cmake-build-debug
1010
.idea
1111
*.log
1212
_codeql_build_dir/
13+
_codeql_detected_source_root

include/art/node_16.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ template <class T> inner_node<T> *node_16<T>::grow() {
115115
new_node->n_children_ = this->n_children_;
116116
std::copy(this->children_, this->children_ + this->n_children_, new_node->children_);
117117
for (int i = 0; i < n_children_; ++i) {
118-
new_node->indexes_[(uint8_t) this->keys_[i]] = i;
118+
new_node->indexes_[128 + (uint8_t) this->keys_[i]] = i;
119119
}
120120
delete this;
121121
return new_node;

test/node_16.cpp

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,4 +236,85 @@ TEST_SUITE("node 16") {
236236
REQUIRE_THROWS_AS(n.prev_partial_key(0), std::out_of_range);
237237
}
238238
}
239+
240+
TEST_CASE("grow to node_48 preserves offset (PR #20)") {
241+
// This test reproduces the bug reported in PR #20:
242+
// When node_16 grows to node_48, the indexes_ array must use 128 offset
243+
244+
leaf_node<void*>* dummy_children[17];
245+
246+
// Create dummy children
247+
for (int i = 0; i < 17; ++i) {
248+
dummy_children[i] = new leaf_node<void*>(nullptr);
249+
}
250+
251+
// Create a node_16 and fill it with 16 children
252+
node_16<void*>* n16 = new node_16<void*>();
253+
254+
// Use ASCII printable characters (which have values > 0)
255+
// This reproduces the scenario described in PR #20 with 'a', 'b', etc.
256+
char test_keys[17];
257+
for (int i = 0; i < 17; ++i) {
258+
test_keys[i] = 'a' + i; // a, b, c, ..., q
259+
}
260+
261+
// Fill node_16 to capacity
262+
for (int i = 0; i < 16; ++i) {
263+
n16->set_child(test_keys[i], dummy_children[i]);
264+
}
265+
266+
REQUIRE(n16->is_full());
267+
268+
// Grow to node_48 by adding one more child
269+
auto* n48 = static_cast<node_48<void*>*>(n16->grow());
270+
REQUIRE(n48 != nullptr);
271+
272+
// CRITICAL TEST: After growing, all original children must still be findable
273+
// This fails without the fix because node_16::grow() doesn't add the 128 offset
274+
for (int i = 0; i < 16; ++i) {
275+
auto** child_ptr = n48->find_child(test_keys[i]);
276+
REQUIRE(child_ptr != nullptr);
277+
REQUIRE(*child_ptr == dummy_children[i]);
278+
}
279+
280+
// Test that we can still add the 17th child
281+
n48->set_child(test_keys[16], dummy_children[16]);
282+
283+
// Verify all 17 children are now accessible
284+
for (int i = 0; i < 17; ++i) {
285+
auto** child_ptr = n48->find_child(test_keys[i]);
286+
REQUIRE(child_ptr != nullptr);
287+
REQUIRE(*child_ptr == dummy_children[i]);
288+
}
289+
290+
// Test iteration (next_partial_key and prev_partial_key)
291+
// This also fails without the fix
292+
SUBCASE("next_partial_key after grow") {
293+
// Starting from first key 'a', we should find all keys in order
294+
char current = n48->next_partial_key('a');
295+
REQUIRE_EQ('a', current);
296+
297+
for (int i = 1; i < 17; ++i) {
298+
current = n48->next_partial_key(current + 1);
299+
REQUIRE_EQ(test_keys[i], current);
300+
}
301+
}
302+
303+
SUBCASE("prev_partial_key after grow") {
304+
// Starting from last key 'q', we should find all keys in reverse order
305+
char current = n48->prev_partial_key('q');
306+
REQUIRE_EQ('q', current);
307+
308+
for (int i = 15; i >= 0; --i) {
309+
current = n48->prev_partial_key(current - 1);
310+
REQUIRE_EQ(test_keys[i], current);
311+
}
312+
}
313+
314+
// Clean up
315+
delete n48;
316+
for (int i = 0; i < 17; ++i) {
317+
delete dummy_children[i];
318+
}
319+
}
239320
}

0 commit comments

Comments
 (0)