From 41faf876c06e6e1c07abc29b466a9c2125472fa4 Mon Sep 17 00:00:00 2001 From: Kishore Nallan Date: Fri, 10 Nov 2023 17:31:28 +0530 Subject: [PATCH] Fixed an edge case in facet count update. --- include/facet_index.h | 2 +- src/facet_index.cpp | 17 +- test/facet_index_test.cpp | 459 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 474 insertions(+), 4 deletions(-) diff --git a/include/facet_index.h b/include/facet_index.h index fe7cc322..453ee626 100644 --- a/include/facet_index.h +++ b/include/facet_index.h @@ -160,7 +160,7 @@ public: static void update_count_nodes(std::list& count_list, std::map::iterator>& count_map, uint32_t old_count, uint32_t new_count, - std::list::iterator& curr) ; + std::list::iterator& curr); bool facet_value_exists(const std::string& field_name, const std::string& fvalue); }; \ No newline at end of file diff --git a/src/facet_index.cpp b/src/facet_index.cpp index 67447948..c8c924fe 100644 --- a/src/facet_index.cpp +++ b/src/facet_index.cpp @@ -130,13 +130,14 @@ void facet_index_t::update_count_nodes(std::list& count_list, // 5, 4, [4 -> 7] // 5, 4, [4 -> 7], 3 // 5, [4 -> 7] + // [4 -> 5] // delete count map entry if `curr` is the anchor for `old_count` if(std::next(curr) == count_list.end() || std::next(curr)->count < old_count) { count_map.erase(old_count); // find a replacement for orig_count - if(std::prev(curr)->count == old_count) { + if(curr != count_list.begin() && std::prev(curr)->count == old_count) { count_map.emplace(old_count, std::prev(curr)); } } @@ -148,9 +149,13 @@ void facet_index_t::update_count_nodes(std::list& count_list, // entry for new_count already exists in count map // a) 10, 7, [5 -> 7], 3 // 10, 7, 5, [5 -> 7] + // 10, 7, 5, [5 -> 8] // 10, 7, [5 -> 7] // 10, 7, [5 -> 7], 5 + // [9 -> 8], 8 + // 10, [9 -> 8], 8 + auto existing_node = count_map_it->second; // delete count map entry if `curr` is the anchor for `old_count` @@ -158,7 +163,7 @@ void facet_index_t::update_count_nodes(std::list& count_list, count_map.erase(old_count); // find a replacement for orig_count - if(std::prev(curr)->count == old_count) { + if(curr != count_list.begin() && std::prev(curr)->count == old_count) { count_map.emplace(old_count, std::prev(curr)); } } @@ -172,6 +177,12 @@ void facet_index_t::update_count_nodes(std::list& count_list, // 10, [7 -> 9], 7 // 10, 7, [5 -> 9], 3 + // [10 -> 9] + // [5 -> 4], 2 + // [5 -> 1], 2 + // 5, 5, [5 -> 4], 5 + // 5, 5, 5, [5 -> 4] + auto gt_node = count_map_it->second; // delete old entry if `orig_count` iterator is same as `curr` @@ -179,7 +190,7 @@ void facet_index_t::update_count_nodes(std::list& count_list, count_map.erase(old_count); // find a replacement for orig_count - if(std::prev(curr)->count == old_count) { + if(curr != count_list.begin() && std::prev(curr)->count == old_count) { count_map.emplace(old_count, std::prev(curr)); } } diff --git a/test/facet_index_test.cpp b/test/facet_index_test.cpp index 7859e392..2e897b28 100644 --- a/test/facet_index_test.cpp +++ b/test/facet_index_test.cpp @@ -98,4 +98,463 @@ TEST(FacetIndexTest, UpdateWhenAllCountsLessThanNewCount) { ASSERT_EQ(2, count_map.size()); ASSERT_EQ(count_list.begin(), count_map[7]); ASSERT_EQ(std::next(count_list.begin(), 1), count_map[5]); + + // [4 -> 5] + + count_list.clear(); + count_map.clear(); + + count_list.push_back(facet_index_t::facet_count_t("", 4, 0)); + count_map.emplace(4, count_list.begin()); + + curr = count_list.begin(); + curr->count = 5; + facet_index_t::update_count_nodes(count_list, count_map, 4, 5, curr); + + ASSERT_EQ(1, count_list.size()); + ASSERT_EQ(5, count_list.begin()->count); + + ASSERT_EQ(1, count_map.size()); + ASSERT_EQ(count_list.begin(), count_map[5]); +} + +TEST(FacetIndexTest, UpdateWhenCountAlreadyExists) { + // 10, 7, [5 -> 7], 3 + std::list count_list; + count_list.push_back(facet_index_t::facet_count_t("", 10, 0)); + count_list.push_back(facet_index_t::facet_count_t("", 7, 1)); + count_list.push_back(facet_index_t::facet_count_t("", 5, 2)); + count_list.push_back(facet_index_t::facet_count_t("", 3, 3)); + + std::map::iterator> count_map; + count_map.emplace(10, count_list.begin()); + count_map.emplace(7, std::next(count_list.begin(), 1)); + count_map.emplace(5, std::next(count_list.begin(), 2)); + count_map.emplace(3, std::next(count_list.begin(), 3)); + + auto curr = std::next(count_list.begin(), 2); + curr->count = 7; + facet_index_t::update_count_nodes(count_list, count_map, 5, 7, curr); + + // New order: 10, 7, 7, 3 + ASSERT_EQ(10, count_list.begin()->count); + ASSERT_EQ(7, std::next(count_list.begin(), 1)->count); + ASSERT_EQ(7, std::next(count_list.begin(), 2)->count); + ASSERT_EQ(3, std::next(count_list.begin(), 3)->count); + + ASSERT_EQ(3, count_map.size()); + ASSERT_EQ(count_list.begin(), count_map[10]); + ASSERT_EQ(std::next(count_list.begin(), 2), count_map[7]); + ASSERT_EQ(std::next(count_list.begin(), 3), count_map[3]); + + // 10, 7, 5, [5 -> 7] + + count_list.clear(); + count_map.clear(); + + count_list.push_back(facet_index_t::facet_count_t("", 10, 0)); + count_list.push_back(facet_index_t::facet_count_t("", 7, 1)); + count_list.push_back(facet_index_t::facet_count_t("", 5, 2)); + count_list.push_back(facet_index_t::facet_count_t("", 5, 3)); + + count_map.emplace(10, count_list.begin()); + count_map.emplace(7, std::next(count_list.begin(), 1)); + count_map.emplace(5, std::next(count_list.begin(), 3)); + + curr = std::next(count_list.begin(), 3); + curr->count = 7; + facet_index_t::update_count_nodes(count_list, count_map, 5, 7, curr); + + // New order: 10, [7], 7, 5 + ASSERT_EQ(10, count_list.begin()->count); + ASSERT_EQ(7, std::next(count_list.begin(), 1)->count); + ASSERT_EQ(7, std::next(count_list.begin(), 2)->count); + ASSERT_EQ(5, std::next(count_list.begin(), 3)->count); + + ASSERT_EQ(3, count_map.size()); + ASSERT_EQ(count_list.begin(), count_map[10]); + ASSERT_EQ(std::next(count_list.begin(), 2), count_map[7]); + ASSERT_EQ(std::next(count_list.begin(), 3), count_map[5]); + + // 10, 7, 5, [5 -> 8] + + count_list.clear(); + count_map.clear(); + + count_list.push_back(facet_index_t::facet_count_t("", 10, 0)); + count_list.push_back(facet_index_t::facet_count_t("", 7, 1)); + count_list.push_back(facet_index_t::facet_count_t("", 5, 2)); + count_list.push_back(facet_index_t::facet_count_t("", 5, 3)); + + count_map.emplace(10, count_list.begin()); + count_map.emplace(7, std::next(count_list.begin(), 1)); + count_map.emplace(5, std::next(count_list.begin(), 3)); + + curr = std::next(count_list.begin(), 3); + curr->count = 8; + facet_index_t::update_count_nodes(count_list, count_map, 5, 8, curr); + + // New order: 10, [8], 7, 5 + ASSERT_EQ(10, count_list.begin()->count); + ASSERT_EQ(8, std::next(count_list.begin(), 1)->count); + ASSERT_EQ(7, std::next(count_list.begin(), 2)->count); + ASSERT_EQ(5, std::next(count_list.begin(), 3)->count); + + ASSERT_EQ(4, count_map.size()); + ASSERT_EQ(count_list.begin(), count_map[10]); + ASSERT_EQ(std::next(count_list.begin(), 1), count_map[8]); + ASSERT_EQ(std::next(count_list.begin(), 2), count_map[7]); + ASSERT_EQ(std::next(count_list.begin(), 3), count_map[5]); + + // 10, 7, [5 -> 7] + + count_list.clear(); + count_map.clear(); + + count_list.push_back(facet_index_t::facet_count_t("", 10, 0)); + count_list.push_back(facet_index_t::facet_count_t("", 7, 1)); + count_list.push_back(facet_index_t::facet_count_t("", 5, 2)); + + count_map.emplace(10, count_list.begin()); + count_map.emplace(7, std::next(count_list.begin(), 1)); + count_map.emplace(5, std::next(count_list.begin(), 2)); + + curr = std::next(count_list.begin(), 2); + curr->count = 7; + facet_index_t::update_count_nodes(count_list, count_map, 5, 7, curr); + + // New order: 10, [7], 7 + ASSERT_EQ(10, count_list.begin()->count); + ASSERT_EQ(7, std::next(count_list.begin(), 1)->count); + ASSERT_EQ(7, std::next(count_list.begin(), 2)->count); + + ASSERT_EQ(2, count_map.size()); + ASSERT_EQ(count_list.begin(), count_map[10]); + ASSERT_EQ(std::next(count_list.begin(), 2), count_map[7]); + + // 10, 7, [5 -> 7], 5 + + count_list.clear(); + count_map.clear(); + + count_list.push_back(facet_index_t::facet_count_t("", 10, 0)); + count_list.push_back(facet_index_t::facet_count_t("", 7, 1)); + count_list.push_back(facet_index_t::facet_count_t("", 5, 2)); + count_list.push_back(facet_index_t::facet_count_t("", 5, 3)); + + count_map.emplace(10, count_list.begin()); + count_map.emplace(7, std::next(count_list.begin(), 1)); + count_map.emplace(5, std::next(count_list.begin(), 3)); + + curr = std::next(count_list.begin(), 2); + curr->count = 7; + facet_index_t::update_count_nodes(count_list, count_map, 5, 7, curr); + + // New order: 10, [7], 7, 5 + ASSERT_EQ(10, count_list.begin()->count); + ASSERT_EQ(7, std::next(count_list.begin(), 1)->count); + ASSERT_EQ(7, std::next(count_list.begin(), 2)->count); + ASSERT_EQ(5, std::next(count_list.begin(), 3)->count); + + ASSERT_EQ(3, count_map.size()); + ASSERT_EQ(count_list.begin(), count_map[10]); + ASSERT_EQ(std::next(count_list.begin(), 2), count_map[7]); + ASSERT_EQ(std::next(count_list.begin(), 3), count_map[5]); +} + +TEST(FacetIndexTest, UpdateWhenGreaterNodeExists) { + // 10, 7, [7 -> 9], 3 + std::list count_list; + count_list.push_back(facet_index_t::facet_count_t("", 10, 0)); + count_list.push_back(facet_index_t::facet_count_t("", 7, 1)); + count_list.push_back(facet_index_t::facet_count_t("", 7, 2)); + count_list.push_back(facet_index_t::facet_count_t("", 3, 3)); + + std::map::iterator> count_map; + count_map.emplace(10, count_list.begin()); + count_map.emplace(7, std::next(count_list.begin(), 2)); + count_map.emplace(3, std::next(count_list.begin(), 3)); + + auto curr = std::next(count_list.begin(), 2); + curr->count = 9; + facet_index_t::update_count_nodes(count_list, count_map, 7, 9, curr); + + // New order: 10, [9], 7, 3 + ASSERT_EQ(10, count_list.begin()->count); + ASSERT_EQ(9, std::next(count_list.begin(), 1)->count); + ASSERT_EQ(7, std::next(count_list.begin(), 2)->count); + ASSERT_EQ(3, std::next(count_list.begin(), 3)->count); + + ASSERT_EQ(4, count_map.size()); + ASSERT_EQ(count_list.begin(), count_map[10]); + ASSERT_EQ(std::next(count_list.begin(), 1), count_map[9]); + ASSERT_EQ(std::next(count_list.begin(), 2), count_map[7]); + ASSERT_EQ(std::next(count_list.begin(), 3), count_map[3]); + + // 10, 7, [7 -> 9] + + count_list.clear(); + count_map.clear(); + + count_list.push_back(facet_index_t::facet_count_t("", 10, 0)); + count_list.push_back(facet_index_t::facet_count_t("", 7, 1)); + count_list.push_back(facet_index_t::facet_count_t("", 7, 2)); + + count_map.emplace(10, count_list.begin()); + count_map.emplace(7, std::next(count_list.begin(), 2)); + + curr = std::next(count_list.begin(), 2); + curr->count = 9; + facet_index_t::update_count_nodes(count_list, count_map, 7, 9, curr); + + // New order: 10, [9], 7 + ASSERT_EQ(10, count_list.begin()->count); + ASSERT_EQ(9, std::next(count_list.begin(), 1)->count); + ASSERT_EQ(7, std::next(count_list.begin(), 2)->count); + + ASSERT_EQ(3, count_map.size()); + ASSERT_EQ(count_list.begin(), count_map[10]); + ASSERT_EQ(std::next(count_list.begin(), 1), count_map[9]); + ASSERT_EQ(std::next(count_list.begin(), 2), count_map[7]); + + // 10, [7 -> 9] + + count_list.clear(); + count_map.clear(); + + count_list.push_back(facet_index_t::facet_count_t("", 10, 0)); + count_list.push_back(facet_index_t::facet_count_t("", 7, 1)); + + count_map.emplace(10, count_list.begin()); + count_map.emplace(7, std::next(count_list.begin(), 1)); + + curr = std::next(count_list.begin(), 1); + curr->count = 9; + facet_index_t::update_count_nodes(count_list, count_map, 7, 9, curr); + + // New order: 10, [9] + ASSERT_EQ(10, count_list.begin()->count); + ASSERT_EQ(9, std::next(count_list.begin(), 1)->count); + + ASSERT_EQ(2, count_map.size()); + ASSERT_EQ(count_list.begin(), count_map[10]); + ASSERT_EQ(std::next(count_list.begin(), 1), count_map[9]); + + // 10, [7 -> 9], 7 + + count_list.clear(); + count_map.clear(); + + count_list.push_back(facet_index_t::facet_count_t("", 10, 0)); + count_list.push_back(facet_index_t::facet_count_t("", 7, 1)); + count_list.push_back(facet_index_t::facet_count_t("", 7, 2)); + + count_map.emplace(10, count_list.begin()); + count_map.emplace(7, std::next(count_list.begin(), 2)); + + curr = std::next(count_list.begin(), 1); + curr->count = 9; + facet_index_t::update_count_nodes(count_list, count_map, 7, 9, curr); + + // New order: 10, [9], 7 + ASSERT_EQ(10, count_list.begin()->count); + ASSERT_EQ(9, std::next(count_list.begin(), 1)->count); + ASSERT_EQ(7, std::next(count_list.begin(), 2)->count); + + ASSERT_EQ(3, count_map.size()); + ASSERT_EQ(count_list.begin(), count_map[10]); + ASSERT_EQ(std::next(count_list.begin(), 1), count_map[9]); + ASSERT_EQ(std::next(count_list.begin(), 2), count_map[7]); + + // 10, 7, [5 -> 9], 3 + + count_list.clear(); + count_map.clear(); + + count_list.push_back(facet_index_t::facet_count_t("", 10, 0)); + count_list.push_back(facet_index_t::facet_count_t("", 7, 1)); + count_list.push_back(facet_index_t::facet_count_t("", 5, 2)); + count_list.push_back(facet_index_t::facet_count_t("", 3, 3)); + + count_map.emplace(10, count_list.begin()); + count_map.emplace(7, std::next(count_list.begin(), 1)); + count_map.emplace(5, std::next(count_list.begin(), 2)); + count_map.emplace(3, std::next(count_list.begin(), 3)); + + curr = std::next(count_list.begin(), 2); + curr->count = 9; + facet_index_t::update_count_nodes(count_list, count_map, 5, 9, curr); + + // New order: 10, [9], 7, 3 + ASSERT_EQ(10, count_list.begin()->count); + ASSERT_EQ(9, std::next(count_list.begin(), 1)->count); + ASSERT_EQ(7, std::next(count_list.begin(), 2)->count); + ASSERT_EQ(3, std::next(count_list.begin(), 3)->count); + + ASSERT_EQ(4, count_map.size()); + ASSERT_EQ(count_list.begin(), count_map[10]); + ASSERT_EQ(std::next(count_list.begin(), 1), count_map[9]); + ASSERT_EQ(std::next(count_list.begin(), 2), count_map[7]); + ASSERT_EQ(std::next(count_list.begin(), 3), count_map[3]); +} + +TEST(FacetIndexTest, DecrementSingleCount) { + // [10 -> 9] + std::list count_list; + count_list.push_back(facet_index_t::facet_count_t("", 10, 0)); + + std::map::iterator> count_map; + count_map.emplace(10, count_list.begin()); + + auto curr = count_list.begin(); + curr->count = 9; + facet_index_t::update_count_nodes(count_list, count_map, 10, 9, curr); + + ASSERT_EQ(1, count_list.size()); + ASSERT_EQ(9, count_list.begin()->count); + + ASSERT_EQ(1, count_map.size()); + ASSERT_EQ(count_list.begin(), count_map[9]); + + // [9 -> 8], 8 + count_list.clear(); + count_map.clear(); + + count_list.push_back(facet_index_t::facet_count_t("", 9, 0)); + count_list.push_back(facet_index_t::facet_count_t("", 8, 1)); + + count_map.emplace(9, count_list.begin()); + count_map.emplace(8, std::next(count_list.begin(), 1)); + + curr = count_list.begin(); + curr->count = 8; + facet_index_t::update_count_nodes(count_list, count_map, 9, 8, curr); + + ASSERT_EQ(2, count_list.size()); + ASSERT_EQ(8, count_list.begin()->count); + ASSERT_EQ(8, std::next(count_list.begin(), 1)->count); + + ASSERT_EQ(1, count_map.size()); + ASSERT_EQ(std::next(count_list.begin(), 1), count_map[8]); + + // 10, [9 -> 8], 8 + count_list.clear(); + count_map.clear(); + + count_list.push_back(facet_index_t::facet_count_t("", 10, 0)); + count_list.push_back(facet_index_t::facet_count_t("", 9, 1)); + count_list.push_back(facet_index_t::facet_count_t("", 8, 2)); + + count_map.emplace(10, count_list.begin()); + count_map.emplace(9, std::next(count_list.begin(), 1)); + count_map.emplace(8, std::next(count_list.begin(), 2)); + + curr = std::next(count_list.begin(), 1); + curr->count = 8; + facet_index_t::update_count_nodes(count_list, count_map, 9, 8, curr); + + ASSERT_EQ(3, count_list.size()); + ASSERT_EQ(10, count_list.begin()->count); + ASSERT_EQ(8, std::next(count_list.begin(), 1)->count); + ASSERT_EQ(8, std::next(count_list.begin(), 2)->count); + + ASSERT_EQ(2, count_map.size()); + ASSERT_EQ(count_list.begin(), count_map[10]); + ASSERT_EQ(std::next(count_list.begin(), 2), count_map[8]); + + // [5 -> 4], 2 + count_list.clear(); + count_map.clear(); + + count_list.push_back(facet_index_t::facet_count_t("", 5, 0)); + count_list.push_back(facet_index_t::facet_count_t("", 2, 1)); + + count_map.emplace(5, count_list.begin()); + count_map.emplace(2, std::next(count_list.begin(), 1)); + + curr = count_list.begin(); + curr->count = 4; + facet_index_t::update_count_nodes(count_list, count_map, 5, 4, curr); + + ASSERT_EQ(2, count_list.size()); + ASSERT_EQ(4, count_list.begin()->count); + ASSERT_EQ(2, std::next(count_list.begin(), 1)->count); + + ASSERT_EQ(2, count_map.size()); + ASSERT_EQ(count_list.begin(), count_map[4]); + ASSERT_EQ(std::next(count_list.begin(), 1), count_map[2]); + + // [5 -> 1], 2 + count_list.clear(); + count_map.clear(); + + count_list.push_back(facet_index_t::facet_count_t("", 5, 0)); + count_list.push_back(facet_index_t::facet_count_t("", 2, 1)); + + count_map.emplace(5, count_list.begin()); + count_map.emplace(2, std::next(count_list.begin(), 1)); + + curr = count_list.begin(); + curr->count = 1; + facet_index_t::update_count_nodes(count_list, count_map, 5, 1, curr); + + ASSERT_EQ(2, count_list.size()); + ASSERT_EQ(2, count_list.begin()->count); + ASSERT_EQ(1, std::next(count_list.begin(), 1)->count); + + ASSERT_EQ(2, count_map.size()); + ASSERT_EQ(count_list.begin(), count_map[2]); + ASSERT_EQ(std::next(count_list.begin(), 1), count_map[1]); + + // 5, 5, [5 -> 4], 5 + // new order: 5, 5, 5, 4 + count_list.clear(); + count_map.clear(); + + count_list.push_back(facet_index_t::facet_count_t("", 5, 0)); + count_list.push_back(facet_index_t::facet_count_t("", 5, 1)); + count_list.push_back(facet_index_t::facet_count_t("", 5, 2)); + count_list.push_back(facet_index_t::facet_count_t("", 5, 3)); + + count_map.emplace(5, std::next(count_list.begin(), 3)); + + curr = std::next(count_list.begin(), 2); + curr->count = 4; + facet_index_t::update_count_nodes(count_list, count_map, 5, 4, curr); + + ASSERT_EQ(4, count_list.size()); + ASSERT_EQ(5, count_list.begin()->count); + ASSERT_EQ(5, std::next(count_list.begin(), 1)->count); + ASSERT_EQ(5, std::next(count_list.begin(), 2)->count); + ASSERT_EQ(4, std::next(count_list.begin(), 3)->count); + + ASSERT_EQ(2, count_map.size()); + ASSERT_EQ(std::next(count_list.begin(), 2), count_map[5]); + ASSERT_EQ(std::next(count_list.begin(), 3), count_map[4]); + + // 5, 5, 5, [5 -> 4] + // new order: 5, 5, 5, 4 + count_list.clear(); + count_map.clear(); + + count_list.push_back(facet_index_t::facet_count_t("", 5, 0)); + count_list.push_back(facet_index_t::facet_count_t("", 5, 1)); + count_list.push_back(facet_index_t::facet_count_t("", 5, 2)); + count_list.push_back(facet_index_t::facet_count_t("", 5, 3)); + + count_map.emplace(5, std::next(count_list.begin(), 3)); + + curr = std::next(count_list.begin(), 3); + curr->count = 4; + facet_index_t::update_count_nodes(count_list, count_map, 5, 4, curr); + + ASSERT_EQ(4, count_list.size()); + ASSERT_EQ(5, count_list.begin()->count); + ASSERT_EQ(5, std::next(count_list.begin(), 1)->count); + ASSERT_EQ(5, std::next(count_list.begin(), 2)->count); + ASSERT_EQ(4, std::next(count_list.begin(), 3)->count); + + ASSERT_EQ(2, count_map.size()); + ASSERT_EQ(std::next(count_list.begin(), 2), count_map[5]); + ASSERT_EQ(std::next(count_list.begin(), 3), count_map[4]); }