SERVER-34846 Forwardize IndexBounds before intersectizing their OILs

11 files changed, 367 insertions, 52 deletions

diff --git a/jstests/core/collation_with_reverse_index.js b/jstests/core/collation_with_reverse_index.js
new file mode 100644
index 00000000000..af246187348
--- /dev/null
+++ b/jstests/core/collation_with_reverse_index.js
@@ -0,0 +1,12 @@
+// Regression test for SERVER-34846.
+(function() {
+    const coll = db.collation_with_reverse_index;
+    coll.drop();
+
+    coll.insertOne({int: 1, text: "hello world"});
+    coll.createIndex({int: -1, text: -1}, {collation: {locale: "en", strength: 1}});
+    const res = coll.find({int: 1}, {_id: 0, int: 1, text: 1}).toArray();
+
+    assert.eq(res.length, 1);
+    assert.eq(res[0].text, "hello world");
+})();
diff --git a/src/mongo/db/query/index_bounds.cpp b/src/mongo/db/query/index_bounds.cpp
index ef8eeb6d8b8..b8b7e4bce74 100644
--- a/src/mongo/db/query/index_bounds.cpp
+++ b/src/mongo/db/query/index_bounds.cpp
@@ -32,6 +32,7 @@
 #include <tuple>
 #include <utility>
 
+#include "mongo/base/simple_string_data_comparator.h"
 #include "mongo/bson/simple_bsonobj_comparator.h"
 
 namespace mongo {
@@ -163,6 +164,21 @@ BoundInclusion IndexBounds::makeBoundInclusionFromBoundBools(bool startKeyInclus
     }
 }
 
+BoundInclusion IndexBounds::reverseBoundInclusion(BoundInclusion b) {
+    switch (b) {
+        case BoundInclusion::kIncludeStartKeyOnly:
+            return BoundInclusion::kIncludeEndKeyOnly;
+        case BoundInclusion::kIncludeEndKeyOnly:
+            return BoundInclusion::kIncludeStartKeyOnly;
+        case BoundInclusion::kIncludeBothStartAndEndKeys:
+        case BoundInclusion::kExcludeBothStartAndEndKeys:
+            // These are both symmetric.
+            return b;
+        default:
+            MONGO_UNREACHABLE;
+    }
+}
+
 
 bool OrderedIntervalList::operator==(const OrderedIntervalList& other) const {
     if (this->name != other.name) {
@@ -186,6 +202,38 @@ bool OrderedIntervalList::operator!=(const OrderedIntervalList& other) const {
     return !(*this == other);
 }
 
+void OrderedIntervalList::reverse() {
+    for (size_t i = 0; i < (intervals.size() + 1) / 2; i++) {
+        const size_t otherIdx = intervals.size() - i - 1;
+        intervals[i].reverse();
+        if (i != otherIdx) {
+            intervals[otherIdx].reverse();
+            std::swap(intervals[i], intervals[otherIdx]);
+        }
+    }
+}
+
+OrderedIntervalList OrderedIntervalList::reverseClone() const {
+    OrderedIntervalList clone(name);
+
+    for (auto it = intervals.rbegin(); it != intervals.rend(); ++it) {
+        clone.intervals.push_back(it->reverseClone());
+    }
+
+    return clone;
+}
+
+Interval::Direction OrderedIntervalList::computeDirection() const {
+    for (auto&& iv : intervals) {
+        const auto dir = iv.getDirection();
+        if (dir != Interval::Direction::kDirectionNone) {
+            return dir;
+        }
+    }
+
+    return Interval::Direction::kDirectionNone;
+}
+
 // static
 void OrderedIntervalList::complement() {
     BSONObjBuilder minBob;
@@ -299,6 +347,40 @@ BSONObj IndexBounds::toBSON() const {
     return bob.obj();
 }
 
+IndexBounds IndexBounds::forwardize() const {
+    IndexBounds newBounds;
+    newBounds.isSimpleRange = isSimpleRange;
+
+    if (isSimpleRange) {
+        const int cmpRes = startKey.woCompare(endKey);
+        if (cmpRes <= 0) {
+            newBounds.startKey = startKey;
+            newBounds.endKey = endKey;
+            newBounds.boundInclusion = boundInclusion;
+        } else {
+            // Swap start and end key.
+            newBounds.endKey = startKey;
+            newBounds.startKey = endKey;
+            newBounds.boundInclusion = IndexBounds::reverseBoundInclusion(boundInclusion);
+        }
+
+        return newBounds;
+    }
+
+    newBounds.fields.reserve(fields.size());
+    std::transform(fields.begin(),
+                   fields.end(),
+                   std::back_inserter(newBounds.fields),
+                   [](const OrderedIntervalList& oil) -> OrderedIntervalList {
+                       if (oil.computeDirection() == Interval::Direction::kDirectionDescending) {
+                           return oil.reverseClone();
+                       }
+                       return oil;
+                   });
+
+    return newBounds;
+}
+
 //
 // Validity checking for bounds
 //
diff --git a/src/mongo/db/query/index_bounds.h b/src/mongo/db/query/index_bounds.h
index 2dfcc122b26..4b04f374b16 100644
--- a/src/mongo/db/query/index_bounds.h
+++ b/src/mongo/db/query/index_bounds.h
@@ -74,6 +74,15 @@ struct OrderedIntervalList {
 
     bool operator==(const OrderedIntervalList& other) const;
     bool operator!=(const OrderedIntervalList& other) const;
+
+    void reverse();
+
+    /**
+     * Return a clone of this OIL, that is reversed.
+     */
+    OrderedIntervalList reverseClone() const;
+
+    Interval::Direction computeDirection() const;
 };
 
 /**
@@ -126,6 +135,11 @@ struct IndexBounds {
     static BoundInclusion makeBoundInclusionFromBoundBools(bool startKeyInclusive,
                                                            bool endKeyInclusive);
 
+    /**
+     * Reverse the BoundInclusion.
+     */
+    static BoundInclusion reverseBoundInclusion(BoundInclusion b);
+
 
     /**
      * BSON format for explain. The format is an array of strings for each field.
@@ -137,6 +151,12 @@ struct IndexBounds {
      */
     BSONObj toBSON() const;
 
+    /**
+     * Return a copy of the index bounds, but with each of the OILs going in the ascending
+     * direction.
+     */
+    IndexBounds forwardize() const;
+
     // TODO: we use this for max/min scan.  Consider migrating that.
     bool isSimpleRange;
     BSONObj startKey;
diff --git a/src/mongo/db/query/index_bounds_builder.cpp b/src/mongo/db/query/index_bounds_builder.cpp
index eeb1c2f1114..a326b960192 100644
--- a/src/mongo/db/query/index_bounds_builder.cpp
+++ b/src/mongo/db/query/index_bounds_builder.cpp
@@ -54,6 +54,23 @@ namespace mongo {
 
 namespace {
 
+// Helper for checking that an OIL "appears" to be ascending given one interval.
+void assertOILIsAscendingLocally(const vector<Interval>& intervals, size_t idx) {
+    // Each individual interval being examined should be ascending or none.
+    const auto dir = intervals[idx].getDirection();
+
+    // Should be either ascending, or have no direction (be a point/null/empty interval).
+    invariant(dir == Interval::Direction::kDirectionAscending ||
+              dir == Interval::Direction::kDirectionNone);
+
+    // The previous OIL's end value should be <= the next OIL's start value.
+    if (idx > 0) {
+        // Pass 'false' to avoid comparing the field names.
+        const int res = intervals[idx - 1].end.woCompare(intervals[idx].start, false);
+        invariant(res <= 0);
+    }
+}
+
 // Tightness rules are shared for $lt, $lte, $gt, $gte.
 IndexBoundsBuilder::BoundsTightness getInequalityPredicateTightness(const BSONElement& dataElt,
                                                                     const IndexEntry& index) {
@@ -624,52 +641,57 @@ Interval IndexBoundsBuilder::makeRangeInterval(const BSONObj& obj, BoundInclusio
 }
 
 // static
-void IndexBoundsBuilder::intersectize(const OrderedIntervalList& arg, OrderedIntervalList* oilOut) {
-    verify(arg.name == oilOut->name);
+void IndexBoundsBuilder::intersectize(const OrderedIntervalList& oilA, OrderedIntervalList* oilB) {
+    invariant(oilB);
+    invariant(oilA.name == oilB->name);
 
-    size_t argidx = 0;
-    const vector<Interval>& argiv = arg.intervals;
+    size_t oilAIdx = 0;
+    const vector<Interval>& oilAIntervals = oilA.intervals;
 
-    size_t ividx = 0;
-    vector<Interval>& iv = oilOut->intervals;
+    size_t oilBIdx = 0;
+    vector<Interval>& oilBIntervals = oilB->intervals;
 
     vector<Interval> result;
 
-    while (argidx < argiv.size() && ividx < iv.size()) {
-        Interval::IntervalComparison cmp = argiv[argidx].compare(iv[ividx]);
+    while (oilAIdx < oilAIntervals.size() && oilBIdx < oilBIntervals.size()) {
+        if (kDebugBuild) {
+            // Ensure that both OILs are ascending.
+            assertOILIsAscendingLocally(oilAIntervals, oilAIdx);
+            assertOILIsAscendingLocally(oilBIntervals, oilBIdx);
+        }
 
+        Interval::IntervalComparison cmp = oilAIntervals[oilAIdx].compare(oilBIntervals[oilBIdx]);
         verify(Interval::INTERVAL_UNKNOWN != cmp);
 
         if (cmp == Interval::INTERVAL_PRECEDES || cmp == Interval::INTERVAL_PRECEDES_COULD_UNION) {
-            // argiv is before iv.  move argiv forward.
-            ++argidx;
+            // oilAIntervals is before oilBIntervals. move oilAIntervals forward.
+            ++oilAIdx;
         } else if (cmp == Interval::INTERVAL_SUCCEEDS) {
-            // iv is before argiv.  move iv forward.
-            ++ividx;
+            // oilBIntervals is before oilAIntervals. move oilBIntervals forward.
+            ++oilBIdx;
         } else {
-            // argiv[argidx] (cmpresults) iv[ividx]
-            Interval newInt = argiv[argidx];
-            newInt.intersect(iv[ividx], cmp);
+            Interval newInt = oilAIntervals[oilAIdx];
+            newInt.intersect(oilBIntervals[oilBIdx], cmp);
             result.push_back(newInt);
 
             if (Interval::INTERVAL_EQUALS == cmp) {
-                ++argidx;
-                ++ividx;
+                ++oilAIdx;
+                ++oilBIdx;
             } else if (Interval::INTERVAL_WITHIN == cmp) {
-                ++argidx;
+                ++oilAIdx;
             } else if (Interval::INTERVAL_CONTAINS == cmp) {
-                ++ividx;
+                ++oilBIdx;
             } else if (Interval::INTERVAL_OVERLAPS_BEFORE == cmp) {
-                ++argidx;
+                ++oilAIdx;
             } else if (Interval::INTERVAL_OVERLAPS_AFTER == cmp) {
-                ++ividx;
+                ++oilBIdx;
             } else {
-                verify(0);
+                MONGO_UNREACHABLE;
             }
         }
     }
 
-    oilOut->intervals.swap(result);
+    oilB->intervals.swap(result);
 }
 
 // static
@@ -892,13 +914,7 @@ void IndexBoundsBuilder::alignBounds(IndexBounds* bounds, const BSONObj& kp, int
         int direction = (elt.number() >= 0) ? 1 : -1;
         direction *= scanDir;
         if (-1 == direction) {
-            vector<Interval>& iv = bounds->fields[oilIdx].intervals;
-            // Step 1: reverse the list.
-            std::reverse(iv.begin(), iv.end());
-            // Step 2: reverse each interval.
-            for (size_t i = 0; i < iv.size(); ++i) {
-                iv[i].reverse();
-            }
+            bounds->fields[oilIdx].reverse();
         }
         ++oilIdx;
     }
diff --git a/src/mongo/db/query/index_bounds_builder_test.cpp b/src/mongo/db/query/index_bounds_builder_test.cpp
index a8ae3651d32..77d447094e5 100644
--- a/src/mongo/db/query/index_bounds_builder_test.cpp
+++ b/src/mongo/db/query/index_bounds_builder_test.cpp
@@ -2187,4 +2187,19 @@ TEST(IndexBoundsBuilderTest, CanUseCoveredMatchingForExistsTrueWithSparseIndex)
     ASSERT_TRUE(IndexBoundsBuilder::canUseCoveredMatching(expr.get(), testIndex));
 }
 
+TEST(IndexBoundsBuilderTest, IntersectizeBasic) {
+    OrderedIntervalList oil1("xyz");
+    oil1.intervals = {Interval(BSON("" << 0 << "" << 5), false, false)};
+
+    OrderedIntervalList oil2("xyz");
+    oil2.intervals = {Interval(BSON("" << 1 << "" << 6), false, false)};
+
+    IndexBoundsBuilder::intersectize(oil1, &oil2);
+
+    OrderedIntervalList expectedIntersection("xyz");
+    expectedIntersection.intervals = {Interval(BSON("" << 1 << "" << 5), false, false)};
+
+    ASSERT_TRUE(oil2 == expectedIntersection);
+}
+
 }  // namespace
diff --git a/src/mongo/db/query/index_bounds_test.cpp b/src/mongo/db/query/index_bounds_test.cpp
index 1fca089e584..5597eaa9f6f 100644
--- a/src/mongo/db/query/index_bounds_test.cpp
+++ b/src/mongo/db/query/index_bounds_test.cpp
@@ -122,6 +122,58 @@ TEST(IndexBoundsTest, ValidOverlapOnlyWhenBothOpen) {
     ASSERT(bounds.isValidFor(BSON("foo" << 1), 1));
 }
 
+TEST(IndexBoundsCheckerTest, CheckOILReverse) {
+    // Check that the reverse of an empty list is empty.
+    OrderedIntervalList emptyList("someField");
+    emptyList.reverse();
+    OrderedIntervalList expectedReversedEmptyList("someField");
+    ASSERT_TRUE(emptyList == expectedReversedEmptyList);
+
+    // The reverse of a single-interval OIL is just an OIL with that interval reversed.
+    OrderedIntervalList singleEltList("xyz");
+    singleEltList.intervals = {Interval(BSON("" << 5 << "" << 0), false, false)};
+    singleEltList.reverse();
+
+    OrderedIntervalList expectedReversedSingleEltList("xyz");
+    expectedReversedSingleEltList.intervals = {Interval(BSON("" << 0 << "" << 5), false, false)};
+    ASSERT_TRUE(singleEltList == expectedReversedSingleEltList);
+
+    // List with a few elements
+    OrderedIntervalList fooList("foo");
+    fooList.intervals = {Interval(BSON("" << 40 << "" << 35), false, true),
+                         Interval(BSON("" << 30 << "" << 21), true, true),
+                         Interval(BSON("" << 20 << "" << 7), true, false)};
+    fooList.reverse();
+
+    OrderedIntervalList expectedReverseFooList("foo");
+    expectedReverseFooList.intervals = {Interval(BSON("" << 7 << "" << 20), false, true),
+                                        Interval(BSON("" << 21 << "" << 30), true, true),
+                                        Interval(BSON("" << 35 << "" << 40), true, false)};
+
+    ASSERT_TRUE(fooList == expectedReverseFooList);
+}
+
+TEST(IndexBoundsTest, OILReverseClone) {
+    OrderedIntervalList emptyA("foo");
+    OrderedIntervalList emptyB = emptyA.reverseClone();
+
+    ASSERT(emptyA == emptyB);
+    ASSERT(emptyA.computeDirection() == Interval::Direction::kDirectionNone);
+    ASSERT(emptyB.computeDirection() == Interval::Direction::kDirectionNone);
+
+    OrderedIntervalList list("foo");
+
+    list.intervals.push_back(Interval(BSON("" << 7 << "" << 20), true, false));
+    list.intervals.push_back(Interval(BSON("" << 20 << "" << 25), false, true));
+
+    OrderedIntervalList listClone = list.reverseClone();
+    OrderedIntervalList reverseList("foo");
+    reverseList.intervals = {Interval(BSON("" << 25 << "" << 20), true, false),
+                             Interval(BSON("" << 20 << "" << 7), false, true)};
+    ASSERT(reverseList == listClone);
+    ASSERT(listClone.computeDirection() == Interval::Direction::kDirectionDescending);
+}
+
 //
 // Tests for OrderedIntervalList::complement()
 //
@@ -519,6 +571,47 @@ TEST(IndexBoundsTest, SimpleRangeBoundsNotEqualDifferentEndKeyInclusive) {
     ASSERT_TRUE(bounds1 != bounds2);
 }
 
+TEST(IndexBoundsTest, ForwardizeSimpleRange) {
+    IndexBounds bounds1;
+    bounds1.isSimpleRange = true;
+    bounds1.startKey = BSON("" << 2 << "" << 4);
+    bounds1.endKey = BSON("" << 1 << "" << 3);
+    bounds1.boundInclusion = BoundInclusion::kIncludeStartKeyOnly;
+
+    IndexBounds expectedBounds1;
+    expectedBounds1.isSimpleRange = true;
+    expectedBounds1.startKey = bounds1.endKey;
+    expectedBounds1.endKey = bounds1.startKey;
+    expectedBounds1.boundInclusion = BoundInclusion::kIncludeEndKeyOnly;
+    ASSERT(bounds1.forwardize() == expectedBounds1);
+
+    IndexBounds bounds2;
+    bounds1.isSimpleRange = true;
+    bounds1.startKey = BSON("" << 1 << "" << 3);
+    bounds1.endKey = BSON("" << 2 << "" << 4);
+    bounds1.boundInclusion = BoundInclusion::kIncludeStartKeyOnly;
+    ASSERT(bounds2 == bounds2.forwardize());
+}
+
+
+TEST(IndexBoundsTest, ForwardizeOnNonSimpleRangeShouldOnlyReverseDescendingRanges) {
+    OrderedIntervalList fooList("foo");
+    fooList.intervals = {Interval(BSON("" << 7 << "" << 20), true, true)};
+
+    OrderedIntervalList barList("bar");
+    barList.intervals = {Interval(BSON("" << 10 << "" << 5), false, false),
+                         Interval(BSON("" << 4 << "" << 3), false, false)};
+
+    IndexBounds bounds;
+    bounds.fields = {fooList, barList};
+
+    IndexBounds forwardizedBounds = bounds.forwardize();
+
+    IndexBounds expectedBounds;
+    expectedBounds.fields = {fooList, barList.reverseClone()};
+    ASSERT(expectedBounds == forwardizedBounds);
+}
+
 //
 // Iteration over
 //
diff --git a/src/mongo/db/query/interval.cpp b/src/mongo/db/query/interval.cpp
index df80321eb60..187d0704a20 100644
--- a/src/mongo/db/query/interval.cpp
+++ b/src/mongo/db/query/interval.cpp
@@ -66,6 +66,19 @@ bool Interval::isNull() const {
     return (!startInclusive || !endInclusive) && 0 == start.woCompare(end, false);
 }
 
+Interval::Direction Interval::getDirection() const {
+    if (isEmpty() || isPoint() || isNull()) {
+        return Direction::kDirectionNone;
+    }
+
+    // 'false' to not consider the field name.
+    const int res = start.woCompare(end, false);
+
+    invariant(res != 0);
+    return res < 0 ? Direction::kDirectionAscending : Direction::kDirectionDescending;
+}
+
+
 //
 // Comparison
 //
@@ -93,6 +106,17 @@ bool Interval::equals(const Interval& other) const {
 }
 
 bool Interval::intersects(const Interval& other) const {
+    if (kDebugBuild) {
+        // This function assumes that both intervals are ascending (or are empty/point intervals).
+        // Determining this may be expensive, so we only do these checks when in a debug build.
+        const auto thisDir = getDirection();
+        invariant(thisDir == Direction::kDirectionAscending ||
+                  thisDir == Direction::kDirectionNone);
+        const auto otherDir = other.getDirection();
+        invariant(otherDir == Direction::kDirectionAscending ||
+                  otherDir == Direction::kDirectionNone);
+    }
+
     int res = this->start.woCompare(other.end, false);
     if (res > 0) {
         return false;
@@ -261,4 +285,15 @@ void Interval::reverse() {
     std::swap(startInclusive, endInclusive);
 }
 
+Interval Interval::reverseClone() const {
+    Interval reversed;
+    reversed.start = end;
+    reversed.end = start;
+    reversed.startInclusive = endInclusive;
+    reversed.endInclusive = startInclusive;
+    reversed._intervalData = _intervalData;
+
+    return reversed;
+}
+
 }  // namespace mongo
diff --git a/src/mongo/db/query/interval.h b/src/mongo/db/query/interval.h
index 66767036f95..21b2cfce4ab 100644
--- a/src/mongo/db/query/interval.h
+++ b/src/mongo/db/query/interval.h
@@ -98,6 +98,18 @@ struct Interval {
      */
     bool isNull() const;
 
+    enum class Direction {
+        // Point intervals, empty intervals, and null intervals have no direction.
+        kDirectionNone,
+        kDirectionAscending,
+        kDirectionDescending
+    };
+
+    /**
+     * Compute the direction.
+     */
+    Direction getDirection() const;
+
     //
     // Comparison with other intervals
     //
@@ -169,6 +181,11 @@ struct Interval {
     void reverse();
 
     /**
+     * Return a new Interval that's a reverse of this one.
+     */
+    Interval reverseClone() const;
+
+    /**
      * Updates 'this' with the intersection of 'this' and 'other'. If 'this' and 'other'
      * have been compare()d before, that result can be optionally passed in 'cmp'
      */
@@ -182,7 +199,7 @@ struct Interval {
 };
 
 inline bool operator==(const Interval& lhs, const Interval& rhs) {
-    return lhs.compare(rhs) == Interval::INTERVAL_EQUALS;
+    return lhs.equals(rhs);
 }
 
 inline bool operator!=(const Interval& lhs, const Interval& rhs) {
diff --git a/src/mongo/db/query/interval_test.cpp b/src/mongo/db/query/interval_test.cpp
index d9e829a254b..608f7e25459 100644
--- a/src/mongo/db/query/interval_test.cpp
+++ b/src/mongo/db/query/interval_test.cpp
@@ -293,4 +293,41 @@ TEST(Union, Succeds) {
     ASSERT_EQUALS(a.compare(Interval(itv, true, true)), Interval::INTERVAL_EQUALS);
 }
 
+TEST(Introspection, GetDirection) {
+    // Empty/uninitialized Interval.
+    boost::optional<Interval> i;
+    i.emplace();
+    ASSERT(i->getDirection() == Interval::Direction::kDirectionNone);
+
+    // Empty Interval.
+    i.emplace(BSON("" << 10 << "" << 10), false, false);
+    ASSERT(i->getDirection() == Interval::Direction::kDirectionNone);
+
+    // Point bound Interval.
+    i.emplace(BSON("" << 10 << "" << 10), true, true);
+    ASSERT(i->getDirection() == Interval::Direction::kDirectionNone);
+
+    // Ascending interval.
+    i.emplace(BSON("" << 10 << "" << 20), true, true);
+    ASSERT(i->getDirection() == Interval::Direction::kDirectionAscending);
+
+    // Descending interval.
+    i.emplace(BSON("" << 11 << "" << 10), true, true);
+    ASSERT(i->getDirection() == Interval::Direction::kDirectionDescending);
+}
+
+TEST(Copying, ReverseClone) {
+    Interval a(BSON("" << 10 << "" << 20), false, true);
+    ASSERT(a.reverseClone() == Interval(BSON("" << 20 << "" << 10), true, false));
+    ASSERT(a.reverseClone() != a);
+
+    Interval b(BSON("" << 10 << "" << 5), true, true);
+    ASSERT(b.reverseClone() == Interval(BSON("" << 5 << "" << 10), true, true));
+    ASSERT(b.reverseClone() != b);
+
+    Interval c(BSON("" << 1 << "" << 1), true, true);
+    ASSERT(c.reverseClone() == c);
+}
+
+
 }  // unnamed namespace
diff --git a/src/mongo/db/query/query_planner_common.cpp b/src/mongo/db/query/query_planner_common.cpp
index 337f3a045fc..1347332c077 100644
--- a/src/mongo/db/query/query_planner_common.cpp
+++ b/src/mongo/db/query/query_planner_common.cpp
@@ -46,27 +46,11 @@ void QueryPlannerCommon::reverseScans(QuerySolutionNode* node) {
         if (isn->bounds.isSimpleRange) {
             std::swap(isn->bounds.startKey, isn->bounds.endKey);
             // If only one bound is included, swap which one is included.
-            switch (isn->bounds.boundInclusion) {
-                case BoundInclusion::kIncludeStartKeyOnly:
-                    isn->bounds.boundInclusion = BoundInclusion::kIncludeEndKeyOnly;
-                    break;
-                case BoundInclusion::kIncludeEndKeyOnly:
-                    isn->bounds.boundInclusion = BoundInclusion::kIncludeStartKeyOnly;
-                    break;
-                case BoundInclusion::kIncludeBothStartAndEndKeys:
-                case BoundInclusion::kExcludeBothStartAndEndKeys:
-                    // These are both symmetric so no change needed.
-                    break;
-            }
+            isn->bounds.boundInclusion =
+                IndexBounds::reverseBoundInclusion(isn->bounds.boundInclusion);
         } else {
             for (size_t i = 0; i < isn->bounds.fields.size(); ++i) {
-                std::vector<Interval>& iv = isn->bounds.fields[i].intervals;
-                // Step 1: reverse the list.
-                std::reverse(iv.begin(), iv.end());
-                // Step 2: reverse each interval.
-                for (size_t j = 0; j < iv.size(); ++j) {
-                    iv[j].reverse();
-                }
+                isn->bounds.fields[i].reverse();
             }
         }
 
diff --git a/src/mongo/db/query/query_solution.cpp b/src/mongo/db/query/query_solution.cpp
index aa2426954c4..9ab324ab3bd 100644
--- a/src/mongo/db/query/query_solution.cpp
+++ b/src/mongo/db/query/query_solution.cpp
@@ -594,9 +594,13 @@ bool IndexScanNode::sortedByDiskLoc() const {
 }
 
 // static
-std::set<StringData> IndexScanNode::getFieldsWithStringBounds(const IndexBounds& bounds,
+std::set<StringData> IndexScanNode::getFieldsWithStringBounds(const IndexBounds& inputBounds,
                                                               const BSONObj& indexKeyPattern) {
-    BSONObjIterator keyPatternIterator = indexKeyPattern.begin();
+    // Produce a copy of the bounds which are all ascending, as we can only compute intersections
+    // of ascending bounds.
+    IndexBounds bounds = inputBounds.forwardize();
+
+    BSONObjIterator keyPatternIterator(indexKeyPattern);
 
     if (bounds.isSimpleRange) {
         // With a simple range, the only cases we can say for sure do not contain strings