SERVER-23172 Allow use of indices for collation-aware queries that match nested objects or arrays.

13 files changed, 243 insertions, 184 deletions

diff --git a/src/mongo/db/index/btree_key_generator_test.cpp b/src/mongo/db/index/btree_key_generator_test.cpp
index ce0b53dd6d8..7840056b447 100644
--- a/src/mongo/db/index/btree_key_generator_test.cpp
+++ b/src/mongo/db/index/btree_key_generator_test.cpp
@@ -1070,22 +1070,22 @@ TEST(BtreeKeyGeneratorTest, CollatorDoesNotAffectNonStringKeys) {
         testKeygen(keyPattern, genKeysFrom, expectedKeys, expectedMultikeyPaths, false, &collator));
 }
 
-TEST(BtreeKeyGeneratorTest, CollatorDoesNotAffectNestedObjectKeys) {
+TEST(BtreeKeyGeneratorTest, GetCollationAwareKeysFromNestedObject) {
     BSONObj keyPattern = fromjson("{a: 1}");
     BSONObj genKeysFrom = fromjson("{b: 4, a: {c: 'foo'}}");
     BSONObjSet expectedKeys;
-    expectedKeys.insert(fromjson("{'': {c: 'foo'}}"));
+    expectedKeys.insert(fromjson("{'': {c: 'oof'}}"));
     CollatorInterfaceMock collator(CollatorInterfaceMock::MockType::kReverseString);
     MultikeyPaths expectedMultikeyPaths{std::set<size_t>{}};
     ASSERT(
         testKeygen(keyPattern, genKeysFrom, expectedKeys, expectedMultikeyPaths, false, &collator));
 }
 
-TEST(BtreeKeyGeneratorTest, CollatorDoesNotAffectNestedArrayKeys) {
+TEST(BtreeKeyGeneratorTest, GetCollationAwareKeysFromNestedArray) {
     BSONObj keyPattern = fromjson("{a: 1}");
     BSONObj genKeysFrom = fromjson("{b: 4, a: {c: ['foo', 'bar', 'baz']}}");
     BSONObjSet expectedKeys;
-    expectedKeys.insert(fromjson("{'': {c: ['foo', 'bar', 'baz']}}"));
+    expectedKeys.insert(fromjson("{'': {c: ['oof', 'rab', 'zab']}}"));
     CollatorInterfaceMock collator(CollatorInterfaceMock::MockType::kReverseString);
     MultikeyPaths expectedMultikeyPaths{std::set<size_t>{}};
     ASSERT(
diff --git a/src/mongo/db/index/hash_key_generator_test.cpp b/src/mongo/db/index/hash_key_generator_test.cpp
index 047c59b9994..b2bf9a2755a 100644
--- a/src/mongo/db/index/hash_key_generator_test.cpp
+++ b/src/mongo/db/index/hash_key_generator_test.cpp
@@ -97,16 +97,16 @@ TEST(HashKeyGeneratorTest, CollationDoesNotAffectNonStringFields) {
     ASSERT(assertKeysetsEqual(expectedKeys, actualKeys));
 }
 
-// TODO SERVER-23172: remove test.
-TEST(HashKeyGeneratorTest, CollationDoesNotAffectStringsInEmbeddedDocuments) {
+TEST(HashKeyGeneratorTest, CollatorAppliedBeforeHashingNestedObject) {
     BSONObj obj = fromjson("{a: {b: 'string'}}");
+    BSONObj backwardsObj = fromjson("{a: {b: 'gnirts'}}");
     BSONObjSet actualKeys;
     CollatorInterfaceMock collator(CollatorInterfaceMock::MockType::kReverseString);
     ExpressionKeysPrivate::getHashKeys(
         obj, "a", kHashSeed, kHashVersion, false, &collator, &actualKeys);
 
     BSONObjSet expectedKeys;
-    expectedKeys.insert(makeHashKey(obj["a"]));
+    expectedKeys.insert(makeHashKey(backwardsObj["a"]));
 
     ASSERT(assertKeysetsEqual(expectedKeys, actualKeys));
 }
diff --git a/src/mongo/db/index/s2_key_generator_test.cpp b/src/mongo/db/index/s2_key_generator_test.cpp
index e7399eed0b3..894e7b8036e 100644
--- a/src/mongo/db/index/s2_key_generator_test.cpp
+++ b/src/mongo/db/index/s2_key_generator_test.cpp
@@ -357,8 +357,7 @@ TEST(S2KeyGeneratorTest, CollationDoesNotAffectNonStringFields) {
                              actualMultikeyPaths);
 }
 
-// TODO SERVER-23172: remove test.
-TEST(S2KeyGeneratorTest, CollationDoesNotAffectStringsInEmbeddedDocuments) {
+TEST(S2KeyGeneratorTest, CollationAppliedToStringsInNestedObjects) {
     BSONObj obj = fromjson("{a: {type: 'Point', coordinates: [0, 0]}, b: {c: 'string'}}");
     BSONObj keyPattern = fromjson("{a: '2dsphere', b: 1}");
     BSONObj infoObj = fromjson("{key: {a: '2dsphere', b: 1}, '2dsphereIndexVersion': 3}");
@@ -372,7 +371,7 @@ TEST(S2KeyGeneratorTest, CollationDoesNotAffectStringsInEmbeddedDocuments) {
 
     BSONObjSet expectedKeys;
     expectedKeys.insert(BSON("" << getCellID(0, 0) << "" << BSON("c"
-                                                                 << "string")));
+                                                                 << "gnirts")));
 
     assertKeysetsEqual(expectedKeys, actualKeys);
     assertMultikeyPathsEqual(MultikeyPaths{std::set<size_t>{}, std::set<size_t>{}},
diff --git a/src/mongo/db/matcher/expression_algo.cpp b/src/mongo/db/matcher/expression_algo.cpp
index ce7cfa70924..6365d3326bb 100644
--- a/src/mongo/db/matcher/expression_algo.cpp
+++ b/src/mongo/db/matcher/expression_algo.cpp
@@ -37,6 +37,7 @@
 #include "mongo/db/matcher/expression_leaf.h"
 #include "mongo/db/matcher/expression_tree.h"
 #include "mongo/db/pipeline/dependencies.h"
+#include "mongo/db/query/collation/collation_index_key.h"
 #include "mongo/db/query/collation/collator_interface.h"
 
 namespace mongo {
@@ -95,12 +96,9 @@ bool _isSubsetOf(const ComparisonMatchExpression* lhs, const ComparisonMatchExpr
         return false;
     }
 
-    if (!CollatorInterface::collatorsMatch(lhs->getCollator(), rhs->getCollator())) {
-        // TODO SERVER-23172: Check that lhsData does not contain string comparison in nested
-        // objects or arrays.
-        if (lhsData.type() == BSONType::String) {
-            return false;
-        }
+    if (!CollatorInterface::collatorsMatch(lhs->getCollator(), rhs->getCollator()) &&
+        CollationIndexKey::isCollatableType(lhsData.type())) {
+        return false;
     }
 
     // Either collator may be used by compareElementValues() here, since either the collators are
diff --git a/src/mongo/db/matcher/expression_algo_test.cpp b/src/mongo/db/matcher/expression_algo_test.cpp
index 1eccce190bb..b9e6cafe134 100644
--- a/src/mongo/db/matcher/expression_algo_test.cpp
+++ b/src/mongo/db/matcher/expression_algo_test.cpp
@@ -692,13 +692,14 @@ TEST(ExpressionAlgoIsSubsetOf, CollationAwareStringComparisonIn) {
     ASSERT_FALSE(expression::isSubsetOf(lhsSomeGTcba.get(), rhsLT.get()));
 }
 
+// TODO SERVER-24674: isSubsetOf should return true after exploring nested objects.
 TEST(ExpressionAlgoIsSubsetOf, NonMatchingCollationsNoStringComparisonLHS) {
     CollatorInterfaceMock collatorAlwaysEqual(CollatorInterfaceMock::MockType::kAlwaysEqual);
     CollatorInterfaceMock collatorReverseString(CollatorInterfaceMock::MockType::kReverseString);
     ParsedMatchExpression lhs("{a: {b: 1}}", &collatorAlwaysEqual);
     ParsedMatchExpression rhs("{a: {$lt: {b: 'abc'}}}", &collatorReverseString);
 
-    ASSERT_TRUE(expression::isSubsetOf(lhs.get(), rhs.get()));
+    ASSERT_FALSE(expression::isSubsetOf(lhs.get(), rhs.get()));
 }
 
 TEST(ExpressionAlgoIsSubsetOf, NonMatchingCollationsNoStringComparison) {
diff --git a/src/mongo/db/query/collation/collation_index_key.cpp b/src/mongo/db/query/collation/collation_index_key.cpp
index eb25acc3f79..ed0f348a887 100644
--- a/src/mongo/db/query/collation/collation_index_key.cpp
+++ b/src/mongo/db/query/collation/collation_index_key.cpp
@@ -33,26 +33,91 @@
 #include "mongo/bson/bsonobj.h"
 #include "mongo/bson/bsonobjbuilder.h"
 #include "mongo/db/query/collation/collator_interface.h"
+#include "mongo/util/assert_util.h"
 
 namespace mongo {
 
-// TODO SERVER-23172: Update this to consider strings inside nested objects or arrays.
+void CollationIndexKey::translate(StringData fieldName,
+                                  BSONElement element,
+                                  const CollatorInterface* collator,
+                                  BSONObjBuilder* out) {
+    invariant(collator);
+
+    switch (element.type()) {
+        case BSONType::String: {
+            out->append(fieldName,
+                        collator->getComparisonKey(element.valueStringData()).getKeyData());
+            return;
+        }
+        case BSONType::Object: {
+            BSONObjBuilder bob(out->subobjStart(fieldName));
+            for (const BSONElement& elt : element.embeddedObject()) {
+                translate(elt.fieldNameStringData(), elt, collator, &bob);
+            }
+            bob.doneFast();
+            return;
+        }
+        case BSONType::Array: {
+            BSONArrayBuilder bab(out->subarrayStart(fieldName));
+            for (const BSONElement& elt : element.Array()) {
+                translate(elt, collator, &bab);
+            }
+            bab.doneFast();
+            return;
+        }
+        default:
+            out->appendAs(element, fieldName);
+    }
+}
+
+void CollationIndexKey::translate(BSONElement element,
+                                  const CollatorInterface* collator,
+                                  BSONArrayBuilder* out) {
+    invariant(collator);
+
+    switch (element.type()) {
+        case BSONType::String: {
+            out->append(collator->getComparisonKey(element.valueStringData()).getKeyData());
+            return;
+        }
+        case BSONType::Object: {
+            BSONObjBuilder bob(out->subobjStart());
+            for (const BSONElement& elt : element.embeddedObject()) {
+                translate(elt.fieldNameStringData(), elt, collator, &bob);
+            }
+            bob.doneFast();
+            return;
+        }
+        case BSONType::Array: {
+            BSONArrayBuilder bab(out->subarrayStart());
+            for (const BSONElement& elt : element.Array()) {
+                translate(elt, collator, &bab);
+            }
+            bab.doneFast();
+            return;
+        }
+        default:
+            out->append(element);
+    }
+}
+
+// TODO SERVER-24674: We may want to check that objects and arrays actually do contain strings
+// before returning true.
 bool CollationIndexKey::shouldUseCollationIndexKey(BSONElement elt,
                                                    const CollatorInterface* collator) {
-    return collator && elt.type() == BSONType::String;
+    return collator && isCollatableType(elt.type());
 }
 
-// TODO SERVER-23172: Update this to convert strings inside nested objects or arrays to their
-// corresponding comparison keys.
 void CollationIndexKey::collationAwareIndexKeyAppend(BSONElement elt,
                                                      const CollatorInterface* collator,
                                                      BSONObjBuilder* out) {
-    if (shouldUseCollationIndexKey(elt, collator)) {
-        auto comparisonKey = collator->getComparisonKey(elt.valueStringData());
-        out->append("", comparisonKey.getKeyData());
-    } else {
+    invariant(out);
+    if (!collator) {
         out->appendAs(elt, "");
+        return;
     }
+
+    translate("", elt, collator, out);
 }
 
 }  // namespace mongo
diff --git a/src/mongo/db/query/collation/collation_index_key.h b/src/mongo/db/query/collation/collation_index_key.h
index 7d86935b11a..dd733e5a08c 100644
--- a/src/mongo/db/query/collation/collation_index_key.h
+++ b/src/mongo/db/query/collation/collation_index_key.h
@@ -28,6 +28,8 @@
 
 #pragma once
 
+#include "mongo/bson/bsontypes.h"
+
 namespace mongo {
 
 class BSONElement;
@@ -41,7 +43,17 @@ class CollatorInterface;
 class CollationIndexKey {
 public:
     /**
-     * Returns true if the index key for 'elt' should be a comparison key generated by 'collator'.
+     * Returns true if type is affected in comparison or sort order by collation.
+     */
+    static bool isCollatableType(BSONType type) {
+        // TODO SERVER-24674 We may want to replace calls to this to shouldUseCollationIndexKey.
+        return type == BSONType::String || type == BSONType::Object || type == BSONType::Array;
+    }
+
+    /**
+     * Returns true if the index key for 'elt' may contain a comparison key generated by 'collator'.
+     *
+     * This is the case when 'elt' is a String, Array, or Object.
      */
     static bool shouldUseCollationIndexKey(BSONElement elt, const CollatorInterface* collator);
 
@@ -52,6 +64,20 @@ public:
     static void collationAwareIndexKeyAppend(BSONElement elt,
                                              const CollatorInterface* collator,
                                              BSONObjBuilder* out);
+
+private:
+    // Translate all strings in 'element' into comparison keys using 'collator'. The result is
+    // appended to the BSONObjBuilder out, with field name fieldName.
+    static void translate(StringData fieldName,
+                          BSONElement element,
+                          const CollatorInterface* collator,
+                          BSONObjBuilder* out);
+
+    // Translate all strings in 'element' into comparison keys using 'collator'.  The result is
+    // append to the BSONArrayBuilder out.
+    static void translate(BSONElement element,
+                          const CollatorInterface* collator,
+                          BSONArrayBuilder* out);
 };
 
 }  // namespace mongo
diff --git a/src/mongo/db/query/collation/collation_index_key_test.cpp b/src/mongo/db/query/collation/collation_index_key_test.cpp
index 88d5b13c70c..bcd341c9488 100644
--- a/src/mongo/db/query/collation/collation_index_key_test.cpp
+++ b/src/mongo/db/query/collation/collation_index_key_test.cpp
@@ -31,6 +31,7 @@
 #include "mongo/db/query/collation/collation_index_key.h"
 
 #include "mongo/bson/bsonobjbuilder.h"
+#include "mongo/bson/json.h"
 #include "mongo/db/query/collation/collator_interface_mock.h"
 #include "mongo/unittest/unittest.h"
 
@@ -38,26 +39,40 @@ namespace {
 
 using namespace mongo;
 
-TEST(CollationIndexKeyTest, ShouldUseCollationKeyFalseWithNullCollator) {
+TEST(CollationIndexKeyTest, ShouldUseCollationIndexKeyFalseWithNullCollator) {
     BSONObj obj = BSON("foo"
                        << "string");
     ASSERT_FALSE(CollationIndexKey::shouldUseCollationIndexKey(obj.firstElement(), nullptr));
 }
 
-TEST(CollationIndexKeyTest, ShouldUseCollationKeyFalseWithNonStringElement) {
+TEST(CollationIndexKeyTest, ShouldUseCollationIndexKeyTrueWithObjectElement) {
     CollatorInterfaceMock collator(CollatorInterfaceMock::MockType::kReverseString);
     BSONObj obj = BSON("foo" << BSON("bar"
                                      << "string"));
-    ASSERT_FALSE(CollationIndexKey::shouldUseCollationIndexKey(obj.firstElement(), &collator));
+    ASSERT_TRUE(CollationIndexKey::shouldUseCollationIndexKey(obj.firstElement(), &collator));
 }
 
-TEST(CollationIndexKeyTest, ShouldUseCollationKeyTrueWithStringElement) {
+TEST(CollationIndexKeyTest, ShouldUseCollationIndexKeyTrueWithArrayElement) {
+    CollatorInterfaceMock collator(CollatorInterfaceMock::MockType::kReverseString);
+    BSONObj obj = BSON("foo" << BSON_ARRAY("one"
+                                           << "two"));
+    ASSERT_TRUE(CollationIndexKey::shouldUseCollationIndexKey(obj.firstElement(), &collator));
+}
+
+TEST(CollationIndexKeyTest, ShouldUseCollationIndexKeyTrueWithStringElement) {
     CollatorInterfaceMock collator(CollatorInterfaceMock::MockType::kReverseString);
     BSONObj obj = BSON("foo"
                        << "string");
     ASSERT_TRUE(CollationIndexKey::shouldUseCollationIndexKey(obj.firstElement(), &collator));
 }
 
+TEST(CollationIndexKeyTest, CollationAwareAppendCorrectlyAppendsElementWithNullCollator) {
+    BSONObj dataObj = BSON("test" << 1);
+    BSONObjBuilder out;
+    CollationIndexKey::collationAwareIndexKeyAppend(dataObj.firstElement(), nullptr, &out);
+    ASSERT_EQ(out.obj(), BSON("" << 1));
+}
+
 TEST(CollationIndexKeyTest, CollationAwareAppendReversesStringWithReverseMockCollator) {
     CollatorInterfaceMock collator(CollatorInterfaceMock::MockType::kReverseString);
     BSONObj dataObj = BSON("foo"
@@ -99,4 +114,44 @@ TEST(CollationIndexKeyTest, CollationAwareAppendCorrectlySerializesWithEmbeddedN
     ASSERT_EQ(out.obj(), expectedObj);
 }
 
+TEST(CollationIndexKeyTest, CollationAwareAppendCorrectlyReversesSimpleEmbeddedObject) {
+    CollatorInterfaceMock collator(CollatorInterfaceMock::MockType::kReverseString);
+    BSONObj dataObj = BSON("" << BSON("a"
+                                      << "!foo"));
+    BSONObj expected = BSON("" << BSON("a"
+                                       << "oof!"));
+
+    BSONObjBuilder out;
+    CollationIndexKey::collationAwareIndexKeyAppend(dataObj.firstElement(), &collator, &out);
+    ASSERT_EQ(out.obj(), expected);
+}
+
+TEST(CollationIndexKeyTest, CollationAwareAppendCorrectlyReversesSimpleEmbeddedArray) {
+    CollatorInterfaceMock collator(CollatorInterfaceMock::MockType::kReverseString);
+    BSONObj dataObj = BSON("" << BSON_ARRAY("foo"
+                                            << "bar"));
+    BSONObj expected = BSON("" << BSON_ARRAY("oof"
+                                             << "rab"));
+
+    BSONObjBuilder out;
+    CollationIndexKey::collationAwareIndexKeyAppend(dataObj.firstElement(), &collator, &out);
+    ASSERT_EQ(out.obj(), expected);
+}
+
+TEST(CollationIndexKeyTest, CollationAwareAppendCorrectlyReversesComplexNesting) {
+    CollatorInterfaceMock collator(CollatorInterfaceMock::MockType::kReverseString);
+    BSONObj dataObj = fromjson(
+        "{ '' : [{'a': 'ha', 'b': 2},"
+        "'bar',"
+        "{'c': 2, 'd': 'ah', 'e': 'abc', 'f': ['cba', 'xyz']}]})");
+    BSONObj expected = fromjson(
+        "{ '' : [{'a': 'ah', 'b': 2},"
+        "'rab',"
+        "{'c': 2, 'd': 'ha', 'e': 'cba', 'f': ['abc', 'zyx']}]})");
+
+    BSONObjBuilder out;
+    CollationIndexKey::collationAwareIndexKeyAppend(dataObj.firstElement(), &collator, &out);
+    ASSERT_EQ(out.obj(), expected);
+}
+
 }  // namespace
diff --git a/src/mongo/db/query/planner_ixselect.cpp b/src/mongo/db/query/planner_ixselect.cpp
index 7c32d516260..9b1214caf9f 100644
--- a/src/mongo/db/query/planner_ixselect.cpp
+++ b/src/mongo/db/query/planner_ixselect.cpp
@@ -168,16 +168,9 @@ bool QueryPlannerIXSelect::compatible(const BSONElement& elt,
                                       const IndexEntry& index,
                                       MatchExpression* node,
                                       const CollatorInterface* collator) {
-    // Nested object or array comparisons require the query collator to be null.
-    // TODO SERVER-23172: remove this check.
-    if (collator != nullptr &&
-        (boundsGeneratingNodeContainsComparisonToType(node, BSONType::Object) ||
-         boundsGeneratingNodeContainsComparisonToType(node, BSONType::Array))) {
-        return false;
-    }
-
-    // String comparisons require the collators to match.
-    if (boundsGeneratingNodeContainsComparisonToType(node, BSONType::String) &&
+    if ((boundsGeneratingNodeContainsComparisonToType(node, BSONType::String) ||
+         boundsGeneratingNodeContainsComparisonToType(node, BSONType::Array) ||
+         boundsGeneratingNodeContainsComparisonToType(node, BSONType::Object)) &&
         !CollatorInterface::collatorsMatch(collator, index.collator)) {
         return false;
     }
diff --git a/src/mongo/db/query/planner_ixselect_test.cpp b/src/mongo/db/query/planner_ixselect_test.cpp
index 098c921e5bd..1c273db8716 100644
--- a/src/mongo/db/query/planner_ixselect_test.cpp
+++ b/src/mongo/db/query/planner_ixselect_test.cpp
@@ -407,6 +407,60 @@ TEST(QueryPlannerIXSelectTest, StringGTEqualCollators) {
 }
 
 /**
+ * $gt array comparison requires matching collator.
+ */
+TEST(QueryPlannerIXSelectTest, ArrayGTUnequalCollators) {
+    CollatorInterfaceMock collator(CollatorInterfaceMock::MockType::kAlwaysEqual);
+    IndexEntry index(BSON("a" << 1));
+    CollatorInterfaceMock indexCollator(CollatorInterfaceMock::MockType::kReverseString);
+    index.collator = &indexCollator;
+    std::vector<IndexEntry> indices;
+    indices.push_back(index);
+    std::set<size_t> expectedIndices;
+    testRateIndices("{a: {$gt: ['string']}}", "", &collator, indices, "a", expectedIndices);
+}
+
+/**
+ * $gt array comparison requires matching collator.
+ */
+TEST(QueryPlannerIXSelectTest, ArrayGTEqualCollators) {
+    CollatorInterfaceMock collator(CollatorInterfaceMock::MockType::kAlwaysEqual);
+    IndexEntry index(BSON("a" << 1));
+    index.collator = &collator;
+    std::vector<IndexEntry> indices;
+    indices.push_back(index);
+    std::set<size_t> expectedIndices = {0};
+    testRateIndices("{a: {$gt: ['string']}}", "", &collator, indices, "a", expectedIndices);
+}
+
+/**
+ * $gt object comparison requires matching collator.
+ */
+TEST(QueryPlannerIXSelectTest, NestedObjectGTUnequalCollators) {
+    CollatorInterfaceMock collator(CollatorInterfaceMock::MockType::kAlwaysEqual);
+    IndexEntry index(BSON("a" << 1));
+    CollatorInterfaceMock indexCollator(CollatorInterfaceMock::MockType::kReverseString);
+    index.collator = &indexCollator;
+    std::vector<IndexEntry> indices;
+    indices.push_back(index);
+    std::set<size_t> expectedIndices;
+    testRateIndices("{a: {$gt: {b: 'string'}}}", "", &collator, indices, "a", expectedIndices);
+}
+
+/**
+ * $gt object comparison requires matching collator.
+ */
+TEST(QueryPlannerIXSelectTest, NestedObjectGTEqualCollators) {
+    CollatorInterfaceMock collator(CollatorInterfaceMock::MockType::kAlwaysEqual);
+    IndexEntry index(BSON("a" << 1));
+    index.collator = &collator;
+    std::vector<IndexEntry> indices;
+    indices.push_back(index);
+    std::set<size_t> expectedIndices = {0};
+    testRateIndices("{a: {$gt: {b : 'string'}}}", "", &collator, indices, "a", expectedIndices);
+}
+
+/**
  * $gte string comparison requires matching collator.
  */
 TEST(QueryPlannerIXSelectTest, StringGTEUnequalCollators) {
@@ -843,105 +897,11 @@ TEST(QueryPlannerIXSelectTest, NoStringComparisonType) {
     std::vector<IndexEntry> indices;
     indices.push_back(index);
     std::set<size_t> expectedIndices = {0};
-    testRateIndices("{a: {$type: 'string'}}", "", &collator, indices, "a", expectedIndices);
-}
-
-/**
- * If nested object comparison is done, the query collator must be null.
- * TODO SERVER-23172: remove this test.
- */
-TEST(QueryPlannerIXSelectTest, NestedObjectNonNullCollators) {
-    CollatorInterfaceMock collator(CollatorInterfaceMock::MockType::kReverseString);
-    IndexEntry index(BSON("a" << 1));
-    index.collator = &collator;
-    std::vector<IndexEntry> indices;
-    indices.push_back(index);
-    std::set<size_t> expectedIndices;
-    testRateIndices("{a: {b: 1}}", "", &collator, indices, "a", expectedIndices);
-}
-
-/**
- * If nested object comparison is done, the query collator must be null.
- * TODO SERVER-23172: remove this test.
- */
-TEST(QueryPlannerIXSelectTest, NestedObjectNullQueryCollator) {
-    CollatorInterfaceMock collator(CollatorInterfaceMock::MockType::kReverseString);
-    IndexEntry index(BSON("a" << 1));
-    index.collator = &collator;
-    std::vector<IndexEntry> indices;
-    indices.push_back(index);
-    std::set<size_t> expectedIndices = {0};
-    testRateIndices("{a: {b: 1}}", "", nullptr, indices, "a", expectedIndices);
-}
-
-/**
- * If nested object comparison is done, the query collator must be null.
- */
-TEST(QueryPlannerIXSelectTest, NestedObjectNonNullQueryCollator) {
-    CollatorInterfaceMock collator(CollatorInterfaceMock::MockType::kReverseString);
-    std::vector<IndexEntry> indices;
-    indices.push_back(IndexEntry(BSON("a" << 1)));
-    std::set<size_t> expectedIndices;
-    testRateIndices("{a: {b: 1}}", "", &collator, indices, "a", expectedIndices);
-}
-
-/**
- * If nested object comparison is done, the query collator must be null.
- */
-TEST(QueryPlannerIXSelectTest, NestedObjectNullCollators) {
-    std::vector<IndexEntry> indices;
-    indices.push_back(IndexEntry(BSON("a" << 1)));
-    std::set<size_t> expectedIndices = {0};
-    testRateIndices("{a: {b: 1}}", "", nullptr, indices, "a", expectedIndices);
-}
-
-/**
- * If nested array comparison is done, the query collator must be null.
- * TODO SERVER-23172: remove this test.
- */
-TEST(QueryPlannerIXSelectTest, NestedArrayNonNullCollators) {
-    CollatorInterfaceMock collator(CollatorInterfaceMock::MockType::kReverseString);
-    IndexEntry index(BSON("a" << 1));
-    index.collator = &collator;
-    std::vector<IndexEntry> indices;
-    indices.push_back(index);
-    std::set<size_t> expectedIndices;
-    testRateIndices("{a: [1]}", "", &collator, indices, "a", expectedIndices);
-}
-
-/**
- * If nested array comparison is done, the query collator must be null.
- * TODO SERVER-23172: remove this test.
- */
-TEST(QueryPlannerIXSelectTest, NestedArrayNullQueryCollator) {
-    CollatorInterfaceMock collator(CollatorInterfaceMock::MockType::kReverseString);
-    IndexEntry index(BSON("a" << 1));
-    index.collator = &collator;
-    std::vector<IndexEntry> indices;
-    indices.push_back(index);
-    std::set<size_t> expectedIndices = {0};
-    testRateIndices("{a: [1]}", "", nullptr, indices, "a", expectedIndices);
-}
-
-/**
- * If nested array comparison is done, the query collator must be null.
- */
-TEST(QueryPlannerIXSelectTest, NestedArrayNonNullQueryCollator) {
-    CollatorInterfaceMock collator(CollatorInterfaceMock::MockType::kReverseString);
-    std::vector<IndexEntry> indices;
-    indices.push_back(IndexEntry(BSON("a" << 1)));
-    std::set<size_t> expectedIndices;
-    testRateIndices("{a: [1]}", "", &collator, indices, "a", expectedIndices);
-}
-
-/**
- * If nested array comparison is done, the query collator must be null.
- */
-TEST(QueryPlannerIXSelectTest, NestedArrayNullCollators) {
-    std::vector<IndexEntry> indices;
-    indices.push_back(IndexEntry(BSON("a" << 1)));
-    std::set<size_t> expectedIndices = {0};
-    testRateIndices("{a: [1]}", "", nullptr, indices, "a", expectedIndices);
+    std::set<std::string> testPatterns = {
+        "{a: {$type: 'string'}}", "{a: {$type: 'object'}}", "{a: {$type: 'array'}}"};
+    for (const auto& pattern : testPatterns) {
+        testRateIndices(pattern.c_str(), "", &collator, indices, "a", expectedIndices);
+    }
 }
 
 }  // namespace
diff --git a/src/mongo/db/query/query_planner_collation_test.cpp b/src/mongo/db/query/query_planner_collation_test.cpp
index 4937ce0b24e..d078511058c 100644
--- a/src/mongo/db/query/query_planner_collation_test.cpp
+++ b/src/mongo/db/query/query_planner_collation_test.cpp
@@ -231,26 +231,26 @@ TEST_F(QueryPlannerTest, ElemMatchObjectResultsInCorrectComparisonKeyBounds) {
         "{'a.b': 1}, filter: null, bounds: {'a.b': [['oof','oofz',true,true]]}}}}}");
 }
 
-TEST_F(QueryPlannerTest, QueryForNestedObjectWithNonNullCollatorCantUseIndex) {
+TEST_F(QueryPlannerTest, QueryForNestedObjectWithMatchingCollatorCanUseIndex) {
     CollatorInterfaceMock collator(CollatorInterfaceMock::MockType::kReverseString);
     addIndex(fromjson("{a: 1}"), &collator);
 
     runQueryAsCommand(
         fromjson("{find: 'testns', filter: {a: {b: 1}}, collation: {locale: 'reverse'}}"));
 
-    assertNumSolutions(1U);
+    assertNumSolutions(2U);
     assertSolutionExists("{cscan: {dir: 1}}");
+    assertSolutionExists("{fetch: {node: {ixscan: {pattern: {a: 1}}}}}");
 }
 
-TEST_F(QueryPlannerTest, QueryForNestedObjectWithNullCollatorCanUseIndexWithCollator) {
+TEST_F(QueryPlannerTest, QueryForNestedObjectWithNonMatchingCollatorCantUseIndexWithCollator) {
     CollatorInterfaceMock indexCollator(CollatorInterfaceMock::MockType::kReverseString);
     addIndex(fromjson("{a: 1}"), &indexCollator);
 
     runQuery(fromjson("{a: {b: 1}}"));
 
-    assertNumSolutions(2U);
+    assertNumSolutions(1U);
     assertSolutionExists("{cscan: {dir: 1}}");
-    assertSolutionExists("{fetch: {filter: null, node: {ixscan: {pattern: {a: 1}}}}}");
 }
 
 TEST_F(QueryPlannerTest, CannotUseIndexWithNonMatchingCollatorForSort) {
@@ -343,21 +343,4 @@ TEST_F(QueryPlannerTest, CompoundIndexWithNonMatchingPrefixedCollatorDoesNotCaus
         "{node: {cscan: {dir: 1, filter : {a: 1, b: 2, c: {a: 1}}}}}}}}");
 }
 
-TEST_F(QueryPlannerTest, EqualityToArrayWithNonMatchingCollatorCausesInMemorySort) {
-    CollatorInterfaceMock indexCollator(CollatorInterfaceMock::MockType::kReverseString);
-    addIndex(fromjson("{a: 1}"), &indexCollator);
-
-    runQueryAsCommand(
-        fromjson("{find: 'testns', filter: {a: [1]},"
-                 "sort: {a: 1}}"));
-
-    assertNumSolutions(2U);
-    assertSolutionExists(
-        "{sort: {pattern: {a: 1}, limit: 0, node: {sortKeyGen:"
-        "{node: {fetch: {node : {ixscan: {pattern: {a: 1}}}}}}}}}");
-    assertSolutionExists(
-        "{sort: {pattern: {a: 1}, limit: 0, node: {sortKeyGen:"
-        "{node: {cscan: {dir: 1, filter: {a: [1]}}}}}}}");
-}
-
 }  // namespace
diff --git a/src/mongo/db/query/query_planner_partialidx_test.cpp b/src/mongo/db/query/query_planner_partialidx_test.cpp
index 476e09041e3..39a025079d0 100644
--- a/src/mongo/db/query/query_planner_partialidx_test.cpp
+++ b/src/mongo/db/query/query_planner_partialidx_test.cpp
@@ -464,21 +464,6 @@ TEST_F(QueryPlannerTest, PartialIndexStringComparisonMatchingCollators) {
     assertNumSolutions(0U);
 }
 
-TEST_F(QueryPlannerTest, PartialIndexFilterStringComparisonNonMatchingCollators) {
-    params.options = QueryPlannerParams::NO_TABLE_SCAN;
-    BSONObj filterObj(fromjson("{a: {$lt: {b: 'abc'}}}"));
-    CollatorInterfaceMock collator(CollatorInterfaceMock::MockType::kReverseString);
-    std::unique_ptr<MatchExpression> filterExpr = parseMatchExpression(filterObj, &collator);
-    addIndex(fromjson("{a: 1}"), filterExpr.get(), &collator);
-
-    runQuery(fromjson("{a: {b: 1}}"));
-    assertNumSolutions(1U);
-    assertSolutionExists(
-        "{fetch: {filter: null, node: {ixscan: "
-        "{filter: null, pattern: {a: 1}, "
-        "bounds: {a: [[{b: 1}, {b: 1}, true, true]]}}}}}");
-}
-
 TEST_F(QueryPlannerTest, PartialIndexNoStringComparisonNonMatchingCollators) {
     params.options = QueryPlannerParams::NO_TABLE_SCAN;
     BSONObj filterObj(fromjson("{a: {$gt: 0}}"));
diff --git a/src/mongo/db/query/query_solution.cpp b/src/mongo/db/query/query_solution.cpp
index df7af9c36d5..51ff8aea132 100644
--- a/src/mongo/db/query/query_solution.cpp
+++ b/src/mongo/db/query/query_solution.cpp
@@ -33,6 +33,7 @@
 #include "mongo/bson/bsontypes.h"
 #include "mongo/db/index_names.h"
 #include "mongo/db/matcher/expression_geo.h"
+#include "mongo/db/query/collation/collation_index_key.h"
 #include "mongo/db/query/index_bounds_builder.h"
 #include "mongo/db/query/planner_analysis.h"
 #include "mongo/db/query/query_planner_common.h"
@@ -41,13 +42,6 @@ namespace mongo {
 
 namespace {
 
-// This function returns true if the BSONElement el can contain a string.  That is, el.type() is in
-// String, Array, or Object.
-inline bool elementCanContainString(const BSONElement& el) {
-    BSONType type = el.type();
-    return type == BSONType::String || type == BSONType::Array || type == BSONType::Object;
-}
-
 // Create an ordred interval list which represents the bounds for all BSON elements of type String,
 // Object, or Array.
 OrderedIntervalList buildStringBoundsOil(const std::string& keyName) {
@@ -593,7 +587,7 @@ std::set<StringData> IndexScanNode::getFieldsWithStringBounds(const IndexBounds&
         while (keyPatternIterator.more() && startKeyIterator.more() && endKeyIterator.more()) {
             BSONElement startKey = startKeyIterator.next();
             BSONElement endKey = endKeyIterator.next();
-            if (startKey != endKey || elementCanContainString(startKey)) {
+            if (startKey != endKey || CollationIndexKey::isCollatableType(startKey.type())) {
                 if (!rangeCanContainString(
                         startKey, endKey, (startKeyIterator.more() || bounds.endKeyInclusive))) {
                     // If the first non-point range cannot contain strings, we don't need to