diff options
author | David Hatch <david.hatch@mongodb.com> | 2016-06-14 11:51:33 -0400 |
---|---|---|
committer | David Hatch <david.hatch@mongodb.com> | 2016-06-22 15:42:59 -0400 |
commit | 40f20eca105a5e06a72df583ac654f946e9b058e (patch) | |
tree | 0ca485f5a24297236b9455a6698e7135802e08cb /src | |
parent | f517b05141a3f554d2fe51838ed33bc98cb8c5f2 (diff) | |
download | mongo-40f20eca105a5e06a72df583ac654f946e9b058e.tar.gz |
SERVER-23172 Allow use of indices for collation-aware queries that match nested objects or arrays.
Diffstat (limited to 'src')
-rw-r--r-- | src/mongo/db/index/btree_key_generator_test.cpp | 8 | ||||
-rw-r--r-- | src/mongo/db/index/hash_key_generator_test.cpp | 6 | ||||
-rw-r--r-- | src/mongo/db/index/s2_key_generator_test.cpp | 5 | ||||
-rw-r--r-- | src/mongo/db/matcher/expression_algo.cpp | 10 | ||||
-rw-r--r-- | src/mongo/db/matcher/expression_algo_test.cpp | 3 | ||||
-rw-r--r-- | src/mongo/db/query/collation/collation_index_key.cpp | 81 | ||||
-rw-r--r-- | src/mongo/db/query/collation/collation_index_key.h | 28 | ||||
-rw-r--r-- | src/mongo/db/query/collation/collation_index_key_test.cpp | 63 | ||||
-rw-r--r-- | src/mongo/db/query/planner_ixselect.cpp | 13 | ||||
-rw-r--r-- | src/mongo/db/query/planner_ixselect_test.cpp | 158 | ||||
-rw-r--r-- | src/mongo/db/query/query_planner_collation_test.cpp | 27 | ||||
-rw-r--r-- | src/mongo/db/query/query_planner_partialidx_test.cpp | 15 | ||||
-rw-r--r-- | src/mongo/db/query/query_solution.cpp | 10 |
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 |