summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDavid Storch <david.storch@10gen.com>2018-01-09 17:57:41 -0500
committerDavid Storch <david.storch@10gen.com>2018-01-11 18:43:25 -0500
commite377025071e4c9fff336fc57741e5cc8511607af (patch)
treed34bb3208881fecce2d41b1905fe34d849f1d92c
parentc16959f19705772d01e1053899048869aafe4537 (diff)
downloadmongo-e377025071e4c9fff336fc57741e5cc8511607af.tar.gz
SERVER-32603 Modernize QueryPlanner methods to use unique_ptr.
-rw-r--r--src/mongo/db/exec/cached_plan.cpp14
-rw-r--r--src/mongo/db/exec/multi_plan.cpp6
-rw-r--r--src/mongo/db/exec/multi_plan.h4
-rw-r--r--src/mongo/db/exec/subplan.cpp35
-rw-r--r--src/mongo/db/query/get_executor.cpp80
-rw-r--r--src/mongo/db/query/plan_cache_test.cpp77
-rw-r--r--src/mongo/db/query/plan_ranker.h4
-rw-r--r--src/mongo/db/query/planner_analysis.cpp7
-rw-r--r--src/mongo/db/query/planner_analysis.h22
-rw-r--r--src/mongo/db/query/query_planner.cpp182
-rw-r--r--src/mongo/db/query/query_planner.h41
-rw-r--r--src/mongo/db/query/query_planner_test.cpp11
-rw-r--r--src/mongo/db/query/query_planner_test_fixture.cpp25
-rw-r--r--src/mongo/dbtests/plan_ranking.cpp10
-rw-r--r--src/mongo/dbtests/query_stage_multiplan.cpp29
-rw-r--r--src/mongo/s/chunk_manager.cpp12
16 files changed, 226 insertions, 333 deletions
diff --git a/src/mongo/db/exec/cached_plan.cpp b/src/mongo/db/exec/cached_plan.cpp
index 55a391b57c3..b1bf5d16cbd 100644
--- a/src/mongo/db/exec/cached_plan.cpp
+++ b/src/mongo/db/exec/cached_plan.cpp
@@ -200,17 +200,15 @@ Status CachedPlanStage::replan(PlanYieldPolicy* yieldPolicy, bool shouldCache) {
_specificStats.replanned = true;
// Use the query planning module to plan the whole query.
- std::vector<QuerySolution*> rawSolutions;
- Status status = QueryPlanner::plan(*_canonicalQuery, _plannerParams, &rawSolutions);
- if (!status.isOK()) {
+ auto statusWithSolutions = QueryPlanner::plan(*_canonicalQuery, _plannerParams);
+ if (!statusWithSolutions.isOK()) {
return Status(ErrorCodes::BadValue,
str::stream() << "error processing query: " << _canonicalQuery->toString()
<< " planner returned error: "
- << status.reason());
+ << statusWithSolutions.getStatus().reason());
}
- std::vector<std::unique_ptr<QuerySolution>> solutions =
- transitional_tools_do_not_use::spool_vector(rawSolutions);
+ auto solutions = std::move(statusWithSolutions.getValue());
// We cannot figure out how to answer the query. Perhaps it requires an index
// we do not have?
@@ -261,8 +259,8 @@ Status CachedPlanStage::replan(PlanYieldPolicy* yieldPolicy, bool shouldCache) {
verify(StageBuilder::build(
getOpCtx(), _collection, *_canonicalQuery, *solutions[ix], _ws, &nextPlanRoot));
- // Takes ownership of 'solutions[ix]' and 'nextPlanRoot'.
- multiPlanStage->addPlan(solutions[ix].release(), nextPlanRoot, _ws);
+ // Takes ownership of 'nextPlanRoot'.
+ multiPlanStage->addPlan(std::move(solutions[ix]), nextPlanRoot, _ws);
}
// Delegate to the MultiPlanStage's plan selection facility.
diff --git a/src/mongo/db/exec/multi_plan.cpp b/src/mongo/db/exec/multi_plan.cpp
index d4c9e732717..34d7e6d5cc8 100644
--- a/src/mongo/db/exec/multi_plan.cpp
+++ b/src/mongo/db/exec/multi_plan.cpp
@@ -77,8 +77,10 @@ MultiPlanStage::MultiPlanStage(OperationContext* opCtx,
invariant(_collection);
}
-void MultiPlanStage::addPlan(QuerySolution* solution, PlanStage* root, WorkingSet* ws) {
- _candidates.push_back(CandidatePlan(solution, root, ws));
+void MultiPlanStage::addPlan(std::unique_ptr<QuerySolution> solution,
+ PlanStage* root,
+ WorkingSet* ws) {
+ _candidates.push_back(CandidatePlan(std::move(solution), root, ws));
_children.emplace_back(root);
}
diff --git a/src/mongo/db/exec/multi_plan.h b/src/mongo/db/exec/multi_plan.h
index 7ec6bad81d7..5d6369e3c0d 100644
--- a/src/mongo/db/exec/multi_plan.h
+++ b/src/mongo/db/exec/multi_plan.h
@@ -96,9 +96,9 @@ public:
const SpecificStats* getSpecificStats() const final;
/**
- * Takes ownership of QuerySolution and PlanStage. not of WorkingSet
+ * Takes ownership of PlanStage. Does not take ownership of WorkingSet.
*/
- void addPlan(QuerySolution* solution, PlanStage* root, WorkingSet* sharedWs);
+ void addPlan(std::unique_ptr<QuerySolution> solution, PlanStage* root, WorkingSet* sharedWs);
/**
* Runs all plans added by addPlan, ranks them, and picks a best.
diff --git a/src/mongo/db/exec/subplan.cpp b/src/mongo/db/exec/subplan.cpp
index 0d3ee99ffc4..caa80dd1461 100644
--- a/src/mongo/db/exec/subplan.cpp
+++ b/src/mongo/db/exec/subplan.cpp
@@ -155,17 +155,15 @@ Status SubplanStage::planSubqueries() {
// We don't set NO_TABLE_SCAN because peeking at the cache data will keep us from
// considering any plan that's a collscan.
invariant(branchResult->solutions.empty());
- std::vector<QuerySolution*> rawSolutions;
- Status status =
- QueryPlanner::plan(*branchResult->canonicalQuery, _plannerParams, &rawSolutions);
- branchResult->solutions = transitional_tools_do_not_use::spool_vector(rawSolutions);
-
- if (!status.isOK()) {
+ auto solutions = QueryPlanner::plan(*branchResult->canonicalQuery, _plannerParams);
+ if (!solutions.isOK()) {
mongoutils::str::stream ss;
ss << "Can't plan for subchild " << branchResult->canonicalQuery->toString() << " "
- << status.reason();
+ << solutions.getStatus().reason();
return Status(ErrorCodes::BadValue, ss);
}
+ branchResult->solutions = std::move(solutions.getValue());
+
LOG(5) << "Subplanner: got " << branchResult->solutions.size() << " solutions";
if (0 == branchResult->solutions.size()) {
@@ -281,8 +279,8 @@ Status SubplanStage::choosePlanForSubqueries(PlanYieldPolicy* yieldPolicy) {
_ws,
&nextPlanRoot));
- // Takes ownership of solution with index 'ix' and 'nextPlanRoot'.
- multiPlanStage->addPlan(branchResult->solutions[ix].release(), nextPlanRoot, _ws);
+ // Takes ownership of 'nextPlanRoot'.
+ multiPlanStage->addPlan(std::move(branchResult->solutions[ix]), nextPlanRoot, _ws);
}
Status planSelectStat = multiPlanStage->pickBestPlan(yieldPolicy);
@@ -343,8 +341,8 @@ Status SubplanStage::choosePlanForSubqueries(PlanYieldPolicy* yieldPolicy) {
LOG(5) << "Subplanner: fully tagged tree is " << redact(solnRoot->toString());
// Takes ownership of 'solnRoot'
- _compositeSolution.reset(
- QueryPlannerAnalysis::analyzeDataAccess(*_query, _plannerParams, std::move(solnRoot)));
+ _compositeSolution =
+ QueryPlannerAnalysis::analyzeDataAccess(*_query, _plannerParams, std::move(solnRoot));
if (NULL == _compositeSolution.get()) {
mongoutils::str::stream ss;
@@ -371,16 +369,15 @@ Status SubplanStage::choosePlanWholeQuery(PlanYieldPolicy* yieldPolicy) {
_ws->clear();
// Use the query planning module to plan the whole query.
- std::vector<QuerySolution*> rawSolutions;
- Status status = QueryPlanner::plan(*_query, _plannerParams, &rawSolutions);
- std::vector<std::unique_ptr<QuerySolution>> solutions =
- transitional_tools_do_not_use::spool_vector(rawSolutions);
- if (!status.isOK()) {
+ auto statusWithSolutions = QueryPlanner::plan(*_query, _plannerParams);
+ if (!statusWithSolutions.isOK()) {
return Status(ErrorCodes::BadValue,
"error processing query: " + _query->toString() +
- " planner returned error: " + status.reason());
+ " planner returned error: " + statusWithSolutions.getStatus().reason());
}
+ auto solutions = std::move(statusWithSolutions.getValue());
+
// We cannot figure out how to answer the query. Perhaps it requires an index
// we do not have?
if (0 == solutions.size()) {
@@ -418,8 +415,8 @@ Status SubplanStage::choosePlanWholeQuery(PlanYieldPolicy* yieldPolicy) {
verify(StageBuilder::build(
getOpCtx(), _collection, *_query, *solutions[ix], _ws, &nextPlanRoot));
- // Takes ownership of 'solutions[ix]' and 'nextPlanRoot'.
- multiPlanStage->addPlan(solutions[ix].release(), nextPlanRoot, _ws);
+ // Takes ownership of 'nextPlanRoot'.
+ multiPlanStage->addPlan(std::move(solutions[ix]), nextPlanRoot, _ws);
}
// Delegate the the MultiPlanStage's plan selection facility.
diff --git a/src/mongo/db/query/get_executor.cpp b/src/mongo/db/query/get_executor.cpp
index 44a51597686..e6b921ed459 100644
--- a/src/mongo/db/query/get_executor.cpp
+++ b/src/mongo/db/query/get_executor.cpp
@@ -251,7 +251,6 @@ StatusWith<PrepareExecutionResult> prepareExecution(OperationContext* opCtx,
invariant(canonicalQuery);
unique_ptr<PlanStage> root;
- unique_ptr<QuerySolution> querySolution;
// This can happen as we're called by internal clients as well.
if (NULL == collection) {
@@ -259,8 +258,7 @@ StatusWith<PrepareExecutionResult> prepareExecution(OperationContext* opCtx,
LOG(2) << "Collection " << ns << " does not exist."
<< " Using EOF plan: " << redact(canonicalQuery->toStringShort());
root = make_unique<EOFStage>(opCtx);
- return PrepareExecutionResult(
- std::move(canonicalQuery), std::move(querySolution), std::move(root));
+ return PrepareExecutionResult(std::move(canonicalQuery), nullptr, std::move(root));
}
// Fill out the planning params. We use these for both cached solutions and non-cached.
@@ -324,8 +322,7 @@ StatusWith<PrepareExecutionResult> prepareExecution(OperationContext* opCtx,
root = make_unique<ProjectionStage>(opCtx, params, ws, root.release());
}
- return PrepareExecutionResult(
- std::move(canonicalQuery), std::move(querySolution), std::move(root));
+ return PrepareExecutionResult(std::move(canonicalQuery), nullptr, std::move(root));
}
// Tailable: If the query requests tailable the collection must be capped.
@@ -343,16 +340,18 @@ StatusWith<PrepareExecutionResult> prepareExecution(OperationContext* opCtx,
collection->infoCache()->getPlanCache()->get(*canonicalQuery, &rawCS).isOK()) {
// We have a CachedSolution. Have the planner turn it into a QuerySolution.
unique_ptr<CachedSolution> cs(rawCS);
- QuerySolution* qs;
- Status status = QueryPlanner::planFromCache(*canonicalQuery, plannerParams, *cs, &qs);
+ auto statusWithQs = QueryPlanner::planFromCache(*canonicalQuery, plannerParams, *cs);
- if (status.isOK()) {
- if ((plannerParams.options & QueryPlannerParams::IS_COUNT) && turnIxscanIntoCount(qs)) {
+ if (statusWithQs.isOK()) {
+ auto querySolution = std::move(statusWithQs.getValue());
+ if ((plannerParams.options & QueryPlannerParams::IS_COUNT) &&
+ turnIxscanIntoCount(querySolution.get())) {
LOG(2) << "Using fast count: " << redact(canonicalQuery->toStringShort());
}
PlanStage* rawRoot;
- verify(StageBuilder::build(opCtx, collection, *canonicalQuery, *qs, ws, &rawRoot));
+ verify(StageBuilder::build(
+ opCtx, collection, *canonicalQuery, *querySolution, ws, &rawRoot));
// Add a CachedPlanStage on top of the previous root.
//
@@ -365,7 +364,6 @@ StatusWith<PrepareExecutionResult> prepareExecution(OperationContext* opCtx,
plannerParams,
cs->decisionWorks,
rawRoot);
- querySolution.reset(qs);
return PrepareExecutionResult(
std::move(canonicalQuery), std::move(querySolution), std::move(root));
}
@@ -377,17 +375,16 @@ StatusWith<PrepareExecutionResult> prepareExecution(OperationContext* opCtx,
root =
make_unique<SubplanStage>(opCtx, collection, ws, plannerParams, canonicalQuery.get());
- return PrepareExecutionResult(
- std::move(canonicalQuery), std::move(querySolution), std::move(root));
+ return PrepareExecutionResult(std::move(canonicalQuery), nullptr, std::move(root));
}
- vector<QuerySolution*> solutions;
- Status status = QueryPlanner::plan(*canonicalQuery, plannerParams, &solutions);
- if (!status.isOK()) {
+ auto statusWithSolutions = QueryPlanner::plan(*canonicalQuery, plannerParams);
+ if (!statusWithSolutions.isOK()) {
return Status(ErrorCodes::BadValue,
"error processing query: " + canonicalQuery->toString() +
- " planner returned error: " + status.reason());
+ " planner returned error: " + statusWithSolutions.getStatus().reason());
}
+ auto solutions = std::move(statusWithSolutions.getValue());
// We cannot figure out how to answer the query. Perhaps it requires an index
// we do not have?
@@ -400,14 +397,7 @@ StatusWith<PrepareExecutionResult> prepareExecution(OperationContext* opCtx,
// See if one of our solutions is a fast count hack in disguise.
if (plannerParams.options & QueryPlannerParams::IS_COUNT) {
for (size_t i = 0; i < solutions.size(); ++i) {
- if (turnIxscanIntoCount(solutions[i])) {
- // Great, we can use solutions[i]. Clean up the other QuerySolution(s).
- for (size_t j = 0; j < solutions.size(); ++j) {
- if (j != i) {
- delete solutions[j];
- }
- }
-
+ if (turnIxscanIntoCount(solutions[i].get())) {
// We're not going to cache anything that's fast count.
PlanStage* rawRoot;
verify(StageBuilder::build(
@@ -417,9 +407,8 @@ StatusWith<PrepareExecutionResult> prepareExecution(OperationContext* opCtx,
LOG(2) << "Using fast count: " << redact(canonicalQuery->toStringShort())
<< ", planSummary: " << redact(Explain::getPlanSummary(root.get()));
- querySolution.reset(solutions[i]);
return PrepareExecutionResult(
- std::move(canonicalQuery), std::move(querySolution), std::move(root));
+ std::move(canonicalQuery), std::move(solutions[i]), std::move(root));
}
}
}
@@ -435,9 +424,8 @@ StatusWith<PrepareExecutionResult> prepareExecution(OperationContext* opCtx,
<< redact(canonicalQuery->toStringShort())
<< ", planSummary: " << redact(Explain::getPlanSummary(root.get()));
- querySolution.reset(solutions[0]);
return PrepareExecutionResult(
- std::move(canonicalQuery), std::move(querySolution), std::move(root));
+ std::move(canonicalQuery), std::move(solutions[0]), std::move(root));
} else {
// Many solutions. Create a MultiPlanStage to pick the best, update the cache,
// and so on. The working set will be shared by all candidate plans.
@@ -453,13 +441,12 @@ StatusWith<PrepareExecutionResult> prepareExecution(OperationContext* opCtx,
verify(StageBuilder::build(
opCtx, collection, *canonicalQuery, *solutions[ix], ws, &nextPlanRoot));
- // Owns none of the arguments
- multiPlanStage->addPlan(solutions[ix], nextPlanRoot, ws);
+ // Takes ownership of 'nextPlanRoot'.
+ multiPlanStage->addPlan(std::move(solutions[ix]), nextPlanRoot, ws);
}
root = std::move(multiPlanStage);
- return PrepareExecutionResult(
- std::move(canonicalQuery), std::move(querySolution), std::move(root));
+ return PrepareExecutionResult(std::move(canonicalQuery), nullptr, std::move(root));
}
}
@@ -1577,8 +1564,7 @@ StatusWith<unique_ptr<PlanExecutor, PlanExecutor::Deleter>> getExecutorDistinct(
QueryPlannerParams params;
- unique_ptr<QuerySolution> soln(
- QueryPlannerAnalysis::analyzeDataAccess(*cq, params, std::move(solnRoot)));
+ auto soln = QueryPlannerAnalysis::analyzeDataAccess(*cq, params, std::move(solnRoot));
invariant(soln);
unique_ptr<WorkingSet> ws = make_unique<WorkingSet>();
@@ -1599,25 +1585,18 @@ StatusWith<unique_ptr<PlanExecutor, PlanExecutor::Deleter>> getExecutorDistinct(
}
// See if we can answer the query in a fast-distinct compatible fashion.
- vector<QuerySolution*> solutions;
- Status status = QueryPlanner::plan(*cq, plannerParams, &solutions);
- if (!status.isOK()) {
+ auto statusWithSolutions = QueryPlanner::plan(*cq, plannerParams);
+ if (!statusWithSolutions.isOK()) {
return getExecutor(opCtx, collection, std::move(cq), yieldPolicy);
}
+ auto solutions = std::move(statusWithSolutions.getValue());
// We look for a solution that has an ixscan we can turn into a distinctixscan
for (size_t i = 0; i < solutions.size(); ++i) {
- if (turnIxscanIntoDistinctIxscan(solutions[i], parsedDistinct->getKey())) {
- // Great, we can use solutions[i]. Clean up the other QuerySolution(s).
- for (size_t j = 0; j < solutions.size(); ++j) {
- if (j != i) {
- delete solutions[j];
- }
- }
-
+ if (turnIxscanIntoDistinctIxscan(solutions[i].get(), parsedDistinct->getKey())) {
// Build and return the SSR over solutions[i].
unique_ptr<WorkingSet> ws = make_unique<WorkingSet>();
- unique_ptr<QuerySolution> currentSolution(solutions[i]);
+ unique_ptr<QuerySolution> currentSolution = std::move(solutions[i]);
PlanStage* rawRoot;
verify(
StageBuilder::build(opCtx, collection, *cq, *currentSolution, ws.get(), &rawRoot));
@@ -1637,12 +1616,7 @@ StatusWith<unique_ptr<PlanExecutor, PlanExecutor::Deleter>> getExecutorDistinct(
}
// If we're here, the planner made a soln with the restricted index set but we couldn't
- // translate any of them into a distinct-compatible soln. So, delete the solutions and just
- // go through normal planning.
- for (size_t i = 0; i < solutions.size(); ++i) {
- delete solutions[i];
- }
-
+ // translate any of them into a distinct-compatible soln. Just go through normal planning.
return getExecutor(opCtx, collection, parsedDistinct->releaseQuery(), yieldPolicy);
}
diff --git a/src/mongo/db/query/plan_cache_test.cpp b/src/mongo/db/query/plan_cache_test.cpp
index 19115ccc1cb..44cce150808 100644
--- a/src/mongo/db/query/plan_cache_test.cpp
+++ b/src/mongo/db/query/plan_cache_test.cpp
@@ -479,12 +479,6 @@ protected:
addIndex(BSON("_id" << 1), "_id_");
}
- void tearDown() {
- for (vector<QuerySolution*>::iterator it = solns.begin(); it != solns.end(); ++it) {
- delete *it;
- }
- }
-
void addIndex(BSONObj keyPattern, const std::string& indexName, bool multikey = false) {
// The first false means not multikey.
// The second false means not sparse.
@@ -569,10 +563,6 @@ protected:
auto opCtx = serviceContext.makeOperationContext();
// Clean up any previous state from a call to runQueryFull or runQueryAsCommand.
- for (vector<QuerySolution*>::iterator it = solns.begin(); it != solns.end(); ++it) {
- delete *it;
- }
-
solns.clear();
auto qr = stdx::make_unique<QueryRequest>(nss);
@@ -597,8 +587,9 @@ protected:
ExtensionsCallbackNoop(),
MatchExpressionParser::kAllowAllSpecialFeatures);
ASSERT_OK(statusWithCQ.getStatus());
- Status s = QueryPlanner::plan(*statusWithCQ.getValue(), params, &solns);
- ASSERT_OK(s);
+ auto statusWithSolutions = QueryPlanner::plan(*statusWithCQ.getValue(), params);
+ ASSERT_OK(statusWithSolutions.getStatus());
+ solns = std::move(statusWithSolutions.getValue());
}
void runQueryAsCommand(const BSONObj& cmdObj) {
@@ -606,10 +597,6 @@ protected:
auto opCtx = serviceContext.makeOperationContext();
// Clean up any previous state from a call to runQueryFull or runQueryAsCommand.
- for (vector<QuerySolution*>::iterator it = solns.begin(); it != solns.end(); ++it) {
- delete *it;
- }
-
solns.clear();
const bool isExplain = false;
@@ -624,8 +611,9 @@ protected:
ExtensionsCallbackNoop(),
MatchExpressionParser::kAllowAllSpecialFeatures);
ASSERT_OK(statusWithCQ.getStatus());
- Status s = QueryPlanner::plan(*statusWithCQ.getValue(), params, &solns);
- ASSERT_OK(s);
+ auto statusWithSolutions = QueryPlanner::plan(*statusWithCQ.getValue(), params);
+ ASSERT_OK(statusWithSolutions.getStatus());
+ solns = std::move(statusWithSolutions.getValue());
}
//
@@ -633,8 +621,8 @@ protected:
//
void dumpSolutions(str::stream& ost) const {
- for (vector<QuerySolution*>::const_iterator it = solns.begin(); it != solns.end(); ++it) {
- ost << (*it)->toString() << '\n';
+ for (auto&& soln : solns) {
+ ost << soln->toString() << '\n';
}
}
@@ -644,8 +632,8 @@ protected:
size_t numSolutionMatches(const string& solnJson) const {
BSONObj testSoln = fromjson(solnJson);
size_t matches = 0;
- for (vector<QuerySolution*>::const_iterator it = solns.begin(); it != solns.end(); ++it) {
- QuerySolutionNode* root = (*it)->root.get();
+ for (auto&& soln : solns) {
+ QuerySolutionNode* root = soln->root.get();
if (QueryPlannerTestLib::solutionMatches(testSoln, root)) {
++matches;
}
@@ -673,27 +661,15 @@ protected:
}
/**
- * Plan 'query' from the cache. A mock cache entry is created using
- * the cacheData stored inside the QuerySolution 'soln'.
- *
- * Does not take ownership of 'soln'.
- */
- QuerySolution* planQueryFromCache(const BSONObj& query, const QuerySolution& soln) const {
- return planQueryFromCache(query, BSONObj(), BSONObj(), BSONObj(), soln);
- }
-
- /**
* Plan 'query' from the cache with sort order 'sort', projection 'proj', and collation
* 'collation'. A mock cache entry is created using the cacheData stored inside the
* QuerySolution 'soln'.
- *
- * Does not take ownership of 'soln'.
*/
- QuerySolution* planQueryFromCache(const BSONObj& query,
- const BSONObj& sort,
- const BSONObj& proj,
- const BSONObj& collation,
- const QuerySolution& soln) const {
+ std::unique_ptr<QuerySolution> planQueryFromCache(const BSONObj& query,
+ const BSONObj& sort,
+ const BSONObj& proj,
+ const BSONObj& collation,
+ const QuerySolution& soln) const {
QueryTestServiceContext serviceContext;
auto opCtx = serviceContext.makeOperationContext();
@@ -721,11 +697,9 @@ protected:
PlanCacheEntry entry(solutions, createDecision(1U));
CachedSolution cachedSoln(ck, entry);
- QuerySolution* out;
- Status s = QueryPlanner::planFromCache(*scopedCq, params, cachedSoln, &out);
- ASSERT_OK(s);
-
- return out;
+ auto statusWithQs = QueryPlanner::planFromCache(*scopedCq, params, cachedSoln);
+ ASSERT_OK(statusWithQs.getStatus());
+ return std::move(statusWithQs.getValue());
}
/**
@@ -736,10 +710,10 @@ protected:
*/
QuerySolution* firstMatchingSolution(const string& solnJson) const {
BSONObj testSoln = fromjson(solnJson);
- for (vector<QuerySolution*>::const_iterator it = solns.begin(); it != solns.end(); ++it) {
- QuerySolutionNode* root = (*it)->root.get();
+ for (auto&& soln : solns) {
+ QuerySolutionNode* root = soln->root.get();
if (QueryPlannerTestLib::solutionMatches(testSoln, root)) {
- return *it;
+ return soln.get();
}
}
@@ -791,10 +765,9 @@ protected:
const BSONObj& proj,
const BSONObj& collation,
const string& solnJson) {
- QuerySolution* bestSoln = firstMatchingSolution(solnJson);
- QuerySolution* planSoln = planQueryFromCache(query, sort, proj, collation, *bestSoln);
- assertSolutionMatches(planSoln, solnJson);
- delete planSoln;
+ auto bestSoln = firstMatchingSolution(solnJson);
+ auto planSoln = planQueryFromCache(query, sort, proj, collation, *bestSoln);
+ assertSolutionMatches(planSoln.get(), solnJson);
}
/**
@@ -813,7 +786,7 @@ protected:
BSONObj queryObj;
QueryPlannerParams params;
- vector<QuerySolution*> solns;
+ std::vector<std::unique_ptr<QuerySolution>> solns;
};
const PlanCacheKey CachePlanSelectionTest::ck = "mock_cache_key";
diff --git a/src/mongo/db/query/plan_ranker.h b/src/mongo/db/query/plan_ranker.h
index 30413a37e78..5265eee2422 100644
--- a/src/mongo/db/query/plan_ranker.h
+++ b/src/mongo/db/query/plan_ranker.h
@@ -69,8 +69,8 @@ public:
* Does not own any of its pointers.
*/
struct CandidatePlan {
- CandidatePlan(QuerySolution* s, PlanStage* r, WorkingSet* w)
- : solution(s), root(r), ws(w), failed(false) {}
+ CandidatePlan(std::unique_ptr<QuerySolution> solution, PlanStage* r, WorkingSet* w)
+ : solution(std::move(solution)), root(r), ws(w), failed(false) {}
std::unique_ptr<QuerySolution> solution;
PlanStage* root; // Not owned here.
diff --git a/src/mongo/db/query/planner_analysis.cpp b/src/mongo/db/query/planner_analysis.cpp
index cd11da91262..6e7588841c0 100644
--- a/src/mongo/db/query/planner_analysis.cpp
+++ b/src/mongo/db/query/planner_analysis.cpp
@@ -615,12 +615,11 @@ QuerySolutionNode* QueryPlannerAnalysis::analyzeSort(const CanonicalQuery& query
return solnRoot;
}
-// static
-QuerySolution* QueryPlannerAnalysis::analyzeDataAccess(
+std::unique_ptr<QuerySolution> QueryPlannerAnalysis::analyzeDataAccess(
const CanonicalQuery& query,
const QueryPlannerParams& params,
std::unique_ptr<QuerySolutionNode> solnRoot) {
- unique_ptr<QuerySolution> soln(new QuerySolution());
+ auto soln = stdx::make_unique<QuerySolution>();
soln->filterData = query.getQueryObj();
soln->indexFilterApplied = params.indexFiltersApplied;
@@ -859,7 +858,7 @@ QuerySolution* QueryPlannerAnalysis::analyzeDataAccess(
}
soln->root = std::move(solnRoot);
- return soln.release();
+ return soln;
}
} // namespace mongo
diff --git a/src/mongo/db/query/planner_analysis.h b/src/mongo/db/query/planner_analysis.h
index 67bab4ad88a..62b126a8bd2 100644
--- a/src/mongo/db/query/planner_analysis.h
+++ b/src/mongo/db/query/planner_analysis.h
@@ -64,21 +64,17 @@ public:
* In brief: performs sort and covering analysis.
*
* The solution rooted at 'solnRoot' provides data for the query, whether through some
- * configuration of indices or through a collection scan. Additional stages may be required
- * to perform sorting, projection, or other operations that are independent of the source
- * of the data. These stages are added atop 'solnRoot'.
+ * configuration of indices or through a collection scan. Additional stages may be required to
+ * perform sorting, projection, or other operations that are independent of the source of the
+ * data. These stages are added atop 'solnRoot'.
*
- * 'taggedRoot' is a copy of the parse tree. Nodes in 'solnRoot' may point into it.
- *
- * Takes ownership of 'solnRoot' and 'taggedRoot'.
- *
- * Returns NULL if a solution cannot be constructed given the requirements in 'params'.
- *
- * Caller owns the returned QuerySolution.
+ * Returns a null pointer if a solution cannot be constructed given the requirements in
+ * 'params'.
*/
- static QuerySolution* analyzeDataAccess(const CanonicalQuery& query,
- const QueryPlannerParams& params,
- std::unique_ptr<QuerySolutionNode> solnRoot);
+ static std::unique_ptr<QuerySolution> analyzeDataAccess(
+ const CanonicalQuery& query,
+ const QueryPlannerParams& params,
+ std::unique_ptr<QuerySolutionNode> solnRoot);
/**
* Sort the results, if there is a sort required.
diff --git a/src/mongo/db/query/query_planner.cpp b/src/mongo/db/query/query_planner.cpp
index f56f2976b12..53c916ef32d 100644
--- a/src/mongo/db/query/query_planner.cpp
+++ b/src/mongo/db/query/query_planner.cpp
@@ -268,18 +268,18 @@ static BSONObj finishMaxObj(const IndexEntry& indexEntry,
}
}
-QuerySolution* buildCollscanSoln(const CanonicalQuery& query,
- bool tailable,
- const QueryPlannerParams& params) {
+std::unique_ptr<QuerySolution> buildCollscanSoln(const CanonicalQuery& query,
+ bool tailable,
+ const QueryPlannerParams& params) {
std::unique_ptr<QuerySolutionNode> solnRoot(
QueryPlannerAccess::makeCollectionScan(query, tailable, params));
return QueryPlannerAnalysis::analyzeDataAccess(query, params, std::move(solnRoot));
}
-QuerySolution* buildWholeIXSoln(const IndexEntry& index,
- const CanonicalQuery& query,
- const QueryPlannerParams& params,
- int direction = 1) {
+std::unique_ptr<QuerySolution> buildWholeIXSoln(const IndexEntry& index,
+ const CanonicalQuery& query,
+ const QueryPlannerParams& params,
+ int direction = 1) {
std::unique_ptr<QuerySolutionNode> solnRoot(
QueryPlannerAccess::scanWholeIndex(index, query, params, direction));
return QueryPlannerAnalysis::analyzeDataAccess(query, params, std::move(solnRoot));
@@ -292,17 +292,13 @@ bool providesSort(const CanonicalQuery& query, const BSONObj& kp) {
// static
const int QueryPlanner::kPlannerVersion = 1;
-Status QueryPlanner::cacheDataFromTaggedTree(const MatchExpression* const taggedTree,
- const vector<IndexEntry>& relevantIndices,
- PlanCacheIndexTree** out) {
- // On any early return, the out-parameter must contain NULL.
- *out = NULL;
-
- if (NULL == taggedTree) {
+StatusWith<std::unique_ptr<PlanCacheIndexTree>> QueryPlanner::cacheDataFromTaggedTree(
+ const MatchExpression* const taggedTree, const vector<IndexEntry>& relevantIndices) {
+ if (!taggedTree) {
return Status(ErrorCodes::BadValue, "Cannot produce cache data: tree is NULL.");
}
- unique_ptr<PlanCacheIndexTree> indexTree(new PlanCacheIndexTree());
+ auto indexTree = stdx::make_unique<PlanCacheIndexTree>();
if (taggedTree->getTag() &&
taggedTree->getTag()->getType() == MatchExpression::TagData::Type::IndexTag) {
@@ -359,16 +355,14 @@ Status QueryPlanner::cacheDataFromTaggedTree(const MatchExpression* const tagged
for (size_t i = 0; i < taggedTree->numChildren(); ++i) {
MatchExpression* taggedChild = taggedTree->getChild(i);
- PlanCacheIndexTree* indexTreeChild;
- Status s = cacheDataFromTaggedTree(taggedChild, relevantIndices, &indexTreeChild);
- if (!s.isOK()) {
- return s;
+ auto statusWithTree = cacheDataFromTaggedTree(taggedChild, relevantIndices);
+ if (!statusWithTree.isOK()) {
+ return statusWithTree.getStatus();
}
- indexTree->children.push_back(indexTreeChild);
+ indexTree->children.push_back(statusWithTree.getValue().release());
}
- *out = indexTree.release();
- return Status::OK();
+ return {std::move(indexTree)};
}
// static
@@ -440,13 +434,11 @@ Status QueryPlanner::tagAccordingToCache(MatchExpression* filter,
return Status::OK();
}
-// static
-Status QueryPlanner::planFromCache(const CanonicalQuery& query,
- const QueryPlannerParams& params,
- const CachedSolution& cachedSoln,
- QuerySolution** out) {
+StatusWith<std::unique_ptr<QuerySolution>> QueryPlanner::planFromCache(
+ const CanonicalQuery& query,
+ const QueryPlannerParams& params,
+ const CachedSolution& cachedSoln) {
invariant(!cachedSoln.plannerData.empty());
- invariant(out);
// A query not suitable for caching should not have made its way into the cache.
invariant(PlanCache::shouldCacheQuery(query));
@@ -456,24 +448,22 @@ Status QueryPlanner::planFromCache(const CanonicalQuery& query,
if (SolutionCacheData::WHOLE_IXSCAN_SOLN == winnerCacheData.solnType) {
// The solution can be constructed by a scan over the entire index.
- QuerySolution* soln = buildWholeIXSoln(
+ auto soln = buildWholeIXSoln(
*winnerCacheData.tree->entry, query, params, winnerCacheData.wholeIXSolnDir);
- if (soln == NULL) {
+ if (!soln) {
return Status(ErrorCodes::BadValue,
"plan cache error: soln that uses index to provide sort");
} else {
- *out = soln;
- return Status::OK();
+ return {std::move(soln)};
}
} else if (SolutionCacheData::COLLSCAN_SOLN == winnerCacheData.solnType) {
// The cached solution is a collection scan. We don't cache collscans
// with tailable==true, hence the false below.
- QuerySolution* soln = buildCollscanSoln(query, false, params);
- if (soln == NULL) {
+ auto soln = buildCollscanSoln(query, false, params);
+ if (!soln) {
return Status(ErrorCodes::BadValue, "plan cache error: collection scan soln");
} else {
- *out = soln;
- return Status::OK();
+ return {std::move(soln)};
}
}
@@ -519,9 +509,7 @@ Status QueryPlanner::planFromCache(const CanonicalQuery& query,
<< query.toStringShort());
}
- // Takes ownership of 'solnRoot'.
- QuerySolution* soln =
- QueryPlannerAnalysis::analyzeDataAccess(query, params, std::move(solnRoot));
+ auto soln = QueryPlannerAnalysis::analyzeDataAccess(query, params, std::move(solnRoot));
if (!soln) {
return Status(ErrorCodes::BadValue,
str::stream() << "Failed to analyze plan from cache. Query: "
@@ -529,20 +517,20 @@ Status QueryPlanner::planFromCache(const CanonicalQuery& query,
}
LOG(5) << "Planner: solution constructed from the cache:\n" << redact(soln->toString());
- *out = soln;
- return Status::OK();
+ return {std::move(soln)};
}
// static
-Status QueryPlanner::plan(const CanonicalQuery& query,
- const QueryPlannerParams& params,
- std::vector<QuerySolution*>* out) {
+StatusWith<std::vector<std::unique_ptr<QuerySolution>>> QueryPlanner::plan(
+ const CanonicalQuery& query, const QueryPlannerParams& params) {
LOG(5) << "Beginning planning..." << endl
<< "=============================" << endl
<< "Options = " << optionString(params.options) << endl
<< "Canonical query:" << endl
<< redact(query.toString()) << "=============================";
+ std::vector<std::unique_ptr<QuerySolution>> out;
+
for (size_t i = 0; i < params.indices.size(); ++i) {
LOG(5) << "Index " << i << " is " << params.indices[i].toString();
}
@@ -551,17 +539,15 @@ Status QueryPlanner::plan(const CanonicalQuery& query,
const bool isTailable = query.getQueryRequest().isTailable();
// If the query requests a tailable cursor, the only solution is a collscan + filter with
- // tailable set on the collscan. TODO: This is a policy departure. Previously I think you
- // could ask for a tailable cursor and it just tried to give you one. Now, we fail if we
- // can't provide one. Is this what we want?
+ // tailable set on the collscan.
if (isTailable) {
if (!QueryPlannerCommon::hasNode(query.root(), MatchExpression::GEO_NEAR) && canTableScan) {
- QuerySolution* soln = buildCollscanSoln(query, isTailable, params);
- if (NULL != soln) {
- out->push_back(soln);
+ auto soln = buildCollscanSoln(query, isTailable, params);
+ if (soln) {
+ out.push_back(std::move(soln));
}
}
- return Status::OK();
+ return {std::move(out)};
}
// The hint or sort can be $natural: 1. If this happens, output a collscan. If both
@@ -581,12 +567,12 @@ Status QueryPlanner::plan(const CanonicalQuery& query,
// min/max are incompatible with $natural.
if (canTableScan && query.getQueryRequest().getMin().isEmpty() &&
query.getQueryRequest().getMax().isEmpty()) {
- QuerySolution* soln = buildCollscanSoln(query, isTailable, params);
- if (NULL != soln) {
- out->push_back(soln);
+ auto soln = buildCollscanSoln(query, isTailable, params);
+ if (soln) {
+ out.push_back(std::move(soln));
}
}
- return Status::OK();
+ return {std::move(out)};
}
}
@@ -627,11 +613,11 @@ Status QueryPlanner::plan(const CanonicalQuery& query,
const bool useIXScan = params.options & QueryPlannerParams::SNAPSHOT_USE_ID;
if (!useIXScan) {
- QuerySolution* soln = buildCollscanSoln(query, isTailable, params);
+ auto soln = buildCollscanSoln(query, isTailable, params);
if (soln) {
- out->push_back(soln);
+ out.push_back(std::move(soln));
}
- return Status::OK();
+ return {std::move(out)};
} else {
// Find the ID index in indexKeyPatterns. It's our hint.
for (size_t i = 0; i < params.indices.size(); ++i) {
@@ -775,13 +761,12 @@ Status QueryPlanner::plan(const CanonicalQuery& query,
params.indices[idxNo], query, params, finishedMinObj, finishedMaxObj));
invariant(solnRoot);
- QuerySolution* soln =
- QueryPlannerAnalysis::analyzeDataAccess(query, params, std::move(solnRoot));
- if (NULL != soln) {
- out->push_back(soln);
+ auto soln = QueryPlannerAnalysis::analyzeDataAccess(query, params, std::move(solnRoot));
+ if (soln) {
+ out.push_back(std::move(soln));
}
- return Status::OK();
+ return {std::move(out)};
}
for (size_t i = 0; i < relevantIndices.size(); ++i) {
@@ -872,7 +857,7 @@ Status QueryPlanner::plan(const CanonicalQuery& query,
isp.init().transitional_ignore();
unique_ptr<MatchExpression> rawTree;
- while ((rawTree = isp.getNext()) && (out->size() < params.maxIndexedSolutions)) {
+ while ((rawTree = isp.getNext()) && (out.size() < params.maxIndexedSolutions)) {
LOG(5) << "About to build solntree from tagged tree:" << endl
<< redact(rawTree.get()->toString());
@@ -880,13 +865,14 @@ Status QueryPlanner::plan(const CanonicalQuery& query,
// the PlanCacheIndexTree has the same sort as the MatchExpression used to generate the
// plan cache key.
std::unique_ptr<MatchExpression> clone(rawTree.get()->shallowClone());
- PlanCacheIndexTree* cacheData;
- Status indexTreeStatus =
- cacheDataFromTaggedTree(clone.get(), relevantIndices, &cacheData);
- if (!indexTreeStatus.isOK()) {
- LOG(5) << "Query is not cachable: " << redact(indexTreeStatus.reason());
+ std::unique_ptr<PlanCacheIndexTree> cacheData;
+ auto statusWithCacheData = cacheDataFromTaggedTree(clone.get(), relevantIndices);
+ if (!statusWithCacheData.isOK()) {
+ LOG(5) << "Query is not cachable: "
+ << redact(statusWithCacheData.getStatus().reason());
+ } else {
+ cacheData = std::move(statusWithCacheData.getValue());
}
- unique_ptr<PlanCacheIndexTree> autoData(cacheData);
// We have already cached the tree in canonical order, so now we can order the nodes for
// access planning.
@@ -900,16 +886,15 @@ Status QueryPlanner::plan(const CanonicalQuery& query,
continue;
}
- QuerySolution* soln =
- QueryPlannerAnalysis::analyzeDataAccess(query, params, std::move(solnRoot));
- if (NULL != soln) {
+ auto soln = QueryPlannerAnalysis::analyzeDataAccess(query, params, std::move(solnRoot));
+ if (soln) {
LOG(5) << "Planner: adding solution:" << endl << redact(soln->toString());
- if (indexTreeStatus.isOK()) {
+ if (statusWithCacheData.isOK()) {
SolutionCacheData* scd = new SolutionCacheData();
- scd->tree.reset(autoData.release());
+ scd->tree = std::move(cacheData);
soln->cacheData.reset(scd);
}
- out->push_back(soln);
+ out.push_back(std::move(soln));
}
}
}
@@ -917,11 +902,11 @@ Status QueryPlanner::plan(const CanonicalQuery& query,
// Don't leave tags on query tree.
query.root()->resetTag();
- LOG(5) << "Planner: outputted " << out->size() << " indexed solutions.";
+ LOG(5) << "Planner: outputted " << out.size() << " indexed solutions.";
// Produce legible error message for failed OR planning with a TEXT child.
// TODO: support collection scan for non-TEXT children of OR.
- if (out->size() == 0 && textNode != NULL && MatchExpression::OR == query.root()->matchType()) {
+ if (out.size() == 0 && textNode != NULL && MatchExpression::OR == query.root()->matchType()) {
MatchExpression* root = query.root();
for (size_t i = 0; i < root->numChildren(); ++i) {
if (textNode == root->getChild(i)) {
@@ -936,16 +921,17 @@ Status QueryPlanner::plan(const CanonicalQuery& query,
// scan the entire index to provide results and output that as our plan. This is the
// desired behavior when an index is hinted that is not relevant to the query.
if (!hintIndex.isEmpty()) {
- if (0 == out->size()) {
+ if (0 == out.size()) {
// 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)) {
+ auto soln = buildWholeIXSoln(params.indices[*hintIndexNumber], query, params);
+ if (soln) {
LOG(5) << "Planner: outputting soln that uses hinted index as scan.";
- out->push_back(soln);
+ out.push_back(std::move(soln));
}
}
- return Status::OK();
+ return {std::move(out)};
}
// If a sort order is requested, there may be an index that provides it, even if that
@@ -957,8 +943,8 @@ Status QueryPlanner::plan(const CanonicalQuery& query,
// See if we have a sort provided from an index already.
// This is implied by the presence of a non-blocking solution.
bool usingIndexToSort = false;
- for (size_t i = 0; i < out->size(); ++i) {
- QuerySolution* soln = (*out)[i];
+ for (size_t i = 0; i < out.size(); ++i) {
+ auto soln = out[i].get();
if (!soln->hasBlockingStage) {
usingIndexToSort = true;
break;
@@ -1004,8 +990,8 @@ Status QueryPlanner::plan(const CanonicalQuery& query,
const BSONObj kp = QueryPlannerAnalysis::getSortPattern(index.keyPattern);
if (providesSort(query, kp)) {
LOG(5) << "Planner: outputting soln that uses index to provide sort.";
- QuerySolution* soln = buildWholeIXSoln(params.indices[i], query, params);
- if (NULL != soln) {
+ auto soln = buildWholeIXSoln(params.indices[i], query, params);
+ if (soln) {
PlanCacheIndexTree* indexTree = new PlanCacheIndexTree();
indexTree->setIndexEntry(params.indices[i]);
SolutionCacheData* scd = new SolutionCacheData();
@@ -1014,15 +1000,15 @@ Status QueryPlanner::plan(const CanonicalQuery& query,
scd->wholeIXSolnDir = 1;
soln->cacheData.reset(scd);
- out->push_back(soln);
+ out.push_back(std::move(soln));
break;
}
}
if (providesSort(query, QueryPlannerCommon::reverseSortObj(kp))) {
LOG(5) << "Planner: outputting soln that uses (reverse) index "
<< "to provide sort.";
- QuerySolution* soln = buildWholeIXSoln(params.indices[i], query, params, -1);
- if (NULL != soln) {
+ auto soln = buildWholeIXSoln(params.indices[i], query, params, -1);
+ if (soln) {
PlanCacheIndexTree* indexTree = new PlanCacheIndexTree();
indexTree->setIndexEntry(params.indices[i]);
SolutionCacheData* scd = new SolutionCacheData();
@@ -1031,7 +1017,7 @@ Status QueryPlanner::plan(const CanonicalQuery& query,
scd->wholeIXSolnDir = -1;
soln->cacheData.reset(scd);
- out->push_back(soln);
+ out.push_back(std::move(soln));
break;
}
}
@@ -1042,7 +1028,7 @@ Status QueryPlanner::plan(const CanonicalQuery& query,
// If a projection exists, there may be an index that allows for a covered plan, even if none
// were considered earlier.
const auto projection = query.getProj();
- if (params.options & QueryPlannerParams::GENERATE_COVERED_IXSCANS && out->size() == 0 &&
+ if (params.options & QueryPlannerParams::GENERATE_COVERED_IXSCANS && out.size() == 0 &&
query.getQueryObj().isEmpty() && projection && !projection->requiresDocument()) {
const auto* indicesToConsider = hintIndex.isEmpty() ? &params.indices : &relevantIndices;
@@ -1067,7 +1053,7 @@ Status QueryPlanner::plan(const CanonicalQuery& query,
scd->wholeIXSolnDir = 1;
soln->cacheData.reset(scd);
- out->push_back(soln);
+ out.push_back(std::move(soln));
break;
}
}
@@ -1083,20 +1069,20 @@ Status QueryPlanner::plan(const CanonicalQuery& query,
bool collscanRequested = (params.options & QueryPlannerParams::INCLUDE_COLLSCAN);
// No indexed plans? We must provide a collscan if possible or else we can't run the query.
- bool collscanNeeded = (0 == out->size() && canTableScan);
+ bool collscanNeeded = (0 == out.size() && canTableScan);
if (possibleToCollscan && (collscanRequested || collscanNeeded)) {
- QuerySolution* collscan = buildCollscanSoln(query, isTailable, params);
- if (NULL != collscan) {
+ auto collscan = buildCollscanSoln(query, isTailable, params);
+ if (collscan) {
+ LOG(5) << "Planner: outputting a collscan:" << endl << redact(collscan->toString());
SolutionCacheData* scd = new SolutionCacheData();
scd->solnType = SolutionCacheData::COLLSCAN_SOLN;
collscan->cacheData.reset(scd);
- out->push_back(collscan);
- LOG(5) << "Planner: outputting a collscan:" << endl << redact(collscan->toString());
+ out.push_back(std::move(collscan));
}
}
- return Status::OK();
+ return {std::move(out)};
}
} // namespace mongo
diff --git a/src/mongo/db/query/query_planner.h b/src/mongo/db/query/query_planner.h
index da4e7225541..ce11efaf7ba 100644
--- a/src/mongo/db/query/query_planner.h
+++ b/src/mongo/db/query/query_planner.h
@@ -48,50 +48,35 @@ public:
static const int kPlannerVersion;
/**
- * Outputs a series of possible solutions for the provided 'query' into 'out'. Uses the
- * indices and other data in 'params' to plan with.
- *
- * Caller owns pointers in *out.
+ * Returns the list of possible query solutions for the provided 'query'. Uses the indices and
+ * other data in 'params' to determine the set of available plans.
*/
- static Status plan(const CanonicalQuery& query,
- const QueryPlannerParams& params,
- std::vector<QuerySolution*>* out);
+ static StatusWith<std::vector<std::unique_ptr<QuerySolution>>> plan(
+ const CanonicalQuery& query, const QueryPlannerParams& params);
/**
- * Attempt to generate a query solution, given data retrieved
- * from the plan cache.
+ * Generates and returns a query solution, given data retrieved from the plan cache.
*
* @param query -- query for which we are generating a plan
* @param params -- planning parameters
* @param cachedSoln -- the CachedSolution retrieved from the plan cache.
- * @param out -- an out-parameter which will be filled in with the solution
- * generated from the cache data
- *
- * On success, the caller is responsible for deleting *out.
*/
- static Status planFromCache(const CanonicalQuery& query,
- const QueryPlannerParams& params,
- const CachedSolution& cachedSoln,
- QuerySolution** out);
+ static StatusWith<std::unique_ptr<QuerySolution>> planFromCache(
+ const CanonicalQuery& query,
+ const QueryPlannerParams& params,
+ const CachedSolution& cachedSoln);
/**
- * Used to generated the index tag tree that will be inserted
- * into the plan cache. This data gets stashed inside a QuerySolution
- * until it can be inserted into the cache proper.
+ * Generates and returns the index tag tree that will be inserted into the plan cache. This data
+ * gets stashed inside a QuerySolution until it can be inserted into the cache proper.
*
* @param taggedTree -- a MatchExpression with index tags that has been
* produced by the enumerator.
* @param relevantIndices -- a list of the index entries used to tag
* the tree (i.e. index numbers in the tags refer to entries in this vector)
- *
- * On success, a new tagged tree is returned through the out-parameter 'out'.
- * The caller has ownership of both taggedTree and *out.
- *
- * On failure, 'out' is set to NULL.
*/
- static Status cacheDataFromTaggedTree(const MatchExpression* const taggedTree,
- const std::vector<IndexEntry>& relevantIndices,
- PlanCacheIndexTree** out);
+ static StatusWith<std::unique_ptr<PlanCacheIndexTree>> cacheDataFromTaggedTree(
+ const MatchExpression* const taggedTree, const std::vector<IndexEntry>& relevantIndices);
/**
* @param filter -- an untagged MatchExpression
diff --git a/src/mongo/db/query/query_planner_test.cpp b/src/mongo/db/query/query_planner_test.cpp
index 8ac5cbd0c06..fe9af7b381c 100644
--- a/src/mongo/db/query/query_planner_test.cpp
+++ b/src/mongo/db/query/query_planner_test.cpp
@@ -4322,13 +4322,9 @@ TEST_F(QueryPlannerTest, KeyPatternOverflowsInt) {
//
TEST_F(QueryPlannerTest, CacheDataFromTaggedTreeFailsOnBadInput) {
- PlanCacheIndexTree* indexTree;
-
// Null match expression.
std::vector<IndexEntry> relevantIndices;
- Status s = QueryPlanner::cacheDataFromTaggedTree(NULL, relevantIndices, &indexTree);
- ASSERT_NOT_OK(s);
- ASSERT(NULL == indexTree);
+ ASSERT_NOT_OK(QueryPlanner::cacheDataFromTaggedTree(NULL, relevantIndices).getStatus());
// No relevant index matching the index tag.
relevantIndices.push_back(IndexEntry(BSON("a" << 1)));
@@ -4340,9 +4336,8 @@ TEST_F(QueryPlannerTest, CacheDataFromTaggedTreeFailsOnBadInput) {
std::unique_ptr<CanonicalQuery> scopedCq = std::move(statusWithCQ.getValue());
scopedCq->root()->setTag(new IndexTag(1));
- s = QueryPlanner::cacheDataFromTaggedTree(scopedCq->root(), relevantIndices, &indexTree);
- ASSERT_NOT_OK(s);
- ASSERT(NULL == indexTree);
+ ASSERT_NOT_OK(
+ QueryPlanner::cacheDataFromTaggedTree(scopedCq->root(), relevantIndices).getStatus());
}
TEST_F(QueryPlannerTest, TagAccordingToCacheFailsOnBadInput) {
diff --git a/src/mongo/db/query/query_planner_test_fixture.cpp b/src/mongo/db/query/query_planner_test_fixture.cpp
index 5ba4eb2648e..a4e937a8ca3 100644
--- a/src/mongo/db/query/query_planner_test_fixture.cpp
+++ b/src/mongo/db/query/query_planner_test_fixture.cpp
@@ -259,9 +259,9 @@ void QueryPlannerTest::runQueryFull(const BSONObj& query,
ASSERT_OK(statusWithCQ.getStatus());
cq = std::move(statusWithCQ.getValue());
- std::vector<QuerySolution*> solnsRaw;
- ASSERT_OK(QueryPlanner::plan(*cq, params, &solnsRaw));
- solns = transitional_tools_do_not_use::spool_vector(solnsRaw);
+ auto statusWithSolutions = QueryPlanner::plan(*cq, params);
+ ASSERT_OK(statusWithSolutions.getStatus());
+ solns = std::move(statusWithSolutions.getValue());
}
void QueryPlannerTest::runInvalidQuery(const BSONObj& query) {
@@ -343,10 +343,8 @@ void QueryPlannerTest::runInvalidQueryFull(const BSONObj& query,
ASSERT_OK(statusWithCQ.getStatus());
cq = std::move(statusWithCQ.getValue());
- std::vector<QuerySolution*> solnsRaw;
- Status s = QueryPlanner::plan(*cq, params, &solnsRaw);
- solns = transitional_tools_do_not_use::spool_vector(solnsRaw);
- ASSERT_NOT_OK(s);
+ auto statusWithSolutions = QueryPlanner::plan(*cq, params);
+ ASSERT_NOT_OK(statusWithSolutions.getStatus());
}
void QueryPlannerTest::runQueryAsCommand(const BSONObj& cmdObj) {
@@ -369,10 +367,9 @@ void QueryPlannerTest::runQueryAsCommand(const BSONObj& cmdObj) {
ASSERT_OK(statusWithCQ.getStatus());
cq = std::move(statusWithCQ.getValue());
- std::vector<QuerySolution*> solnsRaw;
- Status s = QueryPlanner::plan(*cq, params, &solnsRaw);
- solns = transitional_tools_do_not_use::spool_vector(solnsRaw);
- ASSERT_OK(s);
+ auto statusWithSolutions = QueryPlanner::plan(*cq, params);
+ ASSERT_OK(statusWithSolutions.getStatus());
+ solns = std::move(statusWithSolutions.getValue());
}
void QueryPlannerTest::runInvalidQueryAsCommand(const BSONObj& cmdObj) {
@@ -395,10 +392,8 @@ void QueryPlannerTest::runInvalidQueryAsCommand(const BSONObj& cmdObj) {
ASSERT_OK(statusWithCQ.getStatus());
cq = std::move(statusWithCQ.getValue());
- std::vector<QuerySolution*> solnsRaw;
- Status status = QueryPlanner::plan(*cq, params, &solnsRaw);
- solns = transitional_tools_do_not_use::spool_vector(solnsRaw);
- ASSERT_NOT_OK(status);
+ auto statusWithSolutions = QueryPlanner::plan(*cq, params);
+ ASSERT_NOT_OK(statusWithSolutions.getStatus());
}
size_t QueryPlannerTest::getNumSolutions() const {
diff --git a/src/mongo/dbtests/plan_ranking.cpp b/src/mongo/dbtests/plan_ranking.cpp
index 80399f73b06..affc2bca9d0 100644
--- a/src/mongo/dbtests/plan_ranking.cpp
+++ b/src/mongo/dbtests/plan_ranking.cpp
@@ -115,9 +115,9 @@ public:
plannerParams.options &= ~QueryPlannerParams::KEEP_MUTATIONS;
// Plan.
- vector<QuerySolution*> solutions;
- Status status = QueryPlanner::plan(*cq, plannerParams, &solutions);
- ASSERT(status.isOK());
+ auto statusWithSolutions = QueryPlanner::plan(*cq, plannerParams);
+ ASSERT_OK(statusWithSolutions.getStatus());
+ auto solutions = std::move(statusWithSolutions.getValue());
ASSERT_GREATER_THAN_OR_EQUALS(solutions.size(), 1U);
@@ -128,8 +128,8 @@ public:
for (size_t i = 0; i < solutions.size(); ++i) {
PlanStage* root;
ASSERT(StageBuilder::build(&_opCtx, collection, *cq, *solutions[i], ws.get(), &root));
- // Takes ownership of all (actually some) arguments.
- _mps->addPlan(solutions[i], root, ws.get());
+ // Takes ownership of 'root'.
+ _mps->addPlan(std::move(solutions[i]), root, ws.get());
}
// This is what sets a backup plan, should we test for it.
PlanYieldPolicy yieldPolicy(PlanExecutor::NO_YIELD,
diff --git a/src/mongo/dbtests/query_stage_multiplan.cpp b/src/mongo/dbtests/query_stage_multiplan.cpp
index 4a48984871a..7a5629b7117 100644
--- a/src/mongo/dbtests/query_stage_multiplan.cpp
+++ b/src/mongo/dbtests/query_stage_multiplan.cpp
@@ -70,15 +70,12 @@ using stdx::make_unique;
static const NamespaceString nss("unittests.QueryStageMultiPlan");
-/**
- * Create query solution.
- */
-QuerySolution* createQuerySolution() {
- unique_ptr<QuerySolution> soln(new QuerySolution());
- soln->cacheData.reset(new SolutionCacheData());
+std::unique_ptr<QuerySolution> createQuerySolution() {
+ auto soln = stdx::make_unique<QuerySolution>();
+ soln->cacheData = stdx::make_unique<SolutionCacheData>();
soln->cacheData->solnType = SolutionCacheData::COLLSCAN_SOLN;
- soln->cacheData->tree.reset(new PlanCacheIndexTree());
- return soln.release();
+ soln->cacheData->tree = stdx::make_unique<PlanCacheIndexTree>();
+ return soln;
}
class QueryStageMultiPlanTest : public unittest::Test {
@@ -247,9 +244,9 @@ TEST_F(QueryStageMultiPlanTest, MPSBackupPlan) {
plannerParams.options &= ~QueryPlannerParams::KEEP_MUTATIONS;
// Plan.
- vector<QuerySolution*> solutions;
- Status status = QueryPlanner::plan(*cq, plannerParams, &solutions);
- ASSERT(status.isOK());
+ auto statusWithSolutions = QueryPlanner::plan(*cq, plannerParams);
+ ASSERT_OK(statusWithSolutions.getStatus());
+ auto solutions = std::move(statusWithSolutions.getValue());
// We expect a plan using index {a: 1} and plan using index {b: 1} and
// an index intersection plan.
@@ -262,8 +259,8 @@ TEST_F(QueryStageMultiPlanTest, MPSBackupPlan) {
for (size_t i = 0; i < solutions.size(); ++i) {
PlanStage* root;
ASSERT(StageBuilder::build(_opCtx.get(), collection, *cq, *solutions[i], ws.get(), &root));
- // Takes ownership of 'solutions[i]' and 'root'.
- mps->addPlan(solutions[i], root, ws.get());
+ // Takes ownership of 'root'.
+ mps->addPlan(std::move(solutions[i]), root, ws.get());
}
// This sets a backup plan.
@@ -349,10 +346,8 @@ TEST_F(QueryStageMultiPlanTest, MPSExplainAllPlans) {
make_unique<MultiPlanStage>(_opCtx.get(), ctx.getCollection(), cq.get());
// Put each plan into the MultiPlanStage. Takes ownership of 'firstPlan' and 'secondPlan'.
- auto firstSoln = stdx::make_unique<QuerySolution>();
- auto secondSoln = stdx::make_unique<QuerySolution>();
- mps->addPlan(firstSoln.release(), firstPlan.release(), ws.get());
- mps->addPlan(secondSoln.release(), secondPlan.release(), ws.get());
+ mps->addPlan(stdx::make_unique<QuerySolution>(), firstPlan.release(), ws.get());
+ mps->addPlan(stdx::make_unique<QuerySolution>(), secondPlan.release(), ws.get());
// Making a PlanExecutor chooses the best plan.
auto exec = uassertStatusOK(PlanExecutor::make(
diff --git a/src/mongo/s/chunk_manager.cpp b/src/mongo/s/chunk_manager.cpp
index 9445aa7b1da..1644a79782c 100644
--- a/src/mongo/s/chunk_manager.cpp
+++ b/src/mongo/s/chunk_manager.cpp
@@ -237,17 +237,15 @@ IndexBounds ChunkManager::getIndexBoundsForQuery(const BSONObj& key,
NULL /* collator */);
plannerParams.indices.push_back(indexEntry);
- OwnedPointerVector<QuerySolution> solutions;
- Status status = QueryPlanner::plan(canonicalQuery, plannerParams, &solutions.mutableVector());
- uassert(status.code(), status.reason(), status.isOK());
+ auto statusWithSolutions = QueryPlanner::plan(canonicalQuery, plannerParams);
+ uassertStatusOK(statusWithSolutions.getStatus());
+ auto solutions = std::move(statusWithSolutions.getValue());
IndexBounds bounds;
- for (std::vector<QuerySolution*>::const_iterator it = solutions.begin();
- bounds.size() == 0 && it != solutions.end();
- it++) {
+ for (auto&& soln : solutions) {
// Try next solution if we failed to generate index bounds, i.e. bounds.size() == 0
- bounds = collapseQuerySolution((*it)->root.get());
+ bounds = collapseQuerySolution(soln->root.get());
}
if (bounds.size() == 0) {