From bb5d06e2769635370b8e07f1c1046df4d7ba4421 Mon Sep 17 00:00:00 2001 From: florijan Date: Thu, 3 Aug 2017 14:41:51 +0200 Subject: [PATCH] Skiplist::position_and_count fix Summary: Fixed bug for SkipList::position_and_count for an item lesser then all skiplist elements. Reviewers: mislav.bradac Reviewed By: mislav.bradac Differential Revision: https://phabricator.memgraph.io/D629 --- src/data_structures/concurrent/skiplist.hpp | 6 +++++- tests/unit/skiplist_position_and_count.cpp | 21 +++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/data_structures/concurrent/skiplist.hpp b/src/data_structures/concurrent/skiplist.hpp index b12ff92ea..9a0cb1c4d 100644 --- a/src/data_structures/concurrent/skiplist.hpp +++ b/src/data_structures/concurrent/skiplist.hpp @@ -621,7 +621,7 @@ class SkipList : private Lockable { // now we need to estimate the count of elements equal to item // we'll do that by looking for the first element that is greater // than item, and counting how far we have to look - + // first find the rightmost (highest) succ that has value == item int count_level = 0; for (int i = position_level; i >= 0; i--) @@ -632,6 +632,10 @@ class SkipList : private Lockable { count_level = std::min(count_level, count_max_level); succ = succs[count_level]; + // it is possible that succ just became null (even though before + // it wasn't). that happens when item is lesser then all list elems + if (!succ) return std::make_pair(0, 0); + // now expand to the right as long as element value == item // at the same time accumulate count int count = 1 << count_level; diff --git a/tests/unit/skiplist_position_and_count.cpp b/tests/unit/skiplist_position_and_count.cpp index 806294ea6..11d2d7140 100644 --- a/tests/unit/skiplist_position_and_count.cpp +++ b/tests/unit/skiplist_position_and_count.cpp @@ -87,3 +87,24 @@ TEST(SkiplistPosAndCount, DefaultSpeedAndAccuracy) { // estimations are always absolutely accurate EXPECT_POS_COUNT(5000, 5000, 0, 0, 0); } + +#define EXPECT_FOR_OUT_OF_RANGE(skiplist_size, value) \ + { \ + auto sl = SkiplistRange(skiplist_size); \ + auto position_and_count = sl->access().position_and_count(value); \ + EXPECT_EQ(position_and_count.first, value < 0 ? 0 : skiplist_size); \ + EXPECT_EQ(position_and_count.second, 0); \ + } + +TEST(SkiplistPosAndCount, EdgeValues) { + // small list + EXPECT_FOR_OUT_OF_RANGE(100, -20); + EXPECT_FOR_OUT_OF_RANGE(100, -1); + EXPECT_FOR_OUT_OF_RANGE(100, 100); + EXPECT_FOR_OUT_OF_RANGE(100, 120); + // big list + EXPECT_FOR_OUT_OF_RANGE(100000, -20); + EXPECT_FOR_OUT_OF_RANGE(100000, -1); + EXPECT_FOR_OUT_OF_RANGE(100000, 100000); + EXPECT_FOR_OUT_OF_RANGE(100000, 100300); +}