diff options
author | Ruoxin Xu <ruoxin.xu@mongodb.com> | 2023-03-22 09:22:58 +0000 |
---|---|---|
committer | Evergreen Agent <no-reply@evergreen.mongodb.com> | 2023-03-22 09:57:10 +0000 |
commit | 317650c717dcc6d720259bcc21ae65012f1d1da4 (patch) | |
tree | 8eed3d2e109cb74fdc44ed0b34863b713ce3bcdd | |
parent | e20d579e14e8388124e97ce17aab5671aeb7286a (diff) | |
download | mongo-317650c717dcc6d720259bcc21ae65012f1d1da4.tar.gz |
SERVER-74344 Ban use of sparse indexes on internal comparison expression unless explicitly hinted
-rw-r--r-- | jstests/core/index/sparse_index_internal_expr.js | 70 | ||||
-rw-r--r-- | jstests/core/query/expr/expr_index_use.js | 5 | ||||
-rw-r--r-- | src/mongo/db/query/collection_query_info.cpp | 4 | ||||
-rw-r--r-- | src/mongo/db/query/get_executor.cpp | 13 | ||||
-rw-r--r-- | src/mongo/db/query/indexability.h | 20 | ||||
-rw-r--r-- | src/mongo/db/query/planner_ixselect.cpp | 38 | ||||
-rw-r--r-- | src/mongo/db/query/planner_ixselect.h | 27 | ||||
-rw-r--r-- | src/mongo/db/query/planner_ixselect_test.cpp | 114 | ||||
-rw-r--r-- | src/mongo/db/query/query_planner.cpp | 14 | ||||
-rw-r--r-- | src/mongo/db/query/query_planner_index_test.cpp | 14 | ||||
-rw-r--r-- | src/mongo/db/query/query_planner_wildcard_index_test.cpp | 15 |
11 files changed, 235 insertions, 99 deletions
diff --git a/jstests/core/index/sparse_index_internal_expr.js b/jstests/core/index/sparse_index_internal_expr.js new file mode 100644 index 00000000000..9ac47a5c100 --- /dev/null +++ b/jstests/core/index/sparse_index_internal_expr.js @@ -0,0 +1,70 @@ +/* + * Tests that a sparse index cannot be used to answer a $expr query unless the sparse index is + * explicitly hinted. If a sparse index is hinted to answer a $expr query, incomplete results could + * be returned. + * + * @tags: [ + * multiversion_incompatible, + * ] + */ + +(function() { +"use strict"; + +load("jstests/libs/analyze_plan.js"); + +const coll = db.sparse_index_internal_expr; +coll.drop(); + +coll.insert({a: 1}); + +const exprQuery = { + $expr: {$lt: ["$missing", "r"]} +}; + +// Run a query with $expr on a missing field. This query will use a COLLSCAN plan and return +// document '{a: 1}' because $expr expression does not apply type bracketing, specifically, the +// missing field is evaluated to 'null'. The expression returns "true" because 'null' < "r". +let res = coll.find(exprQuery, {_id: 0}).toArray(); + +assert.eq(res.length, 1); +assert.docEq(res[0], {a: 1}); + +// Tests that a non-sparse index {missing: 1} can be used to answer the $expr query. +assert.commandWorked(coll.createIndex({"missing": 1})); + +// Explain the query, and determine whether an indexed solution is available. +let ixScans = getPlanStages(getWinningPlan(coll.find(exprQuery).explain().queryPlanner), "IXSCAN"); + +// Verify that the winning plan uses the $** index with the expected bounds. +assert.gt(ixScans.length, 0, ixScans); +assert.eq("missing_1", ixScans[0].indexName, ixScans); + +// Run the same query. A complete result will be returned. +res = coll.find(exprQuery, {_id: 0}).toArray(); +assert.eq(res.length, 1); +assert.docEq(res[0], {a: 1}); + +// Drop the non-sparse index and create a sparse index with the same key pattern. +assert.commandWorked(coll.dropIndex("missing_1")); +assert.commandWorked(coll.createIndex({'missing': 1}, {'sparse': true})); + +// Run the same query to test that a COLLSCAN plan is used rather than an indexed plan. +const collScans = + getPlanStages(getWinningPlan(coll.find(exprQuery).explain().queryPlanner), "COLLSCAN"); + +// Verify that the winning plan uses the $** index with the expected bounds. +assert.gt(collScans.length, 0, collScans); + +// Test that a sparse index can be hinted to answer $expr query but incomplete results in returned, +// because the document is not indexed by the sparse index. +res = coll.find(exprQuery, {_id: 0}).hint("missing_1").toArray(); +assert.eq(res.length, 0); + +ixScans = getPlanStages( + getWinningPlan(coll.find(exprQuery).hint("missing_1").explain().queryPlanner), "IXSCAN"); + +assert.gt(ixScans.length, 0, ixScans); +assert.eq("missing_1", ixScans[0].indexName, ixScans); +assert.eq(true, ixScans[0].isSparse, ixScans); +}()); diff --git a/jstests/core/query/expr/expr_index_use.js b/jstests/core/query/expr/expr_index_use.js index 2eadaf645be..f7a04aed482 100644 --- a/jstests/core/query/expr/expr_index_use.js +++ b/jstests/core/query/expr/expr_index_use.js @@ -277,11 +277,6 @@ confirmExpectedExprExecution({$eq: ["$w", NaN]}, {nReturned: 1, expectedIndex: { confirmExpectedExprExecution({$eq: ["$w", undefined]}, {nReturned: 16}); confirmExpectedExprExecution({$eq: ["$w", "$$REMOVE"]}, {nReturned: 16}); -// Test that equality to null queries can use a sparse index. -assert.commandWorked(coll.dropIndex({w: "hashed"})); -assert.commandWorked(coll.createIndex({w: 1}, {sparse: true})); -confirmExpectedExprExecution({$eq: ["$w", null]}, {nReturned: 1, expectedIndex: {w: 1}}); - // Equality match against text index prefix is expected to fail. Equality predicates are // required against the prefix fields of a text index, but currently $eq inside $expr does not // qualify. diff --git a/src/mongo/db/query/collection_query_info.cpp b/src/mongo/db/query/collection_query_info.cpp index d13fa755beb..a676438250e 100644 --- a/src/mongo/db/query/collection_query_info.cpp +++ b/src/mongo/db/query/collection_query_info.cpp @@ -191,10 +191,10 @@ void CollectionQueryInfo::computeUpdateIndexData(const IndexCatalogEntry* entry, // handle partial indexes const MatchExpression* filter = entry->getFilterExpression(); if (filter) { - stdx::unordered_set<std::string> paths; + RelevantFieldIndexMap paths; QueryPlannerIXSelect::getFields(filter, &paths); for (auto it = paths.begin(); it != paths.end(); ++it) { - outData->addPath(FieldRef(*it)); + outData->addPath(FieldRef(it->first)); } } } diff --git a/src/mongo/db/query/get_executor.cpp b/src/mongo/db/query/get_executor.cpp index c12b1a15a13..85c35d663b7 100644 --- a/src/mongo/db/query/get_executor.cpp +++ b/src/mongo/db/query/get_executor.cpp @@ -229,10 +229,15 @@ IndexEntry indexEntryFromIndexCatalogEntry(OperationContext* opCtx, MultikeyMetadataAccessStats mkAccessStats; if (canonicalQuery) { - stdx::unordered_set<std::string> fields; - QueryPlannerIXSelect::getFields(canonicalQuery->root(), &fields); - const auto projectedFields = projection_executor_utils::applyProjectionToFields( - wildcardProjection->exec(), fields); + RelevantFieldIndexMap fieldIndexProps; + QueryPlannerIXSelect::getFields(canonicalQuery->root(), &fieldIndexProps); + stdx::unordered_set<std::string> projectedFields; + for (auto&& [fieldName, _] : fieldIndexProps) { + if (projection_executor_utils::applyProjectionToOneField( + wildcardProjection->exec(), fieldName)) { + projectedFields.insert(fieldName); + } + } multikeyPathSet = getWildcardMultikeyPathSet(wam, opCtx, projectedFields, &mkAccessStats); diff --git a/src/mongo/db/query/indexability.h b/src/mongo/db/query/indexability.h index 7e543da4a2d..48b9e0d91b4 100644 --- a/src/mongo/db/query/indexability.h +++ b/src/mongo/db/query/indexability.h @@ -55,6 +55,26 @@ public: } /** + * Type bracketing does not apply to internal Expressions. This could cause the use of a sparse + * index return incomplete results. For example, a query {$expr: {$lt: ["$missing", "r"]}} would + * expect a document like, {a: 1}, with field "missing" missing be returned. However, a sparse + * index, {missing: 1} does not index the document. Therefore, we should ban use of any sparse + * index on following expression types. + */ + static bool nodeSupportedBySparseIndex(const MatchExpression* me) { + switch (me->matchType()) { + case MatchExpression::INTERNAL_EXPR_EQ: + case MatchExpression::INTERNAL_EXPR_GT: + case MatchExpression::INTERNAL_EXPR_GTE: + case MatchExpression::INTERNAL_EXPR_LT: + case MatchExpression::INTERNAL_EXPR_LTE: + return false; + default: + return true; + } + } + + /** * This array operator doesn't have any children with fields and can use an index. * * Example: a: {$elemMatch: {$gte: 1, $lte: 1}}. diff --git a/src/mongo/db/query/planner_ixselect.cpp b/src/mongo/db/query/planner_ixselect.cpp index ba9404c9522..c9795f15198 100644 --- a/src/mongo/db/query/planner_ixselect.cpp +++ b/src/mongo/db/query/planner_ixselect.cpp @@ -249,7 +249,7 @@ static bool boundsGeneratingNodeContainsComparisonToType(MatchExpression* node, // static void QueryPlannerIXSelect::getFields(const MatchExpression* node, string prefix, - stdx::unordered_set<string>* out) { + RelevantFieldIndexMap* out) { // Do not traverse tree beyond a NOR negation node MatchExpression::MatchType exprtype = node->matchType(); if (exprtype == MatchExpression::NOR) { @@ -258,7 +258,8 @@ void QueryPlannerIXSelect::getFields(const MatchExpression* node, // Leaf nodes with a path and some array operators. if (Indexability::nodeCanUseIndexOnOwnField(node)) { - out->insert(prefix + node->path().toString()); + bool supportSparse = Indexability::nodeSupportedBySparseIndex(node); + (*out)[prefix + node->path().toString()] = {supportSparse}; } else if (Indexability::arrayUsesIndexOnChildren(node) && !node->path().empty()) { // If the array uses an index on its children, it's something like // {foo : {$elemMatch: {bar: 1}}}, in which case the predicate is really over foo.bar. @@ -275,8 +276,7 @@ void QueryPlannerIXSelect::getFields(const MatchExpression* node, } } -void QueryPlannerIXSelect::getFields(const MatchExpression* node, - stdx::unordered_set<string>* out) { +void QueryPlannerIXSelect::getFields(const MatchExpression* node, RelevantFieldIndexMap* out) { getFields(node, "", out); } @@ -316,26 +316,40 @@ std::vector<IndexEntry> QueryPlannerIXSelect::findIndexesByHint( // static std::vector<IndexEntry> QueryPlannerIXSelect::findRelevantIndices( - const stdx::unordered_set<std::string>& fields, const std::vector<IndexEntry>& allIndices) { + const RelevantFieldIndexMap& fields, const std::vector<IndexEntry>& allIndices) { std::vector<IndexEntry> out; - for (auto&& entry : allIndices) { - BSONObjIterator it(entry.keyPattern); + for (auto&& index : allIndices) { + BSONObjIterator it(index.keyPattern); BSONElement elt = it.next(); - if (fields.end() != fields.find(elt.fieldName())) { - out.push_back(entry); + const std::string fieldName = elt.fieldNameStringData().toString(); + + // If the index is non-sparse we can use the field regardless its sparsity, otherwise we + // should find the field that can be answered by a sparse index. + if (fields.contains(fieldName) && + (!index.sparse || fields.find(fieldName)->second.isSparse)) { + out.push_back(index); } } return out; } -std::vector<IndexEntry> QueryPlannerIXSelect::expandIndexes( - const stdx::unordered_set<std::string>& fields, std::vector<IndexEntry> relevantIndices) { +std::vector<IndexEntry> QueryPlannerIXSelect::expandIndexes(const RelevantFieldIndexMap& fields, + std::vector<IndexEntry> relevantIndices, + bool indexHinted) { std::vector<IndexEntry> out; + // Filter out fields that cannot be answered by any sparse index. We know wildcard indexes are + // sparse, so we don't want to expand the wildcard index based on such fields. + stdx::unordered_set<std::string> sparseIncompatibleFields; + for (auto&& [fieldName, idxProperty] : fields) { + if (idxProperty.isSparse || indexHinted) { + sparseIncompatibleFields.insert(fieldName); + } + } for (auto&& entry : relevantIndices) { if (entry.type == IndexType::INDEX_WILDCARD) { - wcp::expandWildcardIndexEntry(entry, fields, &out); + wcp::expandWildcardIndexEntry(entry, sparseIncompatibleFields, &out); } else { out.push_back(std::move(entry)); } diff --git a/src/mongo/db/query/planner_ixselect.h b/src/mongo/db/query/planner_ixselect.h index 0ef2d480953..15f1e135d5e 100644 --- a/src/mongo/db/query/planner_ixselect.h +++ b/src/mongo/db/query/planner_ixselect.h @@ -38,16 +38,24 @@ namespace mongo { class CollatorInterface; +struct IndexProperties { + bool isSparse = false; // 'true' if a sparse index can answer the field. +}; + +// A relevant field to index requirement map. +using RelevantFieldIndexMap = stdx::unordered_map<std::string, IndexProperties>; + /** * Methods for determining what fields and predicates can use indices. */ class QueryPlannerIXSelect { public: /** - * Return all the fields in the tree rooted at 'node' that we can use an index on - * in order to answer the query. + * Return all the fields in the tree rooted at 'node' that we can use an index to answer the + * query. The output, 'RelevantFieldIndexMap', contains the requirements of the index that can + * answer the field. e.g. Some fields can be supported only by a non-sparse index. */ - static void getFields(const MatchExpression* node, stdx::unordered_set<std::string>* out); + static void getFields(const MatchExpression* node, RelevantFieldIndexMap* out); /** * Similar to other getFields() method, but with 'prefix' argument which is a path prefix to be @@ -57,7 +65,7 @@ public: */ static void getFields(const MatchExpression* node, std::string prefix, - stdx::unordered_set<std::string>* out); + RelevantFieldIndexMap* out); /** * Finds all indices that correspond to the hinted index. Matches the index both by name and by @@ -70,8 +78,8 @@ public: * Finds all indices prefixed by fields we have predicates over. Only these indices are * useful in answering the query. */ - static std::vector<IndexEntry> findRelevantIndices( - const stdx::unordered_set<std::string>& fields, const std::vector<IndexEntry>& allIndices); + static std::vector<IndexEntry> findRelevantIndices(const RelevantFieldIndexMap& fields, + const std::vector<IndexEntry>& allIndices); /** * Determine how useful all of our relevant 'indices' are to all predicates in the subtree @@ -130,9 +138,12 @@ public: /** * Given a list of IndexEntries and fields used by a query's match expression, return a list * "expanded" indexes (where the $** indexes in the given list have been expanded). + * 'hintedIndexBson' indicates that the indexes in 'relevantIndices' are the results of the + * user's hint. */ - static std::vector<IndexEntry> expandIndexes(const stdx::unordered_set<std::string>& fields, - std::vector<IndexEntry> relevantIndices); + static std::vector<IndexEntry> expandIndexes(const RelevantFieldIndexMap& fields, + std::vector<IndexEntry> relevantIndices, + bool hintedIndexBson = false); /** * Check if this match expression is a leaf and is supported by a wildcard index. diff --git a/src/mongo/db/query/planner_ixselect_test.cpp b/src/mongo/db/query/planner_ixselect_test.cpp index 93c4f12c821..1df4d714e67 100644 --- a/src/mongo/db/query/planner_ixselect_test.cpp +++ b/src/mongo/db/query/planner_ixselect_test.cpp @@ -65,6 +65,20 @@ unique_ptr<MatchExpression> parseMatchExpression(const BSONObj& obj) { return std::move(status.getValue()); } +using FieldIter = RelevantFieldIndexMap::iterator; +string toString(FieldIter begin, FieldIter end) { + str::stream ss; + ss << "["; + for (FieldIter i = begin; i != end; i++) { + if (i != begin) { + ss << " "; + } + ss << i->first; + } + ss << "]"; + return ss; +} + /** * Utility function to join elements in iterator range with comma */ @@ -88,10 +102,13 @@ string toString(Iter begin, Iter end) { * to QueryPlannerIXSelect::getFields() * Results are compared with expected fields (parsed from expectedFieldsStr) */ -void testGetFields(const char* query, const char* prefix, const char* expectedFieldsStr) { +void testGetFields(const char* query, + const char* prefix, + const char* expectedFieldsStr, + bool sparseSupported = true) { BSONObj obj = fromjson(query); unique_ptr<MatchExpression> expr(parseMatchExpression(obj)); - stdx::unordered_set<string> fields; + RelevantFieldIndexMap fields; QueryPlannerIXSelect::getFields(expr.get(), prefix, &fields); // Verify results @@ -99,7 +116,7 @@ void testGetFields(const char* query, const char* prefix, const char* expectedFi vector<string> expectedFields = StringSplitter::split(expectedFieldsStr, ","); for (vector<string>::const_iterator i = expectedFields.begin(); i != expectedFields.end(); i++) { - if (fields.find(*i) == fields.end()) { + if (fields[*i].isSparse != sparseSupported) { str::stream ss; ss << "getFields(query=" << query << ", prefix=" << prefix << "): unable to find " << *i << " in result: " << toString(fields.begin(), fields.end()); @@ -159,6 +176,12 @@ TEST(QueryPlannerIXSelectTest, GetFieldsArrayNegation) { testGetFields("{a: {$all: [{$elemMatch: {b: {$ne: 1}}}]}}", "", "a.b"); } +TEST(QueryPlannerIXSelectTest, GetFieldsInternalExpr) { + testGetFields("{$expr: {$lt: ['$a', 'r']}}", "", "", false /* sparse supported */); + testGetFields("{$expr: {$eq: ['$a', null]}}", "", "", false /* sparse supported */); + testGetFields("{$expr: {$eq: ['$a', 1]}}", "", "", false /* sparse supported */); +} + /** * Performs a pre-order traversal of expression tree. Validates * that all tagged nodes contain an instance of RelevantTag. @@ -1153,25 +1176,6 @@ TEST(QueryPlannerIXSelectTest, InternalExprEqCanUseTextIndexSuffix) { "{a: {$_internalExprEq: 1}}", "", kSimpleCollator, indices, "a", expectedIndices); } -TEST(QueryPlannerIXSelectTest, InternalExprEqCanUseSparseIndexWithComparisonToNull) { - auto entry = buildSimpleIndexEntry(BSON("a" << 1)); - entry.sparse = true; - std::vector<IndexEntry> indices; - indices.push_back(entry); - std::set<size_t> expectedIndices = {0}; - testRateIndices( - "{a: {$_internalExprEq: null}}", "", kSimpleCollator, indices, "a", expectedIndices); -} - -TEST(QueryPlannerIXSelectTest, InternalExprEqCanUseSparseIndexWithComparisonToNonNull) { - auto entry = buildSimpleIndexEntry(BSON("a" << 1)); - entry.sparse = true; - std::vector<IndexEntry> indices; - indices.push_back(entry); - std::set<size_t> expectedIndices = {0}; - testRateIndices( - "{a: {$_internalExprEq: 1}}", "", kSimpleCollator, indices, "a", expectedIndices); -} TEST(QueryPlannerIXSelectTest, NotEqualsNullCanUseIndex) { auto entry = buildSimpleIndexEntry(BSON("a" << 1)); std::set<size_t> expectedIndices = {0}; @@ -1357,18 +1361,17 @@ TEST(QueryPlannerIXSelectTest, ExpandWildcardIndices) { const auto indexEntry = makeIndexEntry(BSON("$**" << 1), {}); // Case where no fields are specified. - std::vector<IndexEntry> result = - QueryPlannerIXSelect::expandIndexes(stdx::unordered_set<string>(), {indexEntry.first}); + std::vector<IndexEntry> result = QueryPlannerIXSelect::expandIndexes({}, {indexEntry.first}); ASSERT_TRUE(result.empty()); - stdx::unordered_set<string> fields = {"fieldA", "fieldB"}; + RelevantFieldIndexMap fields = {{"fieldA", {true}}, {"fieldB", {true}}}; result = QueryPlannerIXSelect::expandIndexes(fields, {indexEntry.first}); std::vector<BSONObj> expectedKeyPatterns = {BSON("fieldA" << 1), BSON("fieldB" << 1)}; ASSERT_TRUE(indexEntryKeyPatternsMatch(&expectedKeyPatterns, &result)); const auto wildcardIndexWithSubpath = makeIndexEntry(BSON("a.b.$**" << 1), {}); - fields = {"a.b", "a.b.c", "a.d"}; + fields = {{"a.b", {true}}, {"a.b.c", {true}}, {"a.d", {true}}}; result = QueryPlannerIXSelect::expandIndexes(fields, {wildcardIndexWithSubpath.first}); expectedKeyPatterns = {BSON("a.b" << 1), BSON("a.b.c" << 1)}; ASSERT_TRUE(indexEntryKeyPatternsMatch(&expectedKeyPatterns, &result)); @@ -1380,7 +1383,8 @@ TEST(QueryPlannerIXSelectTest, ExpandWildcardIndicesInPresenceOfOtherIndices) { auto bIndexEntry = makeIndexEntry(BSON("fieldB" << 1), {}); auto abIndexEntry = makeIndexEntry(BSON("fieldA" << 1 << "fieldB" << 1), {}); - const stdx::unordered_set<string> fields = {"fieldA", "fieldB", "fieldC"}; + const RelevantFieldIndexMap fields = { + {"fieldA", {true}}, {"fieldB", {true}}, {"fieldC", {true}}}; std::vector<BSONObj> expectedKeyPatterns = { BSON("fieldA" << 1), BSON("fieldA" << 1), BSON("fieldB" << 1), BSON("fieldC" << 1)}; @@ -1418,7 +1422,7 @@ TEST(QueryPlannerIXSelectTest, ExpandWildcardIndicesInPresenceOfOtherIndices) { TEST(QueryPlannerIXSelectTest, ExpandedIndexEntriesAreCorrectlyMarkedAsMultikeyOrNonMultikey) { auto wildcardIndexEntry = makeIndexEntry(BSON("$**" << 1), {}, {FieldRef{"a"}}); - const stdx::unordered_set<string> fields = {"a.b", "c.d"}; + RelevantFieldIndexMap fields = {{"a.b", {true}}, {"c.d", {true}}}; std::vector<BSONObj> expectedKeyPatterns = {BSON("a.b" << 1), BSON("c.d" << 1)}; auto result = QueryPlannerIXSelect::expandIndexes(fields, {wildcardIndexEntry.first}); @@ -1442,7 +1446,7 @@ TEST(QueryPlannerIXSelectTest, ExpandedIndexEntriesAreCorrectlyMarkedAsMultikeyO TEST(QueryPlannerIXSelectTest, WildcardIndexExpansionExcludesIdField) { const auto indexEntry = makeIndexEntry(BSON("$**" << 1), {}); - stdx::unordered_set<string> fields = {"_id", "abc", "def"}; + RelevantFieldIndexMap fields = {{"_id", {true}}, {"abc", {true}}, {"def", {true}}}; std::vector<IndexEntry> result = QueryPlannerIXSelect::expandIndexes(fields, {indexEntry.first}); @@ -1454,7 +1458,7 @@ TEST(QueryPlannerIXSelectTest, WildcardIndicesExpandedEntryHasCorrectProperties) auto wildcardIndexEntry = makeIndexEntry(BSON("$**" << 1), {}); wildcardIndexEntry.first.identifier = IndexEntry::Identifier("someIndex"); - stdx::unordered_set<string> fields = {"abc", "def"}; + RelevantFieldIndexMap fields = {{"abc", {true}}, {"def", {true}}}; std::vector<IndexEntry> result = QueryPlannerIXSelect::expandIndexes(fields, {wildcardIndexEntry.first}); @@ -1484,7 +1488,11 @@ TEST(QueryPlannerIXSelectTest, WildcardIndicesExpandedEntryHasCorrectProperties) TEST(QueryPlannerIXSelectTest, WildcardIndicesExcludeNonMatchingKeySubpath) { auto wildcardIndexEntry = makeIndexEntry(BSON("subpath.$**" << 1), {}); - stdx::unordered_set<string> fields = {"abc", "def", "subpath.abc", "subpath.def", "subpath"}; + RelevantFieldIndexMap fields = {{"abc", {true}}, + {"def", {true}}, + {"subpath.abc", {true}}, + {"subpath.def", {true}}, + {"subpath", {true}}}; std::vector<IndexEntry> result = QueryPlannerIXSelect::expandIndexes(fields, {wildcardIndexEntry.first}); @@ -1500,7 +1508,11 @@ TEST(QueryPlannerIXSelectTest, WildcardIndicesExcludeNonMatchingPathsWithInclusi {}, BSON("wildcardProjection" << BSON("abc" << 1 << "subpath.abc" << 1))); - stdx::unordered_set<string> fields = {"abc", "def", "subpath.abc", "subpath.def", "subpath"}; + RelevantFieldIndexMap fields = {{"abc", {true}}, + {"def", {true}}, + {"subpath.abc", {true}}, + {"subpath.def", {true}}, + {"subpath", {true}}}; std::vector<IndexEntry> result = QueryPlannerIXSelect::expandIndexes(fields, {wildcardIndexEntry.first}); @@ -1515,7 +1527,11 @@ TEST(QueryPlannerIXSelectTest, WildcardIndicesExcludeNonMatchingPathsWithExclusi {}, BSON("wildcardProjection" << BSON("abc" << 0 << "subpath.abc" << 0))); - stdx::unordered_set<string> fields = {"abc", "def", "subpath.abc", "subpath.def", "subpath"}; + RelevantFieldIndexMap fields = {{"abc", {true}}, + {"def", {true}}, + {"subpath.abc", {true}}, + {"subpath.def", {true}}, + {"subpath", {true}}}; std::vector<IndexEntry> result = QueryPlannerIXSelect::expandIndexes(fields, {wildcardIndexEntry.first}); @@ -1531,8 +1547,12 @@ TEST(QueryPlannerIXSelectTest, WildcardIndicesWithInclusionProjectionAllowIdExcl {}, BSON("wildcardProjection" << BSON("_id" << 0 << "abc" << 1 << "subpath.abc" << 1))); - stdx::unordered_set<string> fields = { - "_id", "abc", "def", "subpath.abc", "subpath.def", "subpath"}; + RelevantFieldIndexMap fields = {{"_id", {true}}, + {"abc", {true}}, + {"def", {true}}, + {"subpath.abc", {true}}, + {"subpath.def", {true}}, + {"subpath", {true}}}; std::vector<IndexEntry> result = QueryPlannerIXSelect::expandIndexes(fields, {wildcardIndexEntry.first}); @@ -1547,8 +1567,12 @@ TEST(QueryPlannerIXSelectTest, WildcardIndicesWithInclusionProjectionAllowIdIncl {}, BSON("wildcardProjection" << BSON("_id" << 1 << "abc" << 1 << "subpath.abc" << 1))); - stdx::unordered_set<string> fields = { - "_id", "abc", "def", "subpath.abc", "subpath.def", "subpath"}; + RelevantFieldIndexMap fields = {{"_id", {true}}, + {"abc", {true}}, + {"def", {true}}, + {"subpath.abc", {true}}, + {"subpath.def", {true}}, + {"subpath", {true}}}; std::vector<IndexEntry> result = QueryPlannerIXSelect::expandIndexes(fields, {wildcardIndexEntry.first}); @@ -1564,8 +1588,12 @@ TEST(QueryPlannerIXSelectTest, WildcardIndicesWithExclusionProjectionAllowIdIncl {}, BSON("wildcardProjection" << BSON("_id" << 1 << "abc" << 0 << "subpath.abc" << 0))); - stdx::unordered_set<string> fields = { - "_id", "abc", "def", "subpath.abc", "subpath.def", "subpath"}; + RelevantFieldIndexMap fields = {{"_id", {true}}, + {"abc", {true}}, + {"def", {true}}, + {"subpath.abc", {true}}, + {"subpath.def", {true}}, + {"subpath", {true}}}; std::vector<IndexEntry> result = QueryPlannerIXSelect::expandIndexes(fields, {wildcardIndexEntry.first}); @@ -1578,8 +1606,12 @@ TEST(QueryPlannerIXSelectTest, WildcardIndicesIncludeMatchingInternalNodes) { auto wildcardIndexEntry = makeIndexEntry( BSON("$**" << 1), {}, {}, BSON("wildcardProjection" << BSON("_id" << 1 << "subpath" << 1))); - stdx::unordered_set<string> fields = { - "_id", "abc", "def", "subpath.abc", "subpath.def", "subpath"}; + RelevantFieldIndexMap fields = {{"_id", {true}}, + {"abc", {true}}, + {"def", {true}}, + {"subpath.abc", {true}}, + {"subpath.def", {true}}, + {"subpath", {true}}}; std::vector<IndexEntry> result = QueryPlannerIXSelect::expandIndexes(fields, {wildcardIndexEntry.first}); diff --git a/src/mongo/db/query/query_planner.cpp b/src/mongo/db/query/query_planner.cpp index ecd7c67ea5d..877eadb4566 100644 --- a/src/mongo/db/query/query_planner.cpp +++ b/src/mongo/db/query/query_planner.cpp @@ -940,10 +940,11 @@ StatusWith<std::unique_ptr<QuerySolution>> QueryPlanner::planFromCache( "filter"_attr = redact(clone->debugString()), "cacheData"_attr = redact(winnerCacheData.toString())); - stdx::unordered_set<string> fields; + RelevantFieldIndexMap fields; QueryPlannerIXSelect::getFields(query.root(), &fields); + // We will not cache queries with 'hint'. std::vector<IndexEntry> expandedIndexes = - QueryPlannerIXSelect::expandIndexes(fields, params.indices); + QueryPlannerIXSelect::expandIndexes(fields, params.indices, false /* indexHinted */); // Map from index name to index number. map<IndexEntry::Identifier, size_t> indexMap; @@ -1114,7 +1115,7 @@ StatusWith<std::vector<std::unique_ptr<QuerySolution>>> QueryPlanner::plan( // Hints require us to only consider the hinted index. If index filters in the query settings // were used to override the allowed indices for planning, we should not use the hinted index // requested in the query. - boost::optional<BSONObj> hintedIndexBson; + boost::optional<BSONObj> hintedIndexBson = boost::none; if (!params.indexFiltersApplied) { if (auto hintObj = query.getFindCommandRequest().getHint(); !hintObj.isEmpty()) { hintedIndexBson = hintObj; @@ -1170,13 +1171,14 @@ StatusWith<std::vector<std::unique_ptr<QuerySolution>>> QueryPlanner::plan( } // Figure out what fields we care about. - stdx::unordered_set<string> fields; + RelevantFieldIndexMap fields; QueryPlannerIXSelect::getFields(query.root(), &fields); for (auto&& field : fields) { - LOGV2_DEBUG(20970, 5, "Predicate over field", "field"_attr = field); + LOGV2_DEBUG(20970, 5, "Predicate over field", "field"_attr = field.first); } - fullIndexList = QueryPlannerIXSelect::expandIndexes(fields, std::move(fullIndexList)); + fullIndexList = QueryPlannerIXSelect::expandIndexes( + fields, std::move(fullIndexList), hintedIndexBson != boost::none); std::vector<IndexEntry> relevantIndices; if (!hintedIndexEntry) { diff --git a/src/mongo/db/query/query_planner_index_test.cpp b/src/mongo/db/query/query_planner_index_test.cpp index df8ff117eaf..960a64c0189 100644 --- a/src/mongo/db/query/query_planner_index_test.cpp +++ b/src/mongo/db/query/query_planner_index_test.cpp @@ -205,26 +205,20 @@ TEST_F(QueryPlannerTest, SparseIndexForQuery) { "{filter: null, pattern: {a: 1}}}}}"); } -TEST_F(QueryPlannerTest, ExprEqCanUseSparseIndex) { +TEST_F(QueryPlannerTest, ExprEqCannotUseSparseIndex) { params.options &= ~QueryPlannerParams::INCLUDE_COLLSCAN; addIndex(fromjson("{a: 1}"), false, true); runQuery(fromjson("{a: {$_internalExprEq: 1}}")); - assertNumSolutions(1U); - assertSolutionExists( - "{fetch: {filter: null, node: {ixscan: " - "{filter: null, pattern: {a: 1}, bounds: {a: [[1,1,true,true]]}}}}}"); + assertHasOnlyCollscan(); } -TEST_F(QueryPlannerTest, ExprEqCanUseSparseIndexForEqualityToNull) { +TEST_F(QueryPlannerTest, ExprEqCannotUseSparseIndexForEqualityToNull) { params.options &= ~QueryPlannerParams::INCLUDE_COLLSCAN; addIndex(fromjson("{a: 1}"), false, true); runQuery(fromjson("{a: {$_internalExprEq: null}}")); - assertNumSolutions(1U); - assertSolutionExists( - "{fetch: {filter: {a: {$_internalExprEq: null}}, node: {ixscan: {filter: null, pattern: " - "{a: 1}, bounds: {a: [[undefined,undefined,true,true], [null,null,true,true]]}}}}}"); + assertHasOnlyCollscan(); } TEST_F(QueryPlannerTest, NegationCannotUseSparseIndex) { diff --git a/src/mongo/db/query/query_planner_wildcard_index_test.cpp b/src/mongo/db/query/query_planner_wildcard_index_test.cpp index 47532c4685d..8d4af877eb3 100644 --- a/src/mongo/db/query/query_planner_wildcard_index_test.cpp +++ b/src/mongo/db/query/query_planner_wildcard_index_test.cpp @@ -420,25 +420,18 @@ TEST_F(QueryPlannerWildcardTest, EqualityIndexScanOverNestedField) { "bounds: {'$_path': [['a.b','a.b',true,true]], 'a.b': [[5,5,true,true]]}}}}}"); } -TEST_F(QueryPlannerWildcardTest, ExprEqCanUseIndex) { +TEST_F(QueryPlannerWildcardTest, ExprEqCannotUseIndex) { addWildcardIndex(BSON("$**" << 1)); runQuery(fromjson("{a: {$_internalExprEq: 1}}")); - assertNumSolutions(1U); - assertSolutionExists( - "{fetch: {filter: null, node: {ixscan: {pattern: {'$_path': 1, a: 1}," - "bounds: {'$_path': [['a','a',true,true]], a: [[1,1,true,true]]}}}}}"); + assertHasOnlyCollscan(); } -TEST_F(QueryPlannerWildcardTest, ExprEqCanUseSparseIndexForEqualityToNull) { +TEST_F(QueryPlannerWildcardTest, ExprEqCannotUseSparseIndexForEqualityToNull) { addWildcardIndex(BSON("$**" << 1)); runQuery(fromjson("{a: {$_internalExprEq: null}}")); - assertNumSolutions(1U); - assertSolutionExists( - "{fetch: {filter: {a: {$_internalExprEq: null}}, node: {ixscan: {pattern: {'$_path': 1, a: " - "1}, bounds: {'$_path': [['a','a',true,true]], a: [[undefined,undefined,true,true], " - "[null,null,true,true]]}}}}}"); + assertHasOnlyCollscan(); } TEST_F(QueryPlannerWildcardTest, PrefixRegex) { |