summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--jstests/core/agg_hint.js150
-rw-r--r--jstests/core/profile_agg.js13
-rw-r--r--jstests/core/profile_getmore.js5
-rw-r--r--jstests/core/views/views_count.js7
-rw-r--r--jstests/libs/analyze_plan.js50
-rw-r--r--jstests/noPassthrough/currentop_query.js7
-rw-r--r--src/mongo/db/commands/pipeline_command.cpp2
-rw-r--r--src/mongo/db/pipeline/aggregation_request.cpp17
-rw-r--r--src/mongo/db/pipeline/aggregation_request.h14
-rw-r--r--src/mongo/db/pipeline/aggregation_request_test.cpp36
-rw-r--r--src/mongo/db/pipeline/pipeline_d.cpp25
-rw-r--r--src/mongo/db/pipeline/pipeline_d.h5
-rw-r--r--src/mongo/db/query/count_request.cpp10
-rw-r--r--src/mongo/db/query/count_request_test.cpp17
-rw-r--r--src/mongo/db/query/query_planner.cpp11
-rw-r--r--src/mongo/db/query/query_planner_test.cpp18
-rw-r--r--src/mongo/db/query/query_request.cpp7
-rw-r--r--src/mongo/db/query/query_request_test.cpp17
-rw-r--r--src/mongo/db/views/resolved_view.cpp4
19 files changed, 375 insertions, 40 deletions
diff --git a/jstests/core/agg_hint.js b/jstests/core/agg_hint.js
new file mode 100644
index 00000000000..a73e2593936
--- /dev/null
+++ b/jstests/core/agg_hint.js
@@ -0,0 +1,150 @@
+// Confirms correct behavior for hinted aggregation execution. This includes tests for scenarios
+// where agg execution differs from query. It also includes confirmation that hint works for find
+// command against views, which is converted to a hinted aggregation on execution.
+
+(function() {
+ "use strict";
+
+ load("jstests/libs/analyze_plan.js"); // For getAggPlanStage.
+
+ const testDB = db.getSiblingDB("agg_hint");
+ assert.commandWorked(testDB.dropDatabase());
+ const coll = testDB.getCollection("test");
+ const view = testDB.getCollection("view");
+ const NO_HINT = null;
+
+ function confirmWinningPlanUsesExpectedIndex(explainResult, expectedKeyPattern, stageName) {
+ const planStage = getAggPlanStage(explainResult, stageName);
+ assert.neq(null, planStage);
+
+ assert.eq(planStage.keyPattern, expectedKeyPattern, tojson(planStage));
+ }
+
+ // Runs explain on 'command', with the hint specified by 'hintKeyPattern' when not null.
+ // Confirms that the winning query plan uses the index specified by 'expectedKeyPattern'.
+ function confirmCommandUsesIndex(
+ command, hintKeyPattern, expectedKeyPattern, stageName = "IXSCAN") {
+ if (hintKeyPattern) {
+ command["hint"] = hintKeyPattern;
+ }
+ const res =
+ assert.commandWorked(testDB.runCommand({explain: command, verbosity: "queryPlanner"}));
+ confirmWinningPlanUsesExpectedIndex(res, expectedKeyPattern, stageName);
+ }
+
+ // Runs explain on an aggregation with a pipeline specified by 'aggPipeline' and a hint
+ // specified by 'hintKeyPattern' if not null. Confirms that the winning query plan uses the
+ // index specified by 'expectedKeyPattern'.
+ //
+ // This method exists because the explain command does not support the aggregation command.
+ function confirmAggUsesIndex(
+ collName, aggPipeline, hintKeyPattern, expectedKeyPattern, stageName = "IXSCAN") {
+ let options = {};
+ if (hintKeyPattern) {
+ options = {hint: hintKeyPattern};
+ }
+ const res = assert.commandWorked(
+ testDB.getCollection(collName).explain().aggregate(aggPipeline, options));
+ confirmWinningPlanUsesExpectedIndex(res, expectedKeyPattern, stageName);
+ }
+
+ // Specify hint as a string, representing index name.
+ assert.commandWorked(coll.createIndex({x: 1}));
+ for (let i = 0; i < 5; ++i) {
+ assert.writeOK(coll.insert({x: i}));
+ }
+
+ confirmAggUsesIndex("test", [{$match: {x: 3}}], "x_1", {x: 1});
+
+ //
+ // For each of the following tests we confirm:
+ // * That the expected index is chosen by the query planner when no hint is provided.
+ // * That the expected index is chosen when hinted.
+ // * That an index other than the one expected is chosen when hinted.
+ //
+
+ // Hint on poor index choice should force use of the hinted index over one more optimal.
+ coll.drop();
+ assert.commandWorked(coll.createIndex({x: 1}));
+ for (let i = 0; i < 5; ++i) {
+ assert.writeOK(coll.insert({x: i}));
+ }
+
+ confirmAggUsesIndex("test", [{$match: {x: 3}}], NO_HINT, {x: 1});
+ confirmAggUsesIndex("test", [{$match: {x: 3}}], {x: 1}, {x: 1});
+ confirmAggUsesIndex("test", [{$match: {x: 3}}], {_id: 1}, {_id: 1});
+
+ // With no hint specified, aggregation will always prefer an index that provides sort order over
+ // one that requires a blocking sort. A hinted aggregation should allow for choice of an index
+ // that provides blocking sort.
+ coll.drop();
+ assert.commandWorked(coll.createIndex({x: 1}));
+ assert.commandWorked(coll.createIndex({y: 1}));
+ for (let i = 0; i < 5; ++i) {
+ assert.writeOK(coll.insert({x: i, y: i}));
+ }
+
+ confirmAggUsesIndex("test", [{$match: {x: {$gte: 0}}}, {$sort: {y: 1}}], NO_HINT, {y: 1});
+ confirmAggUsesIndex("test", [{$match: {x: {$gte: 0}}}, {$sort: {y: 1}}], {y: 1}, {y: 1});
+ confirmAggUsesIndex("test", [{$match: {x: {$gte: 0}}}, {$sort: {y: 1}}], {x: 1}, {x: 1});
+
+ // With no hint specified, aggregation will always prefer an index that provides a covered
+ // projection over one that does not. A hinted aggregation should allow for choice of an index
+ // that does not cover.
+ coll.drop();
+ assert.commandWorked(coll.createIndex({x: 1}));
+ assert.commandWorked(coll.createIndex({x: 1, y: 1}));
+ for (let i = 0; i < 5; ++i) {
+ assert.writeOK(coll.insert({x: i, y: i}));
+ }
+
+ confirmAggUsesIndex("test",
+ [{$match: {x: {$gte: 0}}}, {$project: {x: 1, y: 1, _id: 0}}],
+ NO_HINT,
+ {x: 1, y: 1});
+ confirmAggUsesIndex("test",
+ [{$match: {x: {$gte: 0}}}, {$project: {x: 1, y: 1, _id: 0}}],
+ {x: 1, y: 1},
+ {x: 1, y: 1});
+ confirmAggUsesIndex(
+ "test", [{$match: {x: {$gte: 0}}}, {$project: {x: 1, y: 1, _id: 0}}], {x: 1}, {x: 1});
+
+ // Confirm that a hinted agg can be executed against a view.
+ coll.drop();
+ view.drop();
+ assert.commandWorked(coll.createIndex({x: 1}));
+ for (let i = 0; i < 5; ++i) {
+ assert.writeOK(coll.insert({x: i}));
+ }
+ assert.commandWorked(testDB.createView("view", "test", []));
+
+ confirmAggUsesIndex("view", [{$match: {x: 3}}], NO_HINT, {x: 1});
+ confirmAggUsesIndex("view", [{$match: {x: 3}}], {x: 1}, {x: 1});
+ confirmAggUsesIndex("view", [{$match: {x: 3}}], {_id: 1}, {_id: 1});
+
+ // Confirm that a hinted find can be executed against a view.
+ coll.drop();
+ view.drop();
+ assert.commandWorked(coll.createIndex({x: 1}));
+ for (let i = 0; i < 5; ++i) {
+ assert.writeOK(coll.insert({x: i}));
+ }
+ assert.commandWorked(testDB.createView("view", "test", []));
+
+ confirmCommandUsesIndex({find: "view", filter: {x: 3}}, NO_HINT, {x: 1});
+ confirmCommandUsesIndex({find: "view", filter: {x: 3}}, {x: 1}, {x: 1});
+ confirmCommandUsesIndex({find: "view", filter: {x: 3}}, {_id: 1}, {_id: 1});
+
+ // Confirm that a hinted count can be executed against a view.
+ coll.drop();
+ view.drop();
+ assert.commandWorked(coll.createIndex({x: 1}));
+ for (let i = 0; i < 5; ++i) {
+ assert.writeOK(coll.insert({x: i}));
+ }
+ assert.commandWorked(testDB.createView("view", "test", []));
+
+ confirmCommandUsesIndex({count: "view", query: {x: 3}}, NO_HINT, {x: 1}, "COUNT_SCAN");
+ confirmCommandUsesIndex({count: "view", query: {x: 3}}, {x: 1}, {x: 1}, "COUNT_SCAN");
+ confirmCommandUsesIndex({count: "view", query: {x: 3}}, {_id: 1}, {_id: 1});
+})(); \ No newline at end of file
diff --git a/jstests/core/profile_agg.js b/jstests/core/profile_agg.js
index 3422d171335..39f4fd788ed 100644
--- a/jstests/core/profile_agg.js
+++ b/jstests/core/profile_agg.js
@@ -69,4 +69,17 @@
profileObj = getLatestProfilerEntry(testDB);
assert.eq(profileObj.fromMultiPlanner, true, tojson(profileObj));
+
+ //
+ // Confirm that the "hint" modifier is in the profiler document.
+ //
+ coll.drop();
+ assert.commandWorked(coll.createIndex({a: 1}));
+ for (i = 0; i < 5; ++i) {
+ assert.writeOK(coll.insert({a: i, b: i}));
+ }
+
+ assert.eq(1, coll.aggregate([{$match: {a: 3, b: 3}}], {hint: {_id: 1}}).itcount());
+ profileObj = getLatestProfilerEntry(testDB);
+ assert.eq(profileObj.command.hint, {_id: 1}, tojson(profileObj));
})();
diff --git a/jstests/core/profile_getmore.js b/jstests/core/profile_getmore.js
index a9272567b1a..09ec72d4bd4 100644
--- a/jstests/core/profile_getmore.js
+++ b/jstests/core/profile_getmore.js
@@ -100,7 +100,7 @@
}
assert.commandWorked(coll.createIndex({a: 1}));
- var cursor = coll.aggregate([{$match: {a: {$gte: 0}}}], {cursor: {batchSize: 0}});
+ var cursor = coll.aggregate([{$match: {a: {$gte: 0}}}], {cursor: {batchSize: 0}, hint: {a: 1}});
var cursorId = getLatestProfilerEntry(testDB).cursorid;
assert.neq(0, cursorId);
@@ -122,4 +122,7 @@
assert.eq(profileObj.keysExamined, 20, tojson(profileObj));
assert.eq(profileObj.docsExamined, 20, tojson(profileObj));
assert.eq(profileObj.appName, "MongoDB Shell", tojson(profileObj));
+ if (!isLegacyReadMode) {
+ assert.eq(profileObj.originatingCommand.hint, {a: 1}, tojson(profileObj));
+ }
})();
diff --git a/jstests/core/views/views_count.js b/jstests/core/views/views_count.js
index e7503e1d74d..ddf97ce0bb2 100644
--- a/jstests/core/views/views_count.js
+++ b/jstests/core/views/views_count.js
@@ -53,11 +53,10 @@
assert.commandWorked(explainPlan);
assert.eq(explainPlan["stages"][0]["$cursor"]["queryPlanner"]["namespace"], "views_count.coll");
+ // Count with hint works on a view.
+ assert.commandWorked(viewsDB.runCommand({count: "identityView", hint: "_id_"}));
+
assert.commandFailedWithCode(
viewsDB.runCommand({count: "identityView", collation: {locale: "en_US"}}),
ErrorCodes.OptionNotSupportedOnView);
-
- // Hint cannot be used when counting on a view.
- assert.commandFailedWithCode(viewsDB.runCommand({count: "identityView", hint: "_id_"}),
- ErrorCodes.InvalidPipelineOperator);
}());
diff --git a/jstests/libs/analyze_plan.js b/jstests/libs/analyze_plan.js
index 62511ae1fac..f5a96b96d92 100644
--- a/jstests/libs/analyze_plan.js
+++ b/jstests/libs/analyze_plan.js
@@ -56,6 +56,56 @@ function getPlanStage(root, stage) {
}
/**
+ * Given the root stage of agg explain's JSON representation of a query plan ('root'), returns all
+ * subdocuments whose stage is 'stage'. Returns an empty array if the plan does not have the
+ * requested stage. Asserts that agg explain structure matches expected format.
+ */
+function getAggPlanStages(root, stage) {
+ let results = [];
+
+ if (root.hasOwnProperty("stages")) {
+ assert(root.stages.constructor === Array);
+ assert(root.stages[0].hasOwnProperty("$cursor"));
+ assert(root.stages[0].$cursor.hasOwnProperty("queryPlanner"));
+ assert(root.stages[0].$cursor.queryPlanner.hasOwnProperty("winningPlan"));
+ results =
+ results.concat(getPlanStages(root.stages[0].$cursor.queryPlanner.winningPlan, stage));
+ }
+
+ if (root.hasOwnProperty("shards")) {
+ for (let elem in root.shards) {
+ assert(root.shards[elem].stages.constructor === Array);
+ assert(root.shards[elem].stages[0].hasOwnProperty("$cursor"));
+ assert(root.shards[elem].stages[0].$cursor.hasOwnProperty("queryPlanner"));
+ assert(root.shards[elem].stages[0].$cursor.queryPlanner.hasOwnProperty("winningPlan"));
+ results = results.concat(
+ getPlanStages(root.shards[elem].stages[0].$cursor.queryPlanner.winningPlan, stage));
+ }
+ }
+
+ return results;
+}
+
+/**
+ * Given the root stage of agg explain's JSON representation of a query plan ('root'), returns the
+ * subdocument with its stage as 'stage'. Returns null if the plan does not have such a stage.
+ * Asserts that no more than one stage is a match.
+ */
+function getAggPlanStage(root, stage) {
+ let planStageList = getAggPlanStages(root, stage);
+
+ if (planStageList.length === 0) {
+ return null;
+ } else {
+ assert.eq(1,
+ planStageList.length,
+ "getAggPlanStage expects to find 0 or 1 matching stages. planStageList: " +
+ tojson(planStageList));
+ return planStageList[0];
+ }
+}
+
+/**
* Given the root stage of explain's BSON representation of a query plan ('root'),
* returns true if the plan has a stage called 'stage'.
*/
diff --git a/jstests/noPassthrough/currentop_query.js b/jstests/noPassthrough/currentop_query.js
index e01413ab58c..c4e2d87c1a6 100644
--- a/jstests/noPassthrough/currentop_query.js
+++ b/jstests/noPassthrough/currentop_query.js
@@ -100,15 +100,16 @@
test: function() {
assert.eq(db.currentop_query
.aggregate([{$match: {a: 1, $comment: "currentop_query"}}],
- {collation: {locale: "fr"}})
+ {collation: {locale: "fr"}, hint: {_id: 1}})
.itcount(),
1);
},
command: "aggregate",
- planSummary: "COLLSCAN",
+ planSummary: "IXSCAN { _id: 1 }",
currentOpFilter: {
"query.pipeline.0.$match.$comment": "currentop_query",
- "query.collation": {locale: "fr"}
+ "query.collation": {locale: "fr"},
+ "query.hint": {_id: 1}
}
},
{
diff --git a/src/mongo/db/commands/pipeline_command.cpp b/src/mongo/db/commands/pipeline_command.cpp
index d548d9523ca..c34ee28a72c 100644
--- a/src/mongo/db/commands/pipeline_command.cpp
+++ b/src/mongo/db/commands/pipeline_command.cpp
@@ -482,7 +482,7 @@ public:
// This does mongod-specific stuff like creating the input PlanExecutor and adding
// it to the front of the pipeline if needed.
- PipelineD::prepareCursorSource(collection, pipeline);
+ PipelineD::prepareCursorSource(collection, &request, pipeline);
// Create the PlanExecutor which returns results from the pipeline. The WorkingSet
// ('ws') and the PipelineProxyStage ('proxy') will be owned by the created
diff --git a/src/mongo/db/pipeline/aggregation_request.cpp b/src/mongo/db/pipeline/aggregation_request.cpp
index 57a8f3644a9..947cfbb3f81 100644
--- a/src/mongo/db/pipeline/aggregation_request.cpp
+++ b/src/mongo/db/pipeline/aggregation_request.cpp
@@ -54,6 +54,7 @@ const StringData AggregationRequest::kPipelineName = "pipeline"_sd;
const StringData AggregationRequest::kCollationName = "collation"_sd;
const StringData AggregationRequest::kExplainName = "explain"_sd;
const StringData AggregationRequest::kAllowDiskUseName = "allowDiskUse"_sd;
+const StringData AggregationRequest::kHintName = "hint"_sd;
const long long AggregationRequest::kDefaultBatchSize = 101;
@@ -119,6 +120,18 @@ StatusWith<AggregationRequest> AggregationRequest::parseFromBSON(NamespaceString
<< typeName(elem.type())};
}
request.setCollation(elem.embeddedObject().getOwned());
+ } else if (kHintName == fieldName) {
+ if (BSONType::Object == elem.type()) {
+ request.setHint(elem.embeddedObject());
+ } else if (BSONType::String == elem.type()) {
+ request.setHint(BSON("$hint" << elem.valueStringData()));
+ } else {
+ return Status(ErrorCodes::FailedToParse,
+ str::stream()
+ << kHintName
+ << " must be specified as a string representing an index"
+ << " name, or an object representing an index's key pattern");
+ }
} else if (kExplainName == fieldName) {
if (elem.type() != BSONType::Bool) {
return {ErrorCodes::TypeMismatch,
@@ -176,7 +189,9 @@ Document AggregationRequest::serializeToCommandObj() const {
// Only serialize a collation if one was specified.
{kCollationName, _collation.isEmpty() ? Value() : Value(_collation)},
// Only serialize batchSize when explain is false.
- {kCursorName, _explain ? Value() : Value(Document{{kBatchSizeName, _batchSize}})}};
+ {kCursorName, _explain ? Value() : Value(Document{{kBatchSizeName, _batchSize}})},
+ // Only serialize a hint if one was specified.
+ {kHintName, _hint.isEmpty() ? Value() : Value(_hint)}};
}
} // namespace mongo
diff --git a/src/mongo/db/pipeline/aggregation_request.h b/src/mongo/db/pipeline/aggregation_request.h
index 4a3ef4c0223..b25eaa020dc 100644
--- a/src/mongo/db/pipeline/aggregation_request.h
+++ b/src/mongo/db/pipeline/aggregation_request.h
@@ -54,6 +54,7 @@ public:
static const StringData kCollationName;
static const StringData kExplainName;
static const StringData kAllowDiskUseName;
+ static const StringData kHintName;
static const long long kDefaultBatchSize;
@@ -119,6 +120,10 @@ public:
return _collation;
}
+ BSONObj getHint() const {
+ return _hint;
+ }
+
//
// Setters for optional fields.
//
@@ -135,6 +140,10 @@ public:
_collation = collation.getOwned();
}
+ void setHint(BSONObj hint) {
+ _hint = hint.getOwned();
+ }
+
void setExplain(bool isExplain) {
_explain = isExplain;
}
@@ -167,6 +176,11 @@ private:
// specified.
BSONObj _collation;
+ // The hint provided, if any. If the hint was by index key pattern, the value of '_hint' is
+ // the key pattern hinted. If the hint was by index name, the value of '_hint' is
+ // {$hint: <String>}, where <String> is the index name hinted.
+ BSONObj _hint;
+
bool _explain = false;
bool _allowDiskUse = false;
bool _fromRouter = false;
diff --git a/src/mongo/db/pipeline/aggregation_request_test.cpp b/src/mongo/db/pipeline/aggregation_request_test.cpp
index 4d1a5f33fb6..45313fc8a1e 100644
--- a/src/mongo/db/pipeline/aggregation_request_test.cpp
+++ b/src/mongo/db/pipeline/aggregation_request_test.cpp
@@ -55,13 +55,15 @@ TEST(AggregationRequestTest, ShouldParseAllKnownOptions) {
NamespaceString nss("a.collection");
const BSONObj inputBson = fromjson(
"{pipeline: [{$match: {a: 'abc'}}], explain: true, allowDiskUse: true, fromRouter: true, "
- "bypassDocumentValidation: true, collation: {locale: 'en_US'}, cursor: {batchSize: 10}}");
+ "bypassDocumentValidation: true, collation: {locale: 'en_US'}, cursor: {batchSize: 10}, "
+ "hint: {a: 1}}");
auto request = unittest::assertGet(AggregationRequest::parseFromBSON(nss, inputBson));
ASSERT_TRUE(request.isExplain());
ASSERT_TRUE(request.shouldAllowDiskUse());
ASSERT_TRUE(request.isFromRouter());
ASSERT_TRUE(request.shouldBypassDocumentValidation());
ASSERT_EQ(request.getBatchSize(), 10);
+ ASSERT_BSONOBJ_EQ(request.getHint(), BSON("a" << 1));
ASSERT_BSONOBJ_EQ(request.getCollation(),
BSON("locale"
<< "en_US"));
@@ -90,6 +92,7 @@ TEST(AggregationRequestTest, ShouldNotSerializeOptionalValuesIfEquivalentToDefau
request.setFromRouter(false);
request.setBypassDocumentValidation(false);
request.setCollation(BSONObj());
+ request.setHint(BSONObj());
auto expectedSerialization =
Document{{AggregationRequest::kCommandName, nss.coll()},
@@ -106,6 +109,8 @@ TEST(AggregationRequestTest, ShouldSerializeOptionalValuesIfSet) {
request.setFromRouter(true);
request.setBypassDocumentValidation(true);
request.setBatchSize(10); // batchSize not serialzed when explain is true.
+ const auto hintObj = BSON("a" << 1);
+ request.setHint(hintObj);
const auto collationObj = BSON("locale"
<< "en_US");
request.setCollation(collationObj);
@@ -117,7 +122,8 @@ TEST(AggregationRequestTest, ShouldSerializeOptionalValuesIfSet) {
{AggregationRequest::kAllowDiskUseName, true},
{AggregationRequest::kFromRouterName, true},
{bypassDocumentValidationCommandOption(), true},
- {AggregationRequest::kCollationName, collationObj}};
+ {AggregationRequest::kCollationName, collationObj},
+ {AggregationRequest::kHintName, hintObj}};
ASSERT_DOCUMENT_EQ(request.serializeToCommandObj(), expectedSerialization);
}
@@ -142,6 +148,17 @@ TEST(AggregationRequestTest, ShouldSetBatchSizeToDefaultOnEmptyCursorObject) {
ASSERT_EQ(request.getValue().getBatchSize(), AggregationRequest::kDefaultBatchSize);
}
+TEST(AggregationRequestTest, ShouldAcceptHintAsString) {
+ NamespaceString nss("a.collection");
+ const BSONObj inputBson =
+ fromjson("{pipeline: [{$match: {a: 'abc'}}], hint: 'a_1', cursor: {}}");
+ auto request = AggregationRequest::parseFromBSON(nss, inputBson);
+ ASSERT_OK(request.getStatus());
+ ASSERT_BSONOBJ_EQ(request.getValue().getHint(),
+ BSON("$hint"
+ << "a_1"));
+}
+
//
// Error cases.
//
@@ -169,6 +186,21 @@ TEST(AggregationRequestTest, ShouldRejectNonObjectCollation) {
AggregationRequest::parseFromBSON(NamespaceString("a.collection"), inputBson).getStatus());
}
+TEST(AggregationRequestTest, ShouldRejectNonStringNonObjectHint) {
+ NamespaceString nss("a.collection");
+ const BSONObj inputBson = fromjson("{pipeline: [{$match: {a: 'abc'}}], cursor: {}, hint: 1}");
+ ASSERT_NOT_OK(
+ AggregationRequest::parseFromBSON(NamespaceString("a.collection"), inputBson).getStatus());
+}
+
+TEST(AggregationRequestTest, ShouldRejectHintAsArray) {
+ NamespaceString nss("a.collection");
+ const BSONObj inputBson =
+ fromjson("{pipeline: [{$match: {a: 'abc'}}], cursor: {}, hint: []}]}");
+ ASSERT_NOT_OK(
+ AggregationRequest::parseFromBSON(NamespaceString("a.collection"), inputBson).getStatus());
+}
+
TEST(AggregationRequestTest, ShouldRejectNonBoolExplain) {
NamespaceString nss("a.collection");
const BSONObj inputBson =
diff --git a/src/mongo/db/pipeline/pipeline_d.cpp b/src/mongo/db/pipeline/pipeline_d.cpp
index 5eee80f6cc7..0de995f29a8 100644
--- a/src/mongo/db/pipeline/pipeline_d.cpp
+++ b/src/mongo/db/pipeline/pipeline_d.cpp
@@ -205,7 +205,7 @@ public:
auto css = CollectionShardingState::get(_ctx->opCtx, expCtx->ns);
uassert(4567, "from collection cannot be sharded", !bool(css->getMetadata()));
- PipelineD::prepareCursorSource(autoColl.getCollection(), pipeline.getValue());
+ PipelineD::prepareCursorSource(autoColl.getCollection(), nullptr, pipeline.getValue());
return pipeline;
}
@@ -296,11 +296,15 @@ StatusWith<std::unique_ptr<PlanExecutor>> attemptToGetExecutor(
BSONObj queryObj,
BSONObj projectionObj,
BSONObj sortObj,
+ const AggregationRequest* aggRequest,
const size_t plannerOpts) {
auto qr = stdx::make_unique<QueryRequest>(pExpCtx->ns);
qr->setFilter(queryObj);
qr->setProj(projectionObj);
qr->setSort(sortObj);
+ if (aggRequest) {
+ qr->setHint(aggRequest->getHint());
+ }
// If the pipeline has a non-null collator, set the collation option to the result of
// serializing the collator's spec back into BSON. We do this in order to fill in all options
@@ -331,6 +335,7 @@ StatusWith<std::unique_ptr<PlanExecutor>> attemptToGetExecutor(
} // namespace
void PipelineD::prepareCursorSource(Collection* collection,
+ const AggregationRequest* aggRequest,
const intrusive_ptr<Pipeline>& pipeline) {
auto expCtx = pipeline->getContext();
dassert(expCtx->opCtx->lockState()->isCollectionLockedForMode(expCtx->ns.ns(), MODE_IS));
@@ -432,6 +437,7 @@ void PipelineD::prepareCursorSource(Collection* collection,
sortStage,
deps,
queryObj,
+ aggRequest,
&sortObj,
&projForQuery));
@@ -448,6 +454,7 @@ StatusWith<std::unique_ptr<PlanExecutor>> PipelineD::prepareExecutor(
const intrusive_ptr<DocumentSourceSort>& sortStage,
const DepsTracker& deps,
const BSONObj& queryObj,
+ const AggregationRequest* aggRequest,
BSONObj* sortObj,
BSONObj* projectionObj) {
// The query system has the potential to use an index to provide a non-blocking sort and/or to
@@ -493,12 +500,18 @@ StatusWith<std::unique_ptr<PlanExecutor>> PipelineD::prepareExecutor(
if (sortStage) {
// See if the query system can provide a non-blocking sort.
auto swExecutorSort = attemptToGetExecutor(
- txn, collection, expCtx, queryObj, emptyProjection, *sortObj, plannerOpts);
+ txn, collection, expCtx, queryObj, emptyProjection, *sortObj, aggRequest, plannerOpts);
if (swExecutorSort.isOK()) {
// Success! Now see if the query system can also cover the projection.
- auto swExecutorSortAndProj = attemptToGetExecutor(
- txn, collection, expCtx, queryObj, *projectionObj, *sortObj, plannerOpts);
+ auto swExecutorSortAndProj = attemptToGetExecutor(txn,
+ collection,
+ expCtx,
+ queryObj,
+ *projectionObj,
+ *sortObj,
+ aggRequest,
+ plannerOpts);
std::unique_ptr<PlanExecutor> exec;
if (swExecutorSortAndProj.isOK()) {
@@ -540,7 +553,7 @@ StatusWith<std::unique_ptr<PlanExecutor>> PipelineD::prepareExecutor(
// See if the query system can cover the projection.
auto swExecutorProj = attemptToGetExecutor(
- txn, collection, expCtx, queryObj, *projectionObj, *sortObj, plannerOpts);
+ txn, collection, expCtx, queryObj, *projectionObj, *sortObj, aggRequest, plannerOpts);
if (swExecutorProj.isOK()) {
// Success! We have a covered projection.
return std::move(swExecutorProj.getValue());
@@ -555,7 +568,7 @@ StatusWith<std::unique_ptr<PlanExecutor>> PipelineD::prepareExecutor(
*projectionObj = BSONObj();
// If this doesn't work, nothing will.
return attemptToGetExecutor(
- txn, collection, expCtx, queryObj, *projectionObj, *sortObj, plannerOpts);
+ txn, collection, expCtx, queryObj, *projectionObj, *sortObj, aggRequest, plannerOpts);
}
void PipelineD::addCursorSource(Collection* collection,
diff --git a/src/mongo/db/pipeline/pipeline_d.h b/src/mongo/db/pipeline/pipeline_d.h
index 20b6cf16965..2df4dc45036 100644
--- a/src/mongo/db/pipeline/pipeline_d.h
+++ b/src/mongo/db/pipeline/pipeline_d.h
@@ -33,6 +33,7 @@
#include "mongo/bson/bsonobj.h"
#include "mongo/db/namespace_string.h"
+#include "mongo/db/pipeline/aggregation_request.h"
namespace mongo {
class Collection;
@@ -71,8 +72,11 @@ public:
* The cursor is added to the front of the pipeline's sources.
*
* Callers must take care to ensure that 'collection' is locked in at least IS-mode.
+ *
+ * When not null, 'aggRequest' provides access to pipeline command options such as hint.
*/
static void prepareCursorSource(Collection* collection,
+ const AggregationRequest* aggRequest,
const boost::intrusive_ptr<Pipeline>& pipeline);
static std::string getPlanSummaryStr(const boost::intrusive_ptr<Pipeline>& pPipeline);
@@ -101,6 +105,7 @@ private:
const boost::intrusive_ptr<DocumentSourceSort>& sortStage,
const DepsTracker& deps,
const BSONObj& queryObj,
+ const AggregationRequest* aggRequest,
BSONObj* sortObj,
BSONObj* projectionObj);
diff --git a/src/mongo/db/query/count_request.cpp b/src/mongo/db/query/count_request.cpp
index f412065ab9c..6abddd36ec1 100644
--- a/src/mongo/db/query/count_request.cpp
+++ b/src/mongo/db/query/count_request.cpp
@@ -143,12 +143,6 @@ StatusWith<CountRequest> CountRequest::parseFromBSON(const std::string& dbname,
}
StatusWith<BSONObj> CountRequest::asAggregationCommand() const {
- // The 'hint' option is not supported in aggregation.
- if (_hint) {
- return {ErrorCodes::InvalidPipelineOperator,
- str::stream() << "Option " << kHintField << " not supported in aggregation."};
- }
-
BSONObjBuilder aggregationBuilder;
aggregationBuilder.append("aggregate", _nss.coll());
@@ -185,6 +179,10 @@ StatusWith<BSONObj> CountRequest::asAggregationCommand() const {
aggregationBuilder.append(kCollationField, *_collation);
}
+ if (_hint) {
+ aggregationBuilder.append(kHintField, *_hint);
+ }
+
// The 'cursor' option is always specified so that aggregation uses the cursor interface.
aggregationBuilder.append("cursor", BSONObj());
diff --git a/src/mongo/db/query/count_request_test.cpp b/src/mongo/db/query/count_request_test.cpp
index 41cff25449f..e2a40f87ccf 100644
--- a/src/mongo/db/query/count_request_test.cpp
+++ b/src/mongo/db/query/count_request_test.cpp
@@ -209,10 +209,23 @@ TEST(CountRequest, ToBSON) {
ASSERT_BSONOBJ_EQ(actualObj, expectedObj);
}
-TEST(CountRequest, ConvertToAggregationWithHintFails) {
+TEST(CountRequest, ConvertToAggregationWithHint) {
CountRequest countRequest(testns, BSONObj());
countRequest.setHint(BSON("x" << 1));
- ASSERT_NOT_OK(countRequest.asAggregationCommand());
+ auto agg = countRequest.asAggregationCommand();
+ ASSERT_OK(agg);
+
+ auto ar = AggregationRequest::parseFromBSON(testns, agg.getValue());
+ ASSERT_OK(ar.getStatus());
+ ASSERT_BSONOBJ_EQ(ar.getValue().getHint(), BSON("x" << 1));
+
+ std::vector<BSONObj> expectedPipeline{BSON("$count"
+ << "count")};
+ ASSERT(std::equal(expectedPipeline.begin(),
+ expectedPipeline.end(),
+ ar.getValue().getPipeline().begin(),
+ ar.getValue().getPipeline().end(),
+ SimpleBSONObjComparator::kInstance.makeEqualTo()));
}
TEST(CountRequest, ConvertToAggregationSucceeds) {
diff --git a/src/mongo/db/query/query_planner.cpp b/src/mongo/db/query/query_planner.cpp
index 850276f35e7..b19a6bf0e5f 100644
--- a/src/mongo/db/query/query_planner.cpp
+++ b/src/mongo/db/query/query_planner.cpp
@@ -841,10 +841,13 @@ Status QueryPlanner::plan(const CanonicalQuery& query,
// desired behavior when an index is hinted that is not relevant to the query.
if (!hintIndex.isEmpty()) {
if (0 == out->size()) {
- QuerySolution* soln = buildWholeIXSoln(params.indices[*hintIndexNumber], query, params);
- verify(NULL != soln);
- LOG(5) << "Planner: outputting soln that uses hinted index as scan.";
- out->push_back(soln);
+ // Push hinted index solution to output list if found. It is possible to end up without
+ // a solution in the case where a filtering QueryPlannerParams argument, such as
+ // NO_BLOCKING_SORT, leads to its exclusion.
+ if (auto soln = buildWholeIXSoln(params.indices[*hintIndexNumber], query, params)) {
+ LOG(5) << "Planner: outputting soln that uses hinted index as scan.";
+ out->push_back(soln);
+ }
}
return Status::OK();
}
diff --git a/src/mongo/db/query/query_planner_test.cpp b/src/mongo/db/query/query_planner_test.cpp
index 2241ecb91e6..2efcc825f58 100644
--- a/src/mongo/db/query/query_planner_test.cpp
+++ b/src/mongo/db/query/query_planner_test.cpp
@@ -2395,6 +2395,24 @@ TEST_F(QueryPlannerTest, HintInvalid) {
runInvalidQueryHint(BSONObj(), fromjson("{b: 1}"));
}
+TEST_F(QueryPlannerTest, HintedBlockingSortIndexFilteredOut) {
+ params.options = QueryPlannerParams::NO_BLOCKING_SORT;
+ addIndex(BSON("a" << 1));
+ addIndex(BSON("b" << 1));
+ runQueryAsCommand(
+ fromjson("{find: 'testns', filter: {a: 1, b: 1}, sort: {b: 1}, hint: {a: 1}}"));
+ assertNumSolutions(0U);
+}
+
+TEST_F(QueryPlannerTest, HintedNotCoveredProjectionIndexFilteredOut) {
+ params.options = QueryPlannerParams::NO_UNCOVERED_PROJECTIONS;
+ addIndex(BSON("a" << 1));
+ addIndex(BSON("a" << 1 << "b" << 1));
+ runQueryAsCommand(fromjson(
+ "{find: 'testns', filter: {a: 1}, projection: {a: 1, b: 1, _id: 0}, hint: {a: 1}}"));
+ assertNumSolutions(0U);
+}
+
//
// Sparse indices, SERVER-8067
// Each index in this block of tests is sparse.
diff --git a/src/mongo/db/query/query_request.cpp b/src/mongo/db/query/query_request.cpp
index b0f1334de8d..b58a0665868 100644
--- a/src/mongo/db/query/query_request.cpp
+++ b/src/mongo/db/query/query_request.cpp
@@ -964,10 +964,6 @@ StatusWith<BSONObj> QueryRequest::asAggregationCommand() const {
return {ErrorCodes::InvalidPipelineOperator,
str::stream() << "Option " << kReturnKeyField << " not supported in aggregation."};
}
- if (!_hint.isEmpty()) {
- return {ErrorCodes::InvalidPipelineOperator,
- str::stream() << "Option " << kHintField << " not supported in aggregation."};
- }
if (!_comment.empty()) {
return {ErrorCodes::InvalidPipelineOperator,
str::stream() << "Option " << kCommentField << " not supported in aggregation."};
@@ -1068,6 +1064,9 @@ StatusWith<BSONObj> QueryRequest::asAggregationCommand() const {
if (_maxTimeMS > 0) {
aggregationBuilder.append(cmdOptionMaxTimeMS, _maxTimeMS);
}
+ if (!_hint.isEmpty()) {
+ aggregationBuilder.append("hint", _hint);
+ }
return StatusWith<BSONObj>(aggregationBuilder.obj());
}
} // namespace mongo
diff --git a/src/mongo/db/query/query_request_test.cpp b/src/mongo/db/query/query_request_test.cpp
index bd40a94ef33..66a03bc8285 100644
--- a/src/mongo/db/query/query_request_test.cpp
+++ b/src/mongo/db/query/query_request_test.cpp
@@ -1123,6 +1123,17 @@ TEST(QueryRequestTest, ConvertToAggregationWithExplainSucceeds) {
ASSERT_BSONOBJ_EQ(ar.getValue().getCollation(), BSONObj());
}
+TEST(QueryRequestTest, ConvertToAggregationWithHintSucceeds) {
+ QueryRequest qr(testns);
+ qr.setHint(fromjson("{a_1: -1}"));
+ const auto aggCmd = qr.asAggregationCommand();
+ ASSERT_OK(aggCmd);
+
+ auto ar = AggregationRequest::parseFromBSON(testns, aggCmd.getValue());
+ ASSERT_OK(ar.getStatus());
+ ASSERT_BSONOBJ_EQ(qr.getHint(), ar.getValue().getHint());
+}
+
TEST(QueryRequestTest, ConvertToAggregationWithMinFails) {
QueryRequest qr(testns);
qr.setMin(fromjson("{a: 1}"));
@@ -1167,12 +1178,6 @@ TEST(QueryRequestTest, ConvertToAggregationWithReturnKeyFails) {
ASSERT_NOT_OK(qr.asAggregationCommand());
}
-TEST(QueryRequestTest, ConvertToAggregationWithHintFails) {
- QueryRequest qr(testns);
- qr.setHint(fromjson("{a_1: -1}"));
- ASSERT_NOT_OK(qr.asAggregationCommand());
-}
-
TEST(QueryRequestTest, ConvertToAggregationWithCommentFails) {
QueryRequest qr(testns);
qr.setComment("test");
diff --git a/src/mongo/db/views/resolved_view.cpp b/src/mongo/db/views/resolved_view.cpp
index c5d955a08d1..5fcdecc857e 100644
--- a/src/mongo/db/views/resolved_view.cpp
+++ b/src/mongo/db/views/resolved_view.cpp
@@ -91,6 +91,10 @@ StatusWith<BSONObj> ResolvedView::asExpandedViewAggregation(
batchSizeBuilder.doneFast();
}
+ if (!request.getHint().isEmpty()) {
+ aggregationBuilder.append(AggregationRequest::kHintName, request.getHint());
+ }
+
if (request.shouldBypassDocumentValidation()) {
aggregationBuilder.append("bypassDocumentValidation", true);
}
View Lorry Controller Status