Use >= instead of > for art_node*_prev_child (#548)

Using > caused it to skip the first child. Modified an existing test to cover this case.
RoaringBitmap · Jan 13, 2024 · 19e6003 · 19e6003
1 parent 4230b8a
commit 19e6003
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 15 deletions.
diff --git a/src/art/art.c b/src/art/art.c
@@ -550,7 +550,7 @@ static inline art_indexed_child_t art_node48_prev_child(
     }
     index--;
     art_indexed_child_t indexed_child;
-    for (int i = index; i > 0; --i) {
+    for (int i = index; i >= 0; --i) {
         if (node->keys[i] != ART_NODE48_EMPTY_VAL) {
             indexed_child.child = node->children[node->keys[i]];
             indexed_child.index = i;
@@ -667,7 +667,7 @@ static inline art_indexed_child_t art_node256_prev_child(
     }
     index--;
     art_indexed_child_t indexed_child;
-    for (int i = index; i > 0; --i) {
+    for (int i = index; i >= 0; --i) {
         if (node->children[i] != NULL) {
             indexed_child.child = node->children[i];
             indexed_child.index = i;

diff --git a/tests/art_unit.cpp b/tests/art_unit.cpp
@@ -201,39 +201,53 @@ DEFINE_TEST(test_art_is_empty) {
 }
 
 DEFINE_TEST(test_art_iterator_next) {
-    std::vector<const char*> keys = {
-        "000001", "000002", "000003", "000004", "001005",
-    };
-    std::vector<Value> values = {{1}, {2}, {3}, {4}, {5}};
+    std::vector<std::array<uint8_t, 6>> keys;
+    std::vector<Value> values;
+    std::vector<size_t> sizes = {4, 16, 48, 256};
+    for (size_t i = 0; i < sizes.size(); i++) {
+        uint8_t size = static_cast<uint8_t>(sizes[i]);
+        for (size_t j = 0; j < size; j++) {
+            keys.push_back(
+                {0, 0, 0, static_cast<uint8_t>(i), static_cast<uint8_t>(j)});
+            values.push_back({static_cast<uint64_t>(i) * j});
+        }
+    }
     art_t art{NULL};
     for (size_t i = 0; i < keys.size(); ++i) {
-        art_insert(&art, (art_key_chunk_t*)keys[i], &values[i]);
+        art_insert(&art, (art_key_chunk_t*)keys[i].data(), &values[i]);
     }
 
     art_iterator_t iterator = art_init_iterator(&art, true);
     size_t i = 0;
     do {
-        assert_key_eq(iterator.key, (art_key_chunk_t*)keys[i]);
+        assert_key_eq(iterator.key, (art_key_chunk_t*)keys[i].data());
         assert_true(iterator.value == &values[i]);
         ++i;
     } while (art_iterator_next(&iterator));
     art_free(&art);
 }
 
 DEFINE_TEST(test_art_iterator_prev) {
-    std::vector<const char*> keys = {
-        "000001", "000002", "000003", "000004", "001005",
-    };
-    std::vector<Value> values = {{1}, {2}, {3}, {4}, {5}};
+    std::vector<std::array<uint8_t, 6>> keys;
+    std::vector<Value> values;
+    std::vector<size_t> sizes = {4, 16, 48, 256};
+    for (size_t i = 0; i < sizes.size(); i++) {
+        uint8_t size = static_cast<uint8_t>(sizes[i]);
+        for (size_t j = 0; j < size; j++) {
+            keys.push_back(
+                {0, 0, 0, static_cast<uint8_t>(i), static_cast<uint8_t>(j)});
+            values.push_back({static_cast<uint64_t>(i) * j});
+        }
+    }
     art_t art{NULL};
     for (size_t i = 0; i < keys.size(); ++i) {
-        art_insert(&art, (art_key_chunk_t*)keys[i], &values[i]);
+        art_insert(&art, (art_key_chunk_t*)keys[i].data(), &values[i]);
     }
 
-    art_iterator_t iterator = art_init_iterator(&art, false);
+    art_iterator_t iterator = art_init_iterator(&art, /*first=*/false);
     size_t i = keys.size() - 1;
     do {
-        assert_key_eq(iterator.key, (art_key_chunk_t*)keys[i]);
+        assert_key_eq(iterator.key, (art_key_chunk_t*)keys[i].data());
         --i;
     } while (art_iterator_prev(&iterator));
     art_free(&art);