SERVER-38227 Fix biggie unique index cursor with non-inclusive bound

Also add unittest to generic sorted data interface testing.

Author: GeertBosch

src/mongo/db/storage/biggie/biggie_sorted_impl.cpp

@@ -94,12 +94,11 @@ std::string createKeyString(const BSONObj& key,
     b.append("", prefixToUse);                                // prefix
     b.append("", std::string(ks.getBuffer(), ks.getSize()));  // key
 
-    std::unique_ptr<KeyString> retKs;
     if (isUnique)
-        retKs = std::make_unique<KeyString>(version, b.obj(), allAscending);
+        ks.resetToKey(b.obj(), allAscending);
     else
-        retKs = std::make_unique<KeyString>(version, b.obj(), allAscending, loc);
-    return std::string(retKs->getBuffer(), retKs->getSize());
+        ks.resetToKey(b.obj(), allAscending, loc);
+    return std::string(ks.getBuffer(), ks.getSize());
 }
 
 bool keysAreIdentical(std::string ks1, std::string ks2, bool isUnique) {
@@ -688,13 +687,20 @@ boost::optional<IndexKeyEntry> SortedDataInterface::Cursor::seekAfterProcessing(
     } else {
         // Otherwise, we seek to the nearest element to our key, but only to the right.
         if (_forward) {
-            _forwardIt = _workingCopy->lower_bound(workingCopyBound);
+            if (inclusive)
+                _forwardIt = _workingCopy->lower_bound(workingCopyBound);
+            else
+                _forwardIt = _workingCopy->upper_bound(workingCopyBound);
         } else {
             // Reverse iterators work with upper bound since upper bound will return the first
             // element past the argument, so when it becomes a reverse iterator, it goes
             // backwards one, (according to the C++ standard) and we end up in the right place.
-            _reverseIt =
-                StringStore::const_reverse_iterator(_workingCopy->upper_bound(workingCopyBound));
+            if (inclusive)
+                _reverseIt = StringStore::const_reverse_iterator(
+                    _workingCopy->upper_bound(workingCopyBound));
+            else
+                _reverseIt = StringStore::const_reverse_iterator(
+                    _workingCopy->lower_bound(workingCopyBound));
         }
         // Once again, we check to make sure the iterator didn't fall off the data structure and
         // still is in the ident.

src/mongo/db/storage/sorted_data_interface_test_cursor.cpp

@@ -32,6 +32,7 @@
 
 #include "mongo/db/storage/sorted_data_interface_test_harness.h"
 
+#include <algorithm>
 #include <memory>
 
 #include "mongo/db/storage/sorted_data_interface.h"
@@ -168,5 +169,82 @@ TEST(SortedDataInterface, ExhaustCursorReversed) {
     }
 }
 
+void testBoundaries(bool unique, bool forward, bool inclusive) {
+    const auto harnessHelper(newSortedDataInterfaceHarnessHelper());
+    const std::unique_ptr<SortedDataInterface> sorted(
+        harnessHelper->newSortedDataInterface(unique));
+
+    const ServiceContext::UniqueOperationContext opCtx(harnessHelper->newOperationContext());
+    ASSERT(sorted->isEmpty(opCtx.get()));
+
+    int nToInsert = 10;
+    for (int i = 0; i < nToInsert; i++) {
+        WriteUnitOfWork uow(opCtx.get());
+        BSONObj key = BSON("" << i);
+        RecordId loc(42 + i * 2);
+        ASSERT_OK(sorted->insert(opCtx.get(), key, loc, true));
+        uow.commit();
+    }
+
+    {
+        const std::unique_ptr<SortedDataInterface::Cursor> cursor(
+            sorted->newCursor(opCtx.get(), forward));
+        int startVal = 2;
+        int endVal = 6;
+        if (!forward)
+            std::swap(startVal, endVal);
+
+        auto startKey = BSON("" << startVal);
+        auto endKey = BSON("" << endVal);
+        cursor->setEndPosition(endKey, inclusive);
+
+        auto entry = cursor->seek(startKey, inclusive);
+
+        // Check that the cursor returns the expected values in range.
+        int step = forward ? 1 : -1;
+        for (int i = startVal + (inclusive ? 0 : step); i != endVal + (inclusive ? step : 0);
+             i += step) {
+            ASSERT_EQ(entry, IndexKeyEntry(BSON("" << i), RecordId(42 + i * 2)));
+            entry = cursor->next();
+        }
+        ASSERT(!entry);
+
+        // Cursor at EOF should remain at EOF when advanced
+        ASSERT(!cursor->next());
+    }
+}
+
+TEST(SortedDataInterfaceBoundaryTest, UniqueForwardWithNonInclusiveBoundaries) {
+    testBoundaries(/*unique*/ true, /*forward*/ true, /*inclusive*/ false);
+}
+
+TEST(SortedDataInterfaceBoundaryTest, NonUniqueForwardWithNonInclusiveBoundaries) {
+    testBoundaries(/*unique*/ false, /*forward*/ true, /*inclusive*/ false);
+}
+
+TEST(SortedDataInterfaceBoundaryTest, UniqueForwardWithInclusiveBoundaries) {
+    testBoundaries(/*unique*/ true, /*forward*/ true, /*inclusive*/ true);
+}
+
+TEST(SortedDataInterfaceBoundaryTest, NonUniqueForwardWithInclusiveBoundaries) {
+    testBoundaries(/*unique*/ false, /*forward*/ true, /*inclusive*/ true);
+}
+
+TEST(SortedDataInterfaceBoundaryTest, UniqueBackwardWithNonInclusiveBoundaries) {
+    testBoundaries(/*unique*/ true, /*forward*/ false, /*inclusive*/ false);
+}
+
+TEST(SortedDataInterfaceBoundaryTest, NonUniqueBackwardWithNonInclusiveBoundaries) {
+    testBoundaries(/*unique*/ false, /*forward*/ false, /*inclusive*/ false);
+}
+
+TEST(SortedDataInterfaceBoundaryTest, UniqueBackwardWithInclusiveBoundaries) {
+    testBoundaries(/*unique*/ true, /*forward*/ false, /*inclusive*/ true);
+}
+
+TEST(SortedDataInterfaceBoundaryTest, NonUniqueBackwardWithInclusiveBoundaries) {
+    testBoundaries(/*unique*/ false, /*forward*/ false, /*inclusive*/ true);
+}
+
 }  // namespace
 }  // namespace mongo