diff options
author | David Storch <david.storch@10gen.com> | 2018-01-09 17:57:41 -0500 |
---|---|---|
committer | David Storch <david.storch@10gen.com> | 2018-01-11 18:43:25 -0500 |
commit | e377025071e4c9fff336fc57741e5cc8511607af (patch) | |
tree | d34bb3208881fecce2d41b1905fe34d849f1d92c | |
parent | c16959f19705772d01e1053899048869aafe4537 (diff) | |
download | mongo-e377025071e4c9fff336fc57741e5cc8511607af.tar.gz |
SERVER-32603 Modernize QueryPlanner methods to use unique_ptr.
-rw-r--r-- | src/mongo/db/exec/cached_plan.cpp | 14 | ||||
-rw-r--r-- | src/mongo/db/exec/multi_plan.cpp | 6 | ||||
-rw-r--r-- | src/mongo/db/exec/multi_plan.h | 4 | ||||
-rw-r--r-- | src/mongo/db/exec/subplan.cpp | 35 | ||||
-rw-r--r-- | src/mongo/db/query/get_executor.cpp | 80 | ||||
-rw-r--r-- | src/mongo/db/query/plan_cache_test.cpp | 77 | ||||
-rw-r--r-- | src/mongo/db/query/plan_ranker.h | 4 | ||||
-rw-r--r-- | src/mongo/db/query/planner_analysis.cpp | 7 | ||||
-rw-r--r-- | src/mongo/db/query/planner_analysis.h | 22 | ||||
-rw-r--r-- | src/mongo/db/query/query_planner.cpp | 182 | ||||
-rw-r--r-- | src/mongo/db/query/query_planner.h | 41 | ||||
-rw-r--r-- | src/mongo/db/query/query_planner_test.cpp | 11 | ||||
-rw-r--r-- | src/mongo/db/query/query_planner_test_fixture.cpp | 25 | ||||
-rw-r--r-- | src/mongo/dbtests/plan_ranking.cpp | 10 | ||||
-rw-r--r-- | src/mongo/dbtests/query_stage_multiplan.cpp | 29 | ||||
-rw-r--r-- | src/mongo/s/chunk_manager.cpp | 12 |
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() ? ¶ms.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) { |