diff options
Diffstat (limited to 'src')
43 files changed, 734 insertions, 789 deletions
diff --git a/src/mongo/db/commands/find_cmd.cpp b/src/mongo/db/commands/find_cmd.cpp index d40ff0f766e..f86fbbe8b34 100644 --- a/src/mongo/db/commands/find_cmd.cpp +++ b/src/mongo/db/commands/find_cmd.cpp @@ -129,17 +129,14 @@ public: } // Finish the parsing step by using the LiteParsedQuery to create a CanonicalQuery. - std::unique_ptr<CanonicalQuery> cq; - { - CanonicalQuery* rawCq; - WhereCallbackReal whereCallback(txn, nss.db()); - Status canonStatus = - CanonicalQuery::canonicalize(lpqStatus.getValue().release(), &rawCq, whereCallback); - if (!canonStatus.isOK()) { - return canonStatus; - } - cq.reset(rawCq); + + WhereCallbackReal whereCallback(txn, nss.db()); + auto statusWithCQ = + CanonicalQuery::canonicalize(lpqStatus.getValue().release(), whereCallback); + if (!statusWithCQ.isOK()) { + return statusWithCQ.getStatus(); } + std::unique_ptr<CanonicalQuery> cq = std::move(statusWithCQ.getValue()); AutoGetCollectionForRead ctx(txn, nss); // The collection may be NULL. If so, getExecutor() should handle it by returning @@ -214,16 +211,12 @@ public: beginQueryOp(txn, nss, cmdObj, ntoreturn, lpq->getSkip()); // 1b) Finish the parsing step by using the LiteParsedQuery to create a CanonicalQuery. - std::unique_ptr<CanonicalQuery> cq; - { - CanonicalQuery* rawCq; - WhereCallbackReal whereCallback(txn, nss.db()); - Status canonStatus = CanonicalQuery::canonicalize(lpq.release(), &rawCq, whereCallback); - if (!canonStatus.isOK()) { - return appendCommandStatus(result, canonStatus); - } - cq.reset(rawCq); + WhereCallbackReal whereCallback(txn, nss.db()); + auto statusWithCQ = CanonicalQuery::canonicalize(lpq.release(), whereCallback); + if (!statusWithCQ.isOK()) { + return appendCommandStatus(result, statusWithCQ.getStatus()); } + std::unique_ptr<CanonicalQuery> cq = std::move(statusWithCQ.getValue()); // 2) Acquire locks. AutoGetCollectionForRead ctx(txn, nss); diff --git a/src/mongo/db/commands/geo_near_cmd.cpp b/src/mongo/db/commands/geo_near_cmd.cpp index 5be50f433f6..df8fd3bdee4 100644 --- a/src/mongo/db/commands/geo_near_cmd.cpp +++ b/src/mongo/db/commands/geo_near_cmd.cpp @@ -181,22 +181,22 @@ public: BSONObj projObj = BSON("$pt" << BSON("$meta" << LiteParsedQuery::metaGeoNearPoint) << "$dis" << BSON("$meta" << LiteParsedQuery::metaGeoNearDistance)); - CanonicalQuery* cq; const WhereCallbackReal whereCallback(txn, nss.db()); - - if (!CanonicalQuery::canonicalize( - nss, rewritten, BSONObj(), projObj, 0, numWanted, BSONObj(), &cq, whereCallback) - .isOK()) { + auto statusWithCQ = CanonicalQuery::canonicalize( + nss, rewritten, BSONObj(), projObj, 0, numWanted, BSONObj(), whereCallback); + if (!statusWithCQ.isOK()) { errmsg = "Can't parse filter / create query"; return false; } + unique_ptr<CanonicalQuery> cq = std::move(statusWithCQ.getValue()); // Prevent chunks from being cleaned up during yields - this allows us to only check the // version on initial entry into geoNear. RangePreserver preserver(collection); PlanExecutor* rawExec; - if (!getExecutor(txn, collection, cq, PlanExecutor::YIELD_AUTO, &rawExec, 0).isOK()) { + if (!getExecutor(txn, collection, cq.release(), PlanExecutor::YIELD_AUTO, &rawExec, 0) + .isOK()) { errmsg = "can't get query executor"; return false; } diff --git a/src/mongo/db/commands/index_filter_commands.cpp b/src/mongo/db/commands/index_filter_commands.cpp index 20783c5b244..0b6345ef06a 100644 --- a/src/mongo/db/commands/index_filter_commands.cpp +++ b/src/mongo/db/commands/index_filter_commands.cpp @@ -268,13 +268,12 @@ Status ClearFilters::clear(OperationContext* txn, // - clear hints for single query shape when a query shape is described in the // command arguments. if (cmdObj.hasField("query")) { - CanonicalQuery* cqRaw; - Status status = PlanCacheCommand::canonicalize(txn, ns, cmdObj, &cqRaw); - if (!status.isOK()) { - return status; + auto statusWithCQ = PlanCacheCommand::canonicalize(txn, ns, cmdObj); + if (!statusWithCQ.isOK()) { + return statusWithCQ.getStatus(); } - unique_ptr<CanonicalQuery> cq(cqRaw); + unique_ptr<CanonicalQuery> cq = std::move(statusWithCQ.getValue()); querySettings->removeAllowedIndices(planCache->computeKey(*cq)); // Remove entry from plan cache @@ -315,11 +314,10 @@ Status ClearFilters::clear(OperationContext* txn, invariant(entry); // Create canonical query. - CanonicalQuery* cqRaw; - Status result = CanonicalQuery::canonicalize( - ns, entry->query, entry->sort, entry->projection, &cqRaw, whereCallback); - invariant(result.isOK()); - unique_ptr<CanonicalQuery> cq(cqRaw); + auto statusWithCQ = CanonicalQuery::canonicalize( + ns, entry->query, entry->sort, entry->projection, whereCallback); + invariant(statusWithCQ.isOK()); + std::unique_ptr<CanonicalQuery> cq = std::move(statusWithCQ.getValue()); // Remove plan cache entry. planCache->remove(*cq); @@ -384,12 +382,11 @@ Status SetFilter::set(OperationContext* txn, indexes.push_back(obj.getOwned()); } - CanonicalQuery* cqRaw; - Status status = PlanCacheCommand::canonicalize(txn, ns, cmdObj, &cqRaw); - if (!status.isOK()) { - return status; + auto statusWithCQ = PlanCacheCommand::canonicalize(txn, ns, cmdObj); + if (!statusWithCQ.isOK()) { + return statusWithCQ.getStatus(); } - unique_ptr<CanonicalQuery> cq(cqRaw); + unique_ptr<CanonicalQuery> cq = std::move(statusWithCQ.getValue()); // Add allowed indices to query settings, overriding any previous entries. querySettings->setAllowedIndices(*cq, planCache->computeKey(*cq), indexes); diff --git a/src/mongo/db/commands/index_filter_commands_test.cpp b/src/mongo/db/commands/index_filter_commands_test.cpp index ba6f6c87e6c..7da3619d1f7 100644 --- a/src/mongo/db/commands/index_filter_commands_test.cpp +++ b/src/mongo/db/commands/index_filter_commands_test.cpp @@ -120,9 +120,9 @@ void addQueryShapeToPlanCache(PlanCache* planCache, BSONObj projectionObj = fromjson(projectionStr); // Create canonical query. - CanonicalQuery* cqRaw; - ASSERT_OK(CanonicalQuery::canonicalize(ns, queryObj, sortObj, projectionObj, &cqRaw)); - unique_ptr<CanonicalQuery> cq(cqRaw); + auto statusWithCQ = CanonicalQuery::canonicalize(ns, queryObj, sortObj, projectionObj); + ASSERT_OK(statusWithCQ.getStatus()); + std::unique_ptr<CanonicalQuery> cq = std::move(statusWithCQ.getValue()); QuerySolution qs; qs.cacheData.reset(new SolutionCacheData()); @@ -144,9 +144,9 @@ bool planCacheContains(const PlanCache& planCache, BSONObj projectionObj = fromjson(projectionStr); // Create canonical query. - CanonicalQuery* cqRaw; - ASSERT_OK(CanonicalQuery::canonicalize(ns, queryObj, sortObj, projectionObj, &cqRaw)); - unique_ptr<CanonicalQuery> cq(cqRaw); + auto statusWithInputQuery = CanonicalQuery::canonicalize(ns, queryObj, sortObj, projectionObj); + ASSERT_OK(statusWithInputQuery.getStatus()); + unique_ptr<CanonicalQuery> inputQuery = std::move(statusWithInputQuery.getValue()); // Retrieve cache entries from plan cache. vector<PlanCacheEntry*> entries = planCache.getAllEntries(); @@ -159,11 +159,12 @@ bool planCacheContains(const PlanCache& planCache, // Canonicalizing query shape in cache entry to get cache key. // Alternatively, we could add key to PlanCacheEntry but that would be used in one place // only. - ASSERT_OK( - CanonicalQuery::canonicalize(ns, entry->query, entry->sort, entry->projection, &cqRaw)); - unique_ptr<CanonicalQuery> currentQuery(cqRaw); + auto statusWithCurrentQuery = + CanonicalQuery::canonicalize(ns, entry->query, entry->sort, entry->projection); + ASSERT_OK(statusWithCurrentQuery.getStatus()); + unique_ptr<CanonicalQuery> currentQuery = std::move(statusWithCurrentQuery.getValue()); - if (planCache.computeKey(*currentQuery) == planCache.computeKey(*cq)) { + if (planCache.computeKey(*currentQuery) == planCache.computeKey(*inputQuery)) { found = true; } // Release resources for cache entry after extracting key. diff --git a/src/mongo/db/commands/mr.cpp b/src/mongo/db/commands/mr.cpp index f32ebe3b8d8..fa4add10df0 100644 --- a/src/mongo/db/commands/mr.cpp +++ b/src/mongo/db/commands/mr.cpp @@ -1004,10 +1004,10 @@ void State::finalReduce(CurOp* op, ProgressMeterHolder& pm) { const NamespaceString nss(_config.incLong); const WhereCallbackReal whereCallback(_txn, nss.db()); - CanonicalQuery* cqRaw; - verify(CanonicalQuery::canonicalize( - _config.incLong, BSONObj(), sortKey, BSONObj(), &cqRaw, whereCallback).isOK()); - std::unique_ptr<CanonicalQuery> cq(cqRaw); + auto statusWithCQ = + CanonicalQuery::canonicalize(_config.incLong, BSONObj(), sortKey, BSONObj(), whereCallback); + verify(statusWithCQ.isOK()); + std::unique_ptr<CanonicalQuery> cq = std::move(statusWithCQ.getValue()); Collection* coll = getCollectionOrUassert(ctx->getDb(), _config.incLong); invariant(coll); @@ -1363,14 +1363,13 @@ public: const WhereCallbackReal whereCallback(txn, nss.db()); - CanonicalQuery* cqRaw; - if (!CanonicalQuery::canonicalize( - config.ns, config.filter, config.sort, BSONObj(), &cqRaw, whereCallback) - .isOK()) { + auto statusWithCQ = CanonicalQuery::canonicalize( + config.ns, config.filter, config.sort, BSONObj(), whereCallback); + if (!statusWithCQ.isOK()) { uasserted(17238, "Can't canonicalize query " + config.filter.toString()); return 0; } - std::unique_ptr<CanonicalQuery> cq(cqRaw); + std::unique_ptr<CanonicalQuery> cq = std::move(statusWithCQ.getValue()); Database* db = scopedAutoDb->getDb(); Collection* coll = state.getCollectionOrUassert(db, config.ns); diff --git a/src/mongo/db/commands/plan_cache_commands.cpp b/src/mongo/db/commands/plan_cache_commands.cpp index 1fc0b40493c..2232632c9a5 100644 --- a/src/mongo/db/commands/plan_cache_commands.cpp +++ b/src/mongo/db/commands/plan_cache_commands.cpp @@ -169,10 +169,9 @@ Status PlanCacheCommand::checkAuthForCommand(ClientBasic* client, } // static -Status PlanCacheCommand::canonicalize(OperationContext* txn, - const string& ns, - const BSONObj& cmdObj, - CanonicalQuery** canonicalQueryOut) { +StatusWith<unique_ptr<CanonicalQuery>> PlanCacheCommand::canonicalize(OperationContext* txn, + const string& ns, + const BSONObj& cmdObj) { // query - required BSONElement queryElt = cmdObj.getField("query"); if (queryElt.eoo()) { @@ -207,19 +206,15 @@ Status PlanCacheCommand::canonicalize(OperationContext* txn, } // Create canonical query - CanonicalQuery* cqRaw; - const NamespaceString nss(ns); const WhereCallbackReal whereCallback(txn, nss.db()); - Status result = - CanonicalQuery::canonicalize(ns, queryObj, sortObj, projObj, &cqRaw, whereCallback); - if (!result.isOK()) { - return result; + auto statusWithCQ = CanonicalQuery::canonicalize(ns, queryObj, sortObj, projObj, whereCallback); + if (!statusWithCQ.isOK()) { + return statusWithCQ.getStatus(); } - *canonicalQueryOut = cqRaw; - return Status::OK(); + return std::move(statusWithCQ.getValue()); } PlanCacheListQueryShapes::PlanCacheListQueryShapes() @@ -304,13 +299,12 @@ Status PlanCacheClear::clear(OperationContext* txn, // - clear plans for single query shape when a query shape is described in the // command arguments. if (cmdObj.hasField("query")) { - CanonicalQuery* cqRaw; - Status status = PlanCacheCommand::canonicalize(txn, ns, cmdObj, &cqRaw); - if (!status.isOK()) { - return status; + auto statusWithCQ = PlanCacheCommand::canonicalize(txn, ns, cmdObj); + if (!statusWithCQ.isOK()) { + return statusWithCQ.getStatus(); } - unique_ptr<CanonicalQuery> cq(cqRaw); + unique_ptr<CanonicalQuery> cq = std::move(statusWithCQ.getValue()); if (!planCache->contains(*cq)) { // Log if asked to clear non-existent query shape. @@ -374,13 +368,11 @@ Status PlanCacheListPlans::list(OperationContext* txn, const std::string& ns, const BSONObj& cmdObj, BSONObjBuilder* bob) { - CanonicalQuery* cqRaw; - Status status = canonicalize(txn, ns, cmdObj, &cqRaw); - if (!status.isOK()) { - return status; + auto statusWithCQ = canonicalize(txn, ns, cmdObj); + if (!statusWithCQ.isOK()) { + return statusWithCQ.getStatus(); } - - unique_ptr<CanonicalQuery> cq(cqRaw); + unique_ptr<CanonicalQuery> cq = std::move(statusWithCQ.getValue()); if (!planCache.contains(*cq)) { // Return empty plans in results if query shape does not diff --git a/src/mongo/db/commands/plan_cache_commands.h b/src/mongo/db/commands/plan_cache_commands.h index 3858704dbde..d42773f42e7 100644 --- a/src/mongo/db/commands/plan_cache_commands.h +++ b/src/mongo/db/commands/plan_cache_commands.h @@ -94,10 +94,9 @@ public: /** * Validatess query shape from command object and returns canonical query. */ - static Status canonicalize(OperationContext* txn, - const std::string& ns, - const BSONObj& cmdObj, - CanonicalQuery** canonicalQueryOut); + static StatusWith<std::unique_ptr<CanonicalQuery>> canonicalize(OperationContext* txn, + const std::string& ns, + const BSONObj& cmdObj); private: std::string helpText; diff --git a/src/mongo/db/commands/plan_cache_commands_test.cpp b/src/mongo/db/commands/plan_cache_commands_test.cpp index 8a7eee783d8..c1216ea197d 100644 --- a/src/mongo/db/commands/plan_cache_commands_test.cpp +++ b/src/mongo/db/commands/plan_cache_commands_test.cpp @@ -125,9 +125,9 @@ TEST(PlanCacheCommandsTest, planCacheListQueryShapesEmpty) { TEST(PlanCacheCommandsTest, planCacheListQueryShapesOneKey) { // Create a canonical query - CanonicalQuery* cqRaw; - ASSERT_OK(CanonicalQuery::canonicalize(ns, fromjson("{a: 1}"), &cqRaw)); - unique_ptr<CanonicalQuery> cq(cqRaw); + auto statusWithCQ = CanonicalQuery::canonicalize(ns, fromjson("{a: 1}")); + ASSERT_OK(statusWithCQ.getStatus()); + unique_ptr<CanonicalQuery> cq = std::move(statusWithCQ.getValue()); // Plan cache with one entry PlanCache planCache; @@ -150,9 +150,9 @@ TEST(PlanCacheCommandsTest, planCacheListQueryShapesOneKey) { TEST(PlanCacheCommandsTest, planCacheClearAllShapes) { // Create a canonical query - CanonicalQuery* cqRaw; - ASSERT_OK(CanonicalQuery::canonicalize(ns, fromjson("{a: 1}"), &cqRaw)); - unique_ptr<CanonicalQuery> cq(cqRaw); + auto statusWithCQ = CanonicalQuery::canonicalize(ns, fromjson("{a: 1}")); + ASSERT_OK(statusWithCQ.getStatus()); + unique_ptr<CanonicalQuery> cq = std::move(statusWithCQ.getValue()); // Plan cache with one entry PlanCache planCache; @@ -178,52 +178,57 @@ TEST(PlanCacheCommandsTest, planCacheClearAllShapes) { TEST(PlanCacheCommandsTest, Canonicalize) { // Invalid parameters PlanCache planCache; - CanonicalQuery* cqRaw; OperationContextNoop txn; // Missing query field - ASSERT_NOT_OK(PlanCacheCommand::canonicalize(&txn, ns, fromjson("{}"), &cqRaw)); + ASSERT_NOT_OK(PlanCacheCommand::canonicalize(&txn, ns, fromjson("{}")).getStatus()); // Query needs to be an object - ASSERT_NOT_OK(PlanCacheCommand::canonicalize(&txn, ns, fromjson("{query: 1}"), &cqRaw)); + ASSERT_NOT_OK(PlanCacheCommand::canonicalize(&txn, ns, fromjson("{query: 1}")).getStatus()); // Sort needs to be an object ASSERT_NOT_OK( - PlanCacheCommand::canonicalize(&txn, ns, fromjson("{query: {}, sort: 1}"), &cqRaw)); + PlanCacheCommand::canonicalize(&txn, ns, fromjson("{query: {}, sort: 1}")).getStatus()); // Bad query (invalid sort order) - ASSERT_NOT_OK( - PlanCacheCommand::canonicalize(&txn, ns, fromjson("{query: {}, sort: {a: 0}}"), &cqRaw)); + ASSERT_NOT_OK(PlanCacheCommand::canonicalize(&txn, ns, fromjson("{query: {}, sort: {a: 0}}")) + .getStatus()); // Valid parameters - ASSERT_OK(PlanCacheCommand::canonicalize(&txn, ns, fromjson("{query: {a: 1, b: 1}}"), &cqRaw)); - unique_ptr<CanonicalQuery> query(cqRaw); + auto statusWithCQ = PlanCacheCommand::canonicalize(&txn, ns, fromjson("{query: {a: 1, b: 1}}")); + ASSERT_OK(statusWithCQ.getStatus()); + unique_ptr<CanonicalQuery> query = std::move(statusWithCQ.getValue()); // Equivalent query should generate same key. - ASSERT_OK(PlanCacheCommand::canonicalize(&txn, ns, fromjson("{query: {b: 1, a: 1}}"), &cqRaw)); - unique_ptr<CanonicalQuery> equivQuery(cqRaw); + statusWithCQ = PlanCacheCommand::canonicalize(&txn, ns, fromjson("{query: {b: 1, a: 1}}")); + ASSERT_OK(statusWithCQ.getStatus()); + unique_ptr<CanonicalQuery> equivQuery = std::move(statusWithCQ.getValue()); ASSERT_EQUALS(planCache.computeKey(*query), planCache.computeKey(*equivQuery)); // Sort query should generate different key from unsorted query. - ASSERT_OK(PlanCacheCommand::canonicalize( - &txn, ns, fromjson("{query: {a: 1, b: 1}, sort: {a: 1, b: 1}}"), &cqRaw)); - unique_ptr<CanonicalQuery> sortQuery1(cqRaw); + statusWithCQ = PlanCacheCommand::canonicalize( + &txn, ns, fromjson("{query: {a: 1, b: 1}, sort: {a: 1, b: 1}}")); + ASSERT_OK(statusWithCQ.getStatus()); + unique_ptr<CanonicalQuery> sortQuery1 = std::move(statusWithCQ.getValue()); ASSERT_NOT_EQUALS(planCache.computeKey(*query), planCache.computeKey(*sortQuery1)); // Confirm sort arguments are properly delimited (SERVER-17158) - ASSERT_OK(PlanCacheCommand::canonicalize( - &txn, ns, fromjson("{query: {a: 1, b: 1}, sort: {aab: 1}}"), &cqRaw)); - unique_ptr<CanonicalQuery> sortQuery2(cqRaw); + statusWithCQ = + PlanCacheCommand::canonicalize(&txn, ns, fromjson("{query: {a: 1, b: 1}, sort: {aab: 1}}")); + ASSERT_OK(statusWithCQ.getStatus()); + unique_ptr<CanonicalQuery> sortQuery2 = std::move(statusWithCQ.getValue()); ASSERT_NOT_EQUALS(planCache.computeKey(*sortQuery1), planCache.computeKey(*sortQuery2)); // Changing order and/or value of predicates should not change key - ASSERT_OK(PlanCacheCommand::canonicalize( - &txn, ns, fromjson("{query: {b: 3, a: 3}, sort: {a: 1, b: 1}}"), &cqRaw)); - unique_ptr<CanonicalQuery> sortQuery3(cqRaw); + statusWithCQ = PlanCacheCommand::canonicalize( + &txn, ns, fromjson("{query: {b: 3, a: 3}, sort: {a: 1, b: 1}}")); + ASSERT_OK(statusWithCQ.getStatus()); + unique_ptr<CanonicalQuery> sortQuery3 = std::move(statusWithCQ.getValue()); ASSERT_EQUALS(planCache.computeKey(*sortQuery1), planCache.computeKey(*sortQuery3)); // Projected query should generate different key from unprojected query. - ASSERT_OK(PlanCacheCommand::canonicalize( - &txn, ns, fromjson("{query: {a: 1, b: 1}, projection: {_id: 0, a: 1}}"), &cqRaw)); - unique_ptr<CanonicalQuery> projectionQuery(cqRaw); + statusWithCQ = PlanCacheCommand::canonicalize( + &txn, ns, fromjson("{query: {a: 1, b: 1}, projection: {_id: 0, a: 1}}")); + ASSERT_OK(statusWithCQ.getStatus()); + unique_ptr<CanonicalQuery> projectionQuery = std::move(statusWithCQ.getValue()); ASSERT_NOT_EQUALS(planCache.computeKey(*query), planCache.computeKey(*projectionQuery)); } @@ -258,11 +263,12 @@ TEST(PlanCacheCommandsTest, planCacheClearUnknownKey) { TEST(PlanCacheCommandsTest, planCacheClearOneKey) { // Create 2 canonical queries. - CanonicalQuery* cqRaw; - ASSERT_OK(CanonicalQuery::canonicalize(ns, fromjson("{a: 1}"), &cqRaw)); - unique_ptr<CanonicalQuery> cqA(cqRaw); - ASSERT_OK(CanonicalQuery::canonicalize(ns, fromjson("{b: 1}"), &cqRaw)); - unique_ptr<CanonicalQuery> cqB(cqRaw); + auto statusWithCQA = CanonicalQuery::canonicalize(ns, fromjson("{a: 1}")); + ASSERT_OK(statusWithCQA.getStatus()); + unique_ptr<CanonicalQuery> cqA = std::move(statusWithCQA.getValue()); + auto statusWithCQB = CanonicalQuery::canonicalize(ns, fromjson("{b: 1}")); + ASSERT_OK(statusWithCQB.getStatus()); + unique_ptr<CanonicalQuery> cqB = std::move(statusWithCQB.getValue()); // Create plan cache with 2 entries. PlanCache planCache; @@ -377,9 +383,9 @@ TEST(PlanCacheCommandsTest, planCacheListPlansUnknownKey) { TEST(PlanCacheCommandsTest, planCacheListPlansOnlyOneSolutionTrue) { // Create a canonical query - CanonicalQuery* cqRaw; - ASSERT_OK(CanonicalQuery::canonicalize(ns, fromjson("{a: 1}"), &cqRaw)); - unique_ptr<CanonicalQuery> cq(cqRaw); + auto statusWithCQ = CanonicalQuery::canonicalize(ns, fromjson("{a: 1}")); + ASSERT_OK(statusWithCQ.getStatus()); + unique_ptr<CanonicalQuery> cq = std::move(statusWithCQ.getValue()); // Plan cache with one entry PlanCache planCache; @@ -396,9 +402,9 @@ TEST(PlanCacheCommandsTest, planCacheListPlansOnlyOneSolutionTrue) { TEST(PlanCacheCommandsTest, planCacheListPlansOnlyOneSolutionFalse) { // Create a canonical query - CanonicalQuery* cqRaw; - ASSERT_OK(CanonicalQuery::canonicalize(ns, fromjson("{a: 1}"), &cqRaw)); - unique_ptr<CanonicalQuery> cq(cqRaw); + auto statusWithCQ = CanonicalQuery::canonicalize(ns, fromjson("{a: 1}")); + ASSERT_OK(statusWithCQ.getStatus()); + unique_ptr<CanonicalQuery> cq = std::move(statusWithCQ.getValue()); // Plan cache with one entry PlanCache planCache; diff --git a/src/mongo/db/dbcommands.cpp b/src/mongo/db/dbcommands.cpp index 935b4ec41f6..ba7e84b6e31 100644 --- a/src/mongo/db/dbcommands.cpp +++ b/src/mongo/db/dbcommands.cpp @@ -587,11 +587,12 @@ public: BSONObj sort = BSON("files_id" << 1 << "n" << 1); MONGO_WRITE_CONFLICT_RETRY_LOOP_BEGIN { - CanonicalQuery* cq; - if (!CanonicalQuery::canonicalize(ns, query, sort, BSONObj(), &cq).isOK()) { + auto statusWithCQ = CanonicalQuery::canonicalize(ns, query, sort, BSONObj()); + if (!statusWithCQ.isOK()) { uasserted(17240, "Can't canonicalize query " + query.toString()); return 0; } + unique_ptr<CanonicalQuery> cq = std::move(statusWithCQ.getValue()); // Check shard version at startup. // This will throw before we've done any work if shard version is outdated @@ -602,7 +603,7 @@ public: PlanExecutor* rawExec; if (!getExecutor(txn, coll, - cq, + cq.release(), PlanExecutor::YIELD_MANUAL, &rawExec, QueryPlannerParams::NO_TABLE_SCAN).isOK()) { diff --git a/src/mongo/db/dbhelpers.cpp b/src/mongo/db/dbhelpers.cpp index e5e198d7ff0..0517552c7c2 100644 --- a/src/mongo/db/dbhelpers.cpp +++ b/src/mongo/db/dbhelpers.cpp @@ -129,18 +129,19 @@ RecordId Helpers::findOne(OperationContext* txn, if (!collection) return RecordId(); - CanonicalQuery* cq; const WhereCallbackReal whereCallback(txn, collection->ns().db()); - massert(17244, - "Could not canonicalize " + query.toString(), - CanonicalQuery::canonicalize(collection->ns(), query, &cq, whereCallback).isOK()); + auto statusWithCQ = CanonicalQuery::canonicalize(collection->ns(), query, whereCallback); + massert(17244, "Could not canonicalize " + query.toString(), statusWithCQ.isOK()); + unique_ptr<CanonicalQuery> cq = std::move(statusWithCQ.getValue()); PlanExecutor* rawExec; size_t options = requireIndex ? QueryPlannerParams::NO_TABLE_SCAN : QueryPlannerParams::DEFAULT; - massert(17245, - "Could not get executor for query " + query.toString(), - getExecutor(txn, collection, cq, PlanExecutor::YIELD_MANUAL, &rawExec, options).isOK()); + massert( + 17245, + "Could not get executor for query " + query.toString(), + getExecutor(txn, collection, cq.release(), PlanExecutor::YIELD_MANUAL, &rawExec, options) + .isOK()); unique_ptr<PlanExecutor> exec(rawExec); PlanExecutor::ExecState state; diff --git a/src/mongo/db/exec/sort.cpp b/src/mongo/db/exec/sort.cpp index 7a98b5113c2..7aeef7ee68b 100644 --- a/src/mongo/db/exec/sort.cpp +++ b/src/mongo/db/exec/sort.cpp @@ -226,10 +226,10 @@ void SortStageKeyGenerator::getBoundsForSort(const BSONObj& queryObj, const BSON sortObj, IndexNames::BTREE, true, false, false, "doesnt_matter", NULL, BSONObj()); params.indices.push_back(sortOrder); - CanonicalQuery* rawQueryForSort; - verify(CanonicalQuery::canonicalize("fake_ns", queryObj, &rawQueryForSort, WhereCallbackNoop()) - .isOK()); - unique_ptr<CanonicalQuery> queryForSort(rawQueryForSort); + auto statusWithQueryForSort = + CanonicalQuery::canonicalize("fake_ns", queryObj, WhereCallbackNoop()); + verify(statusWithQueryForSort.isOK()); + unique_ptr<CanonicalQuery> queryForSort = std::move(statusWithQueryForSort.getValue()); vector<QuerySolution*> solns; LOG(5) << "Sort stage: Planning to obtain bounds for sort." << endl; diff --git a/src/mongo/db/exec/subplan.cpp b/src/mongo/db/exec/subplan.cpp index 62daed09f32..a5b33be1ba9 100644 --- a/src/mongo/db/exec/subplan.cpp +++ b/src/mongo/db/exec/subplan.cpp @@ -130,27 +130,23 @@ Status SubplanStage::planSubqueries() { MatchExpression* orChild = orExpr->getChild(i); // Turn the i-th child into its own query. - { - CanonicalQuery* orChildCQ; - Status childCQStatus = - CanonicalQuery::canonicalize(*_query, orChild, &orChildCQ, whereCallback); - if (!childCQStatus.isOK()) { - mongoutils::str::stream ss; - ss << "Can't canonicalize subchild " << orChild->toString() << " " - << childCQStatus.reason(); - return Status(ErrorCodes::BadValue, ss); - } - - branchResult->canonicalQuery.reset(orChildCQ); + auto statusWithCQ = CanonicalQuery::canonicalize(*_query, orChild, whereCallback); + if (!statusWithCQ.isOK()) { + mongoutils::str::stream ss; + ss << "Can't canonicalize subchild " << orChild->toString() << " " + << statusWithCQ.getStatus().reason(); + return Status(ErrorCodes::BadValue, ss); } + branchResult->canonicalQuery = std::move(statusWithCQ.getValue()); + // Plan the i-th child. We might be able to find a plan for the i-th child in the plan // cache. If there's no cached plan, then we generate and rank plans using the MPS. CachedSolution* rawCS; - if (PlanCache::shouldCacheQuery(*branchResult->canonicalQuery.get()) && + if (PlanCache::shouldCacheQuery(*branchResult->canonicalQuery) && _collection->infoCache() ->getPlanCache() - ->get(*branchResult->canonicalQuery.get(), &rawCS) + ->get(*branchResult->canonicalQuery, &rawCS) .isOK()) { // We have a CachedSolution. Store it for later. LOG(5) << "Subplanner: cached plan found for child " << i << " of " @@ -163,7 +159,7 @@ 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. - Status status = QueryPlanner::plan(*branchResult->canonicalQuery.get(), + Status status = QueryPlanner::plan(*branchResult->canonicalQuery, _plannerParams, &branchResult->solutions.mutableVector()); diff --git a/src/mongo/db/exec/update.cpp b/src/mongo/db/exec/update.cpp index 7d2a768664f..e37d716d338 100644 --- a/src/mongo/db/exec/update.cpp +++ b/src/mongo/db/exec/update.cpp @@ -619,7 +619,7 @@ Status UpdateStage::applyUpdateOpsForInsert(const CanonicalQuery* cq, BSONObj original; if (cq) { - Status status = driver->populateDocumentWithQueryFields(cq, immutablePaths, *doc); + Status status = driver->populateDocumentWithQueryFields(*cq, immutablePaths, *doc); if (!status.isOK()) { return status; } diff --git a/src/mongo/db/ops/parsed_delete.cpp b/src/mongo/db/ops/parsed_delete.cpp index 46a2a33752d..ddb24fa2faf 100644 --- a/src/mongo/db/ops/parsed_delete.cpp +++ b/src/mongo/db/ops/parsed_delete.cpp @@ -69,7 +69,6 @@ Status ParsedDelete::parseRequest() { Status ParsedDelete::parseQueryToCQ() { dassert(!_canonicalQuery.get()); - CanonicalQuery* cqRaw; const WhereCallbackReal whereCallback(_txn, _request->getNamespaceString().db()); // Limit should only used for the findAndModify command when a sort is specified. If a sort @@ -83,25 +82,24 @@ Status ParsedDelete::parseQueryToCQ() { // The projection needs to be applied after the delete operation, so we specify an empty // BSONObj as the projection during canonicalization. const BSONObj emptyObj; - Status status = CanonicalQuery::canonicalize(_request->getNamespaceString().ns(), - _request->getQuery(), - _request->getSort(), - emptyObj, // projection - 0, // skip - limit, - emptyObj, // hint - emptyObj, // min - emptyObj, // max - false, // snapshot - _request->isExplain(), - &cqRaw, - whereCallback); - - if (status.isOK()) { - _canonicalQuery.reset(cqRaw); + auto statusWithCQ = CanonicalQuery::canonicalize(_request->getNamespaceString().ns(), + _request->getQuery(), + _request->getSort(), + emptyObj, // projection + 0, // skip + limit, + emptyObj, // hint + emptyObj, // min + emptyObj, // max + false, // snapshot + _request->isExplain(), + whereCallback); + + if (statusWithCQ.isOK()) { + _canonicalQuery = std::move(statusWithCQ.getValue()); } - return status; + return statusWithCQ.getStatus(); } const DeleteRequest* ParsedDelete::getRequest() const { @@ -123,9 +121,9 @@ bool ParsedDelete::hasParsedQuery() const { return _canonicalQuery.get() != NULL; } -CanonicalQuery* ParsedDelete::releaseParsedQuery() { +std::unique_ptr<CanonicalQuery> ParsedDelete::releaseParsedQuery() { invariant(_canonicalQuery.get() != NULL); - return _canonicalQuery.release(); + return std::move(_canonicalQuery); } } // namespace mongo diff --git a/src/mongo/db/ops/parsed_delete.h b/src/mongo/db/ops/parsed_delete.h index 0745cbb85e8..cdf22b4cac7 100644 --- a/src/mongo/db/ops/parsed_delete.h +++ b/src/mongo/db/ops/parsed_delete.h @@ -102,7 +102,7 @@ public: /** * Releases ownership of the canonical query to the caller. */ - CanonicalQuery* releaseParsedQuery(); + std::unique_ptr<CanonicalQuery> releaseParsedQuery(); private: // Transactional context. Not owned by us. diff --git a/src/mongo/db/ops/parsed_update.cpp b/src/mongo/db/ops/parsed_update.cpp index 80138e0385d..41b010f1d68 100644 --- a/src/mongo/db/ops/parsed_update.cpp +++ b/src/mongo/db/ops/parsed_update.cpp @@ -73,7 +73,6 @@ Status ParsedUpdate::parseQuery() { Status ParsedUpdate::parseQueryToCQ() { dassert(!_canonicalQuery.get()); - CanonicalQuery* cqRaw; const WhereCallbackReal whereCallback(_txn, _request->getNamespaceString().db()); // Limit should only used for the findAndModify command when a sort is specified. If a sort @@ -87,24 +86,23 @@ Status ParsedUpdate::parseQueryToCQ() { // The projection needs to be applied after the update operation, so we specify an empty // BSONObj as the projection during canonicalization. const BSONObj emptyObj; - Status status = CanonicalQuery::canonicalize(_request->getNamespaceString().ns(), - _request->getQuery(), - _request->getSort(), - emptyObj, // projection - 0, // skip - limit, - emptyObj, // hint - emptyObj, // min - emptyObj, // max - false, // snapshot - _request->isExplain(), - &cqRaw, - whereCallback); - if (status.isOK()) { - _canonicalQuery.reset(cqRaw); + auto statusWithCQ = CanonicalQuery::canonicalize(_request->getNamespaceString().ns(), + _request->getQuery(), + _request->getSort(), + emptyObj, // projection + 0, // skip + limit, + emptyObj, // hint + emptyObj, // min + emptyObj, // max + false, // snapshot + _request->isExplain(), + whereCallback); + if (statusWithCQ.isOK()) { + _canonicalQuery = std::move(statusWithCQ.getValue()); } - return status; + return statusWithCQ.getStatus(); } Status ParsedUpdate::parseUpdate() { @@ -138,9 +136,9 @@ bool ParsedUpdate::hasParsedQuery() const { return _canonicalQuery.get() != NULL; } -CanonicalQuery* ParsedUpdate::releaseParsedQuery() { +std::unique_ptr<CanonicalQuery> ParsedUpdate::releaseParsedQuery() { invariant(_canonicalQuery.get() != NULL); - return _canonicalQuery.release(); + return std::move(_canonicalQuery); } const UpdateRequest* ParsedUpdate::getRequest() const { diff --git a/src/mongo/db/ops/parsed_update.h b/src/mongo/db/ops/parsed_update.h index 8ce08aaabc3..3c0cf015c94 100644 --- a/src/mongo/db/ops/parsed_update.h +++ b/src/mongo/db/ops/parsed_update.h @@ -107,7 +107,7 @@ public: /** * Releases ownership of the canonical query to the caller. */ - CanonicalQuery* releaseParsedQuery(); + std::unique_ptr<CanonicalQuery> releaseParsedQuery(); private: /** diff --git a/src/mongo/db/ops/update_driver.cpp b/src/mongo/db/ops/update_driver.cpp index ccd966dbb81..9045b691e16 100644 --- a/src/mongo/db/ops/update_driver.cpp +++ b/src/mongo/db/ops/update_driver.cpp @@ -169,18 +169,19 @@ inline Status UpdateDriver::addAndParse(const modifiertable::ModifierType type, Status UpdateDriver::populateDocumentWithQueryFields(const BSONObj& query, const vector<FieldRef*>* immutablePaths, mutablebson::Document& doc) const { - CanonicalQuery* rawCG; // We canonicalize the query to collapse $and/$or, and the first arg (ns) is not needed // Also, because this is for the upsert case, where we insert a new document if one was // not found, the $where clause does not make sense, hence empty WhereCallback. - Status s = CanonicalQuery::canonicalize("", query, &rawCG, WhereCallbackNoop()); - if (!s.isOK()) - return s; - unique_ptr<CanonicalQuery> cq(rawCG); - return populateDocumentWithQueryFields(rawCG, immutablePaths, doc); + auto statusWithCQ = CanonicalQuery::canonicalize("", query, WhereCallbackNoop()); + if (!statusWithCQ.isOK()) { + return statusWithCQ.getStatus(); + } + unique_ptr<CanonicalQuery> cq = std::move(statusWithCQ.getValue()); + + return populateDocumentWithQueryFields(*cq, immutablePaths, doc); } -Status UpdateDriver::populateDocumentWithQueryFields(const CanonicalQuery* query, +Status UpdateDriver::populateDocumentWithQueryFields(const CanonicalQuery& query, const vector<FieldRef*>* immutablePathsPtr, mutablebson::Document& doc) const { EqualityMatches equalities; @@ -200,10 +201,10 @@ Status UpdateDriver::populateDocumentWithQueryFields(const CanonicalQuery* query // Extract only immutable fields from replacement-style status = - pathsupport::extractFullEqualityMatches(*query->root(), pathsToExtract, &equalities); + pathsupport::extractFullEqualityMatches(*query.root(), pathsToExtract, &equalities); } else { // Extract all fields from op-style - status = pathsupport::extractEqualityMatches(*query->root(), &equalities); + status = pathsupport::extractEqualityMatches(*query.root(), &equalities); } if (!status.isOK()) diff --git a/src/mongo/db/ops/update_driver.h b/src/mongo/db/ops/update_driver.h index 00e3e2eb10f..bb11ee42bb4 100644 --- a/src/mongo/db/ops/update_driver.h +++ b/src/mongo/db/ops/update_driver.h @@ -71,7 +71,7 @@ public: const std::vector<FieldRef*>* immutablePaths, mutablebson::Document& doc) const; - Status populateDocumentWithQueryFields(const CanonicalQuery* query, + Status populateDocumentWithQueryFields(const CanonicalQuery& query, const std::vector<FieldRef*>* immutablePaths, mutablebson::Document& doc) const; diff --git a/src/mongo/db/pipeline/pipeline_d.cpp b/src/mongo/db/pipeline/pipeline_d.cpp index 6dbcfe4c812..0a10cfc5f3d 100644 --- a/src/mongo/db/pipeline/pipeline_d.cpp +++ b/src/mongo/db/pipeline/pipeline_d.cpp @@ -48,6 +48,7 @@ namespace mongo { using boost::intrusive_ptr; using std::shared_ptr; using std::string; +using std::unique_ptr; namespace { class MongodImplementation final : public DocumentSourceNeedsMongod::MongodInterface { @@ -180,14 +181,17 @@ shared_ptr<PlanExecutor> PipelineD::prepareCursorSource( const WhereCallbackReal whereCallback(pExpCtx->opCtx, pExpCtx->ns.db()); if (sortStage) { - CanonicalQuery* cq; - Status status = CanonicalQuery::canonicalize( - pExpCtx->ns, queryObj, sortObj, projectionForQuery, &cq, whereCallback); + auto statusWithCQ = CanonicalQuery::canonicalize( + pExpCtx->ns, queryObj, sortObj, projectionForQuery, whereCallback); PlanExecutor* rawExec; - if (status.isOK() && - getExecutor(txn, collection, cq, PlanExecutor::YIELD_AUTO, &rawExec, runnerOptions) - .isOK()) { + if (statusWithCQ.isOK() && + getExecutor(txn, + collection, + statusWithCQ.getValue().release(), + PlanExecutor::YIELD_AUTO, + &rawExec, + runnerOptions).isOK()) { // success: The PlanExecutor will handle sorting for us using an index. exec.reset(rawExec); sortInRunner = true; @@ -202,13 +206,14 @@ shared_ptr<PlanExecutor> PipelineD::prepareCursorSource( if (!exec.get()) { const BSONObj noSort; - CanonicalQuery* cq; - uassertStatusOK(CanonicalQuery::canonicalize( - pExpCtx->ns, queryObj, noSort, projectionForQuery, &cq, whereCallback)); + auto statusWithCQ = CanonicalQuery::canonicalize( + pExpCtx->ns, queryObj, noSort, projectionForQuery, whereCallback); + uassertStatusOK(statusWithCQ.getStatus()); + unique_ptr<CanonicalQuery> cq = std::move(statusWithCQ.getValue()); PlanExecutor* rawExec; - uassertStatusOK( - getExecutor(txn, collection, cq, PlanExecutor::YIELD_AUTO, &rawExec, runnerOptions)); + uassertStatusOK(getExecutor( + txn, collection, cq.release(), PlanExecutor::YIELD_AUTO, &rawExec, runnerOptions)); exec.reset(rawExec); } diff --git a/src/mongo/db/query/canonical_query.cpp b/src/mongo/db/query/canonical_query.cpp index ac6ba1627d1..701a5fd2545 100644 --- a/src/mongo/db/query/canonical_query.cpp +++ b/src/mongo/db/query/canonical_query.cpp @@ -100,20 +100,20 @@ bool matchExpressionLessThan(const MatchExpression* lhs, const MatchExpression* // // static -Status CanonicalQuery::canonicalize(const std::string& ns, - const BSONObj& query, - CanonicalQuery** out, - const MatchExpressionParser::WhereCallback& whereCallback) { +StatusWith<std::unique_ptr<CanonicalQuery>> CanonicalQuery::canonicalize( + const std::string& ns, + const BSONObj& query, + const MatchExpressionParser::WhereCallback& whereCallback) { const BSONObj emptyObj; - return CanonicalQuery::canonicalize(ns, query, emptyObj, emptyObj, 0, 0, out, whereCallback); + return CanonicalQuery::canonicalize(ns, query, emptyObj, emptyObj, 0, 0, whereCallback); } // static -Status CanonicalQuery::canonicalize(const std::string& ns, - const BSONObj& query, - bool explain, - CanonicalQuery** out, - const MatchExpressionParser::WhereCallback& whereCallback) { +StatusWith<std::unique_ptr<CanonicalQuery>> CanonicalQuery::canonicalize( + const std::string& ns, + const BSONObj& query, + bool explain, + const MatchExpressionParser::WhereCallback& whereCallback) { const BSONObj emptyObj; return CanonicalQuery::canonicalize(ns, query, @@ -126,56 +126,54 @@ Status CanonicalQuery::canonicalize(const std::string& ns, emptyObj, // max false, // snapshot explain, - out, whereCallback); } // static -Status CanonicalQuery::canonicalize(const std::string& ns, - const BSONObj& query, - long long skip, - long long limit, - CanonicalQuery** out, - const MatchExpressionParser::WhereCallback& whereCallback) { +StatusWith<std::unique_ptr<CanonicalQuery>> CanonicalQuery::canonicalize( + const std::string& ns, + const BSONObj& query, + long long skip, + long long limit, + const MatchExpressionParser::WhereCallback& whereCallback) { const BSONObj emptyObj; - return CanonicalQuery::canonicalize( - ns, query, emptyObj, emptyObj, skip, limit, out, whereCallback); + return CanonicalQuery::canonicalize(ns, query, emptyObj, emptyObj, skip, limit, whereCallback); } // static -Status CanonicalQuery::canonicalize(const std::string& ns, - const BSONObj& query, - const BSONObj& sort, - const BSONObj& proj, - CanonicalQuery** out, - const MatchExpressionParser::WhereCallback& whereCallback) { - return CanonicalQuery::canonicalize(ns, query, sort, proj, 0, 0, out, whereCallback); +StatusWith<std::unique_ptr<CanonicalQuery>> CanonicalQuery::canonicalize( + const std::string& ns, + const BSONObj& query, + const BSONObj& sort, + const BSONObj& proj, + const MatchExpressionParser::WhereCallback& whereCallback) { + return CanonicalQuery::canonicalize(ns, query, sort, proj, 0, 0, whereCallback); } // static -Status CanonicalQuery::canonicalize(const std::string& ns, - const BSONObj& query, - const BSONObj& sort, - const BSONObj& proj, - long long skip, - long long limit, - CanonicalQuery** out, - const MatchExpressionParser::WhereCallback& whereCallback) { +StatusWith<std::unique_ptr<CanonicalQuery>> CanonicalQuery::canonicalize( + const std::string& ns, + const BSONObj& query, + const BSONObj& sort, + const BSONObj& proj, + long long skip, + long long limit, + const MatchExpressionParser::WhereCallback& whereCallback) { const BSONObj emptyObj; return CanonicalQuery::canonicalize( - ns, query, sort, proj, skip, limit, emptyObj, out, whereCallback); + ns, query, sort, proj, skip, limit, emptyObj, whereCallback); } // static -Status CanonicalQuery::canonicalize(const std::string& ns, - const BSONObj& query, - const BSONObj& sort, - const BSONObj& proj, - long long skip, - long long limit, - const BSONObj& hint, - CanonicalQuery** out, - const MatchExpressionParser::WhereCallback& whereCallback) { +StatusWith<std::unique_ptr<CanonicalQuery>> CanonicalQuery::canonicalize( + const std::string& ns, + const BSONObj& query, + const BSONObj& sort, + const BSONObj& proj, + long long skip, + long long limit, + const BSONObj& hint, + const MatchExpressionParser::WhereCallback& whereCallback) { const BSONObj emptyObj; return CanonicalQuery::canonicalize(ns, query, @@ -188,7 +186,6 @@ Status CanonicalQuery::canonicalize(const std::string& ns, emptyObj, false, // snapshot false, // explain - out, whereCallback); } @@ -197,22 +194,20 @@ Status CanonicalQuery::canonicalize(const std::string& ns, // // static -Status CanonicalQuery::canonicalize(const QueryMessage& qm, - CanonicalQuery** out, - const MatchExpressionParser::WhereCallback& whereCallback) { +StatusWith<std::unique_ptr<CanonicalQuery>> CanonicalQuery::canonicalize( + const QueryMessage& qm, const MatchExpressionParser::WhereCallback& whereCallback) { // Make LiteParsedQuery. auto lpqStatus = LiteParsedQuery::fromLegacyQueryMessage(qm); if (!lpqStatus.isOK()) { return lpqStatus.getStatus(); } - return CanonicalQuery::canonicalize(lpqStatus.getValue().release(), out, whereCallback); + return CanonicalQuery::canonicalize(lpqStatus.getValue().release(), whereCallback); } // static -Status CanonicalQuery::canonicalize(LiteParsedQuery* lpq, - CanonicalQuery** out, - const MatchExpressionParser::WhereCallback& whereCallback) { +StatusWith<std::unique_ptr<CanonicalQuery>> CanonicalQuery::canonicalize( + LiteParsedQuery* lpq, const MatchExpressionParser::WhereCallback& whereCallback) { std::unique_ptr<LiteParsedQuery> autoLpq(lpq); // Make MatchExpression. @@ -231,15 +226,14 @@ Status CanonicalQuery::canonicalize(LiteParsedQuery* lpq, if (!initStatus.isOK()) { return initStatus; } - *out = cq.release(); - return Status::OK(); + return std::move(cq); } // static -Status CanonicalQuery::canonicalize(const CanonicalQuery& baseQuery, - MatchExpression* root, - CanonicalQuery** out, - const MatchExpressionParser::WhereCallback& whereCallback) { +StatusWith<std::unique_ptr<CanonicalQuery>> CanonicalQuery::canonicalize( + const CanonicalQuery& baseQuery, + MatchExpression* root, + const MatchExpressionParser::WhereCallback& whereCallback) { // Pass empty sort and projection. BSONObj emptyObj; @@ -269,24 +263,23 @@ Status CanonicalQuery::canonicalize(const CanonicalQuery& baseQuery, if (!initStatus.isOK()) { return initStatus; } - *out = cq.release(); - return Status::OK(); + return std::move(cq); } // static -Status CanonicalQuery::canonicalize(const std::string& ns, - const BSONObj& query, - const BSONObj& sort, - const BSONObj& proj, - long long skip, - long long limit, - const BSONObj& hint, - const BSONObj& minObj, - const BSONObj& maxObj, - bool snapshot, - bool explain, - CanonicalQuery** out, - const MatchExpressionParser::WhereCallback& whereCallback) { +StatusWith<std::unique_ptr<CanonicalQuery>> CanonicalQuery::canonicalize( + const std::string& ns, + const BSONObj& query, + const BSONObj& sort, + const BSONObj& proj, + long long skip, + long long limit, + const BSONObj& hint, + const BSONObj& minObj, + const BSONObj& maxObj, + bool snapshot, + bool explain, + const MatchExpressionParser::WhereCallback& whereCallback) { // Pass empty sort and projection. BSONObj emptyObj; @@ -312,8 +305,7 @@ Status CanonicalQuery::canonicalize(const std::string& ns, if (!initStatus.isOK()) { return initStatus; } - *out = cq.release(); - return Status::OK(); + return std::move(cq); } Status CanonicalQuery::init(LiteParsedQuery* lpq, diff --git a/src/mongo/db/query/canonical_query.h b/src/mongo/db/query/canonical_query.h index 58a2c46f3a8..56f06a20c9d 100644 --- a/src/mongo/db/query/canonical_query.h +++ b/src/mongo/db/query/canonical_query.h @@ -41,26 +41,28 @@ namespace mongo { class CanonicalQuery { public: /** - * Caller owns the pointer in 'out' if any call to canonicalize returns Status::OK(). + * If parsing succeeds, returns a std::unique_ptr<CanonicalQuery> representing the parsed + * query (which will never be NULL). If parsing fails, returns an error Status. * * Used for legacy find through the OP_QUERY message. */ - static Status canonicalize(const QueryMessage& qm, - CanonicalQuery** out, - const MatchExpressionParser::WhereCallback& whereCallback = - MatchExpressionParser::WhereCallback()); + static StatusWith<std::unique_ptr<CanonicalQuery>> canonicalize( + const QueryMessage& qm, + const MatchExpressionParser::WhereCallback& whereCallback = + MatchExpressionParser::WhereCallback()); /** * Takes ownership of 'lpq'. * - * Caller owns the pointer in 'out' if any call to canonicalize returns Status::OK(). + * If parsing succeeds, returns a std::unique_ptr<CanonicalQuery> representing the parsed + * query (which will never be NULL). If parsing fails, returns an error Status. * * Used for finds using the find command path. */ - static Status canonicalize(LiteParsedQuery* lpq, - CanonicalQuery** out, - const MatchExpressionParser::WhereCallback& whereCallback = - MatchExpressionParser::WhereCallback()); + static StatusWith<std::unique_ptr<CanonicalQuery>> canonicalize( + LiteParsedQuery* lpq, + const MatchExpressionParser::WhereCallback& whereCallback = + MatchExpressionParser::WhereCallback()); /** * For testing or for internal clients to use. @@ -73,76 +75,76 @@ public: * * Does not take ownership of 'root'. */ - static Status canonicalize(const CanonicalQuery& baseQuery, - MatchExpression* root, - CanonicalQuery** out, - const MatchExpressionParser::WhereCallback& whereCallback = - MatchExpressionParser::WhereCallback()); - - static Status canonicalize(const std::string& ns, - const BSONObj& query, - CanonicalQuery** out, - const MatchExpressionParser::WhereCallback& whereCallback = - MatchExpressionParser::WhereCallback()); - - static Status canonicalize(const std::string& ns, - const BSONObj& query, - bool explain, - CanonicalQuery** out, - const MatchExpressionParser::WhereCallback& whereCallback = - MatchExpressionParser::WhereCallback()); - - static Status canonicalize(const std::string& ns, - const BSONObj& query, - long long skip, - long long limit, - CanonicalQuery** out, - const MatchExpressionParser::WhereCallback& whereCallback = - MatchExpressionParser::WhereCallback()); - - static Status canonicalize(const std::string& ns, - const BSONObj& query, - const BSONObj& sort, - const BSONObj& proj, - CanonicalQuery** out, - const MatchExpressionParser::WhereCallback& whereCallback = - MatchExpressionParser::WhereCallback()); - - static Status canonicalize(const std::string& ns, - const BSONObj& query, - const BSONObj& sort, - const BSONObj& proj, - long long skip, - long long limit, - CanonicalQuery** out, - const MatchExpressionParser::WhereCallback& whereCallback = - MatchExpressionParser::WhereCallback()); - - static Status canonicalize(const std::string& ns, - const BSONObj& query, - const BSONObj& sort, - const BSONObj& proj, - long long skip, - long long limit, - const BSONObj& hint, - CanonicalQuery** out, - const MatchExpressionParser::WhereCallback& whereCallback = - MatchExpressionParser::WhereCallback()); - - static Status canonicalize(const std::string& ns, - const BSONObj& query, - const BSONObj& sort, - const BSONObj& proj, - long long skip, - long long limit, - const BSONObj& hint, - const BSONObj& minObj, - const BSONObj& maxObj, - bool snapshot, - bool explain, - CanonicalQuery** out, - const MatchExpressionParser::WhereCallback& whereCallback = - MatchExpressionParser::WhereCallback()); + static StatusWith<std::unique_ptr<CanonicalQuery>> canonicalize( + const CanonicalQuery& baseQuery, + MatchExpression* root, + const MatchExpressionParser::WhereCallback& whereCallback = + MatchExpressionParser::WhereCallback()); + + static StatusWith<std::unique_ptr<CanonicalQuery>> canonicalize( + const std::string& ns, + const BSONObj& query, + const MatchExpressionParser::WhereCallback& whereCallback = + MatchExpressionParser::WhereCallback()); + + static StatusWith<std::unique_ptr<CanonicalQuery>> canonicalize( + const std::string& ns, + const BSONObj& query, + bool explain, + const MatchExpressionParser::WhereCallback& whereCallback = + MatchExpressionParser::WhereCallback()); + + static StatusWith<std::unique_ptr<CanonicalQuery>> canonicalize( + const std::string& ns, + const BSONObj& query, + long long skip, + long long limit, + const MatchExpressionParser::WhereCallback& whereCallback = + MatchExpressionParser::WhereCallback()); + + static StatusWith<std::unique_ptr<CanonicalQuery>> canonicalize( + const std::string& ns, + const BSONObj& query, + const BSONObj& sort, + const BSONObj& proj, + const MatchExpressionParser::WhereCallback& whereCallback = + MatchExpressionParser::WhereCallback()); + + static StatusWith<std::unique_ptr<CanonicalQuery>> canonicalize( + const std::string& ns, + const BSONObj& query, + const BSONObj& sort, + const BSONObj& proj, + long long skip, + long long limit, + const MatchExpressionParser::WhereCallback& whereCallback = + MatchExpressionParser::WhereCallback()); + + static StatusWith<std::unique_ptr<CanonicalQuery>> canonicalize( + const std::string& ns, + const BSONObj& query, + const BSONObj& sort, + const BSONObj& proj, + long long skip, + long long limit, + const BSONObj& hint, + const MatchExpressionParser::WhereCallback& whereCallback = + MatchExpressionParser::WhereCallback()); + + static StatusWith<std::unique_ptr<CanonicalQuery>> canonicalize( + const std::string& ns, + const BSONObj& query, + const BSONObj& sort, + const BSONObj& proj, + long long skip, + long long limit, + const BSONObj& hint, + const BSONObj& minObj, + const BSONObj& maxObj, + bool snapshot, + bool explain, + const MatchExpressionParser::WhereCallback& whereCallback = + MatchExpressionParser::WhereCallback()); /** * Returns true if "query" describes an exact-match query on _id, possibly with diff --git a/src/mongo/db/query/canonical_query_test.cpp b/src/mongo/db/query/canonical_query_test.cpp index d3fb9a55fd4..a754cde8502 100644 --- a/src/mongo/db/query/canonical_query_test.cpp +++ b/src/mongo/db/query/canonical_query_test.cpp @@ -52,6 +52,7 @@ MatchExpression* parseMatchExpression(const BSONObj& obj) { << ". Reason: " << status.getStatus().toString(); FAIL(ss); } + return status.getValue(); } @@ -62,8 +63,7 @@ MatchExpression* parseMatchExpression(const BSONObj& obj) { */ Status isValid(const std::string& queryStr, const LiteParsedQuery& lpqRaw) { BSONObj queryObj = fromjson(queryStr); - std::unique_ptr<MatchExpression> me( - CanonicalQuery::normalizeTree(parseMatchExpression(queryObj))); + unique_ptr<MatchExpression> me(CanonicalQuery::normalizeTree(parseMatchExpression(queryObj))); return CanonicalQuery::isValid(me.get(), lpqRaw); } @@ -461,22 +461,22 @@ TEST(CanonicalQueryTest, SortTreeNumChildrenComparison) { /** * Utility function to create a CanonicalQuery */ -CanonicalQuery* canonicalize(const char* queryStr) { +unique_ptr<CanonicalQuery> canonicalize(const char* queryStr) { BSONObj queryObj = fromjson(queryStr); - CanonicalQuery* cq; - Status result = CanonicalQuery::canonicalize(ns, queryObj, &cq); - ASSERT_OK(result); - return cq; + auto statusWithCQ = CanonicalQuery::canonicalize(ns, queryObj); + ASSERT_OK(statusWithCQ.getStatus()); + return std::move(statusWithCQ.getValue()); } -CanonicalQuery* canonicalize(const char* queryStr, const char* sortStr, const char* projStr) { +std::unique_ptr<CanonicalQuery> canonicalize(const char* queryStr, + const char* sortStr, + const char* projStr) { BSONObj queryObj = fromjson(queryStr); BSONObj sortObj = fromjson(sortStr); BSONObj projObj = fromjson(projStr); - CanonicalQuery* cq; - Status result = CanonicalQuery::canonicalize(ns, queryObj, sortObj, projObj, &cq); - ASSERT_OK(result); - return cq; + auto statusWithCQ = CanonicalQuery::canonicalize(ns, queryObj, sortObj, projObj); + ASSERT_OK(statusWithCQ.getStatus()); + return std::move(statusWithCQ.getValue()); } // Don't do anything with a double OR. diff --git a/src/mongo/db/query/find.cpp b/src/mongo/db/query/find.cpp index f170dc8ea04..2283bc66e9f 100644 --- a/src/mongo/db/query/find.cpp +++ b/src/mongo/db/query/find.cpp @@ -278,9 +278,9 @@ QueryResult::View getMore(OperationContext* txn, // Note that we declare our locks before our ClientCursorPin, in order to ensure that the // pin's destructor is called before the lock destructors (so that the unpin occurs under // the lock). - std::unique_ptr<AutoGetCollectionForRead> ctx; - std::unique_ptr<Lock::DBLock> unpinDBLock; - std::unique_ptr<Lock::CollectionLock> unpinCollLock; + unique_ptr<AutoGetCollectionForRead> ctx; + unique_ptr<Lock::DBLock> unpinDBLock; + unique_ptr<Lock::CollectionLock> unpinCollLock; CursorManager* cursorManager; CursorManager* globalCursorManager = CursorManager::getGlobalCursorManager(); @@ -315,7 +315,7 @@ QueryResult::View getMore(OperationContext* txn, // has a valid RecoveryUnit. As such, we use RAII to accomplish this. // // This must be destroyed before the ClientCursor is destroyed. - std::unique_ptr<ScopedRecoveryUnitSwapper> ruSwapper; + unique_ptr<ScopedRecoveryUnitSwapper> ruSwapper; // These are set in the QueryResult msg we return. int resultFlags = ResultFlag_AwaitCapable; @@ -416,7 +416,7 @@ QueryResult::View getMore(OperationContext* txn, if (PlanExecutor::DEAD == state || PlanExecutor::FAILURE == state) { // Propagate this error to caller. - const std::unique_ptr<PlanStageStats> stats(exec->getStats()); + const unique_ptr<PlanStageStats> stats(exec->getStats()); error() << "getMore executor error, stats: " << Explain::statsToBSON(*stats); uasserted(17406, "getMore executor error: " + WorkingSetCommon::toStatusString(obj)); } @@ -506,17 +506,14 @@ std::string runQuery(OperationContext* txn, beginQueryOp(txn, nss, q.query, q.ntoreturn, q.ntoskip); // Parse the qm into a CanonicalQuery. - std::unique_ptr<CanonicalQuery> cq; - { - CanonicalQuery* cqRaw; - Status canonStatus = - CanonicalQuery::canonicalize(q, &cqRaw, WhereCallbackReal(txn, nss.db())); - if (!canonStatus.isOK()) { - uasserted(17287, - str::stream() << "Can't canonicalize query: " << canonStatus.toString()); - } - cq.reset(cqRaw); + + auto statusWithCQ = CanonicalQuery::canonicalize(q, WhereCallbackReal(txn, nss.db())); + if (!statusWithCQ.isOK()) { + uasserted( + 17287, + str::stream() << "Can't canonicalize query: " << statusWithCQ.getStatus().toString()); } + unique_ptr<CanonicalQuery> cq = std::move(statusWithCQ.getValue()); invariant(cq.get()); LOG(5) << "Running query:\n" << cq->toString(); @@ -530,7 +527,7 @@ std::string runQuery(OperationContext* txn, ctx.getDb() ? ctx.getDb()->getProfilingLevel() : serverGlobalParams.defaultProfile; // We have a parsed query. Time to get the execution plan for it. - std::unique_ptr<PlanExecutor> exec; + unique_ptr<PlanExecutor> exec; { PlanExecutor* rawExec; Status execStatus = @@ -636,7 +633,7 @@ std::string runQuery(OperationContext* txn, // Caller expects exceptions thrown in certain cases. if (PlanExecutor::FAILURE == state || PlanExecutor::DEAD == state) { - const std::unique_ptr<PlanStageStats> stats(exec->getStats()); + const unique_ptr<PlanStageStats> stats(exec->getStats()); error() << "Plan executor error during find: " << PlanExecutor::statestr(state) << ", stats: " << Explain::statsToBSON(*stats); uasserted(17144, "Executor error: " + WorkingSetCommon::toStatusString(obj)); diff --git a/src/mongo/db/query/get_executor.cpp b/src/mongo/db/query/get_executor.cpp index 1472d6693ae..a098175b112 100644 --- a/src/mongo/db/query/get_executor.cpp +++ b/src/mongo/db/query/get_executor.cpp @@ -467,14 +467,15 @@ Status getExecutor(OperationContext* txn, if (!CanonicalQuery::isSimpleIdQuery(unparsedQuery) || !collection->getIndexCatalog()->findIdIndex(txn)) { const WhereCallbackReal whereCallback(txn, collection->ns().db()); - CanonicalQuery* cq; - Status status = - CanonicalQuery::canonicalize(collection->ns(), unparsedQuery, &cq, whereCallback); - if (!status.isOK()) - return status; + auto statusWithCQ = + CanonicalQuery::canonicalize(collection->ns(), unparsedQuery, whereCallback); + if (!statusWithCQ.isOK()) { + return statusWithCQ.getStatus(); + } + std::unique_ptr<CanonicalQuery> cq = std::move(statusWithCQ.getValue()); // Takes ownership of 'cq'. - return getExecutor(txn, collection, cq, yieldPolicy, out, plannerOptions); + return getExecutor(txn, collection, cq.release(), yieldPolicy, out, plannerOptions); } LOG(2) << "Using idhack: " << unparsedQuery.toString(); @@ -968,13 +969,13 @@ Status getExecutorGroup(OperationContext* txn, const NamespaceString nss(request.ns); const WhereCallbackReal whereCallback(txn, nss.db()); - CanonicalQuery* rawCanonicalQuery; - Status canonicalizeStatus = CanonicalQuery::canonicalize( - request.ns, request.query, request.explain, &rawCanonicalQuery, whereCallback); - if (!canonicalizeStatus.isOK()) { - return canonicalizeStatus; + + auto statusWithCQ = + CanonicalQuery::canonicalize(request.ns, request.query, request.explain, whereCallback); + if (!statusWithCQ.isOK()) { + return statusWithCQ.getStatus(); } - unique_ptr<CanonicalQuery> canonicalQuery(rawCanonicalQuery); + unique_ptr<CanonicalQuery> canonicalQuery = std::move(statusWithCQ.getValue()); const size_t defaultPlannerOptions = 0; Status status = prepareExecution(txn, @@ -1133,7 +1134,8 @@ std::string getProjectedDottedField(const std::string& field, bool* isIDOut) { std::vector<std::string> prefixStrings(res); prefixStrings.resize(i); // Reset projectedField. Instead of overwriting, joinStringDelim() appends joined - // string to the end of projectedField. + // string + // to the end of projectedField. std::string projectedField; mongo::joinStringDelim(prefixStrings, &projectedField, '.'); return projectedField; @@ -1200,8 +1202,7 @@ Status getExecutorCount(OperationContext* txn, if (!request.getQuery().isEmpty() || !request.getHint().isEmpty()) { // If query or hint is not empty, canonicalize the query before working with collection. typedef MatchExpressionParser::WhereCallback WhereCallback; - CanonicalQuery* rawCq = NULL; - Status canonStatus = CanonicalQuery::canonicalize( + auto statusWithCQ = CanonicalQuery::canonicalize( request.getNs(), request.getQuery(), BSONObj(), // sort @@ -1213,14 +1214,13 @@ Status getExecutorCount(OperationContext* txn, BSONObj(), // max false, // snapshot explain, - &rawCq, collection ? static_cast<const WhereCallback&>(WhereCallbackReal(txn, collection->ns().db())) : static_cast<const WhereCallback&>(WhereCallbackNoop())); - if (!canonStatus.isOK()) { - return canonStatus; + if (!statusWithCQ.isOK()) { + return statusWithCQ.getStatus(); } - cq.reset(rawCq); + cq = std::move(statusWithCQ.getValue()); } if (!collection) { @@ -1348,15 +1348,15 @@ Status getExecutorDistinct(OperationContext* txn, // If there are no suitable indices for the distinct hack bail out now into regular planning // with no projection. if (plannerParams.indices.empty()) { - CanonicalQuery* cq; - Status status = - CanonicalQuery::canonicalize(collection->ns().ns(), query, &cq, whereCallback); - if (!status.isOK()) { - return status; + auto statusWithCQ = + CanonicalQuery::canonicalize(collection->ns().ns(), query, whereCallback); + if (!statusWithCQ.isOK()) { + return statusWithCQ.getStatus(); } + std::unique_ptr<CanonicalQuery> cq = std::move(statusWithCQ.getValue()); // Takes ownership of 'cq'. - return getExecutor(txn, collection, cq, yieldPolicy, out); + return getExecutor(txn, collection, cq.release(), yieldPolicy, out); } // @@ -1369,14 +1369,13 @@ Status getExecutorDistinct(OperationContext* txn, BSONObj projection = getDistinctProjection(field); // Apply a projection of the key. Empty BSONObj() is for the sort. - CanonicalQuery* cq; - Status status = CanonicalQuery::canonicalize( - collection->ns().ns(), query, BSONObj(), projection, &cq, whereCallback); - if (!status.isOK()) { - return status; + auto statusWithCQ = CanonicalQuery::canonicalize( + collection->ns().ns(), query, BSONObj(), projection, whereCallback); + if (!statusWithCQ.isOK()) { + return statusWithCQ.getStatus(); } - unique_ptr<CanonicalQuery> autoCq(cq); + unique_ptr<CanonicalQuery> cq = std::move(statusWithCQ.getValue()); // If there's no query, we can just distinct-scan one of the indices. // Not every index in plannerParams.indices may be suitable. Refer to @@ -1403,15 +1402,14 @@ Status getExecutorDistinct(OperationContext* txn, << ", planSummary: " << Explain::getPlanSummary(root); // Takes ownership of its arguments (except for 'collection'). - return PlanExecutor::make( - txn, ws, root, soln, autoCq.release(), collection, yieldPolicy, out); + return PlanExecutor::make(txn, ws, root, soln, cq.release(), collection, yieldPolicy, out); } // See if we can answer the query in a fast-distinct compatible fashion. vector<QuerySolution*> solutions; - status = QueryPlanner::plan(*cq, plannerParams, &solutions); + Status status = QueryPlanner::plan(*cq, plannerParams, &solutions); if (!status.isOK()) { - return getExecutor(txn, collection, autoCq.release(), yieldPolicy, out); + return getExecutor(txn, collection, cq.release(), yieldPolicy, out); } // We look for a solution that has an ixscan we can turn into a distinctixscan @@ -1432,9 +1430,9 @@ Status getExecutorDistinct(OperationContext* txn, LOG(2) << "Using fast distinct: " << cq->toStringShort() << ", planSummary: " << Explain::getPlanSummary(root); - // Takes ownership of 'ws', 'root', 'solutions[i]', and 'autoCq'. + // Takes ownership of 'ws', 'root', 'solutions[i]', and 'cq'. return PlanExecutor::make( - txn, ws, root, solutions[i], autoCq.release(), collection, yieldPolicy, out); + txn, ws, root, solutions[i], cq.release(), collection, yieldPolicy, out); } } @@ -1446,15 +1444,15 @@ Status getExecutorDistinct(OperationContext* txn, } // We drop the projection from the 'cq'. Unfortunately this is not trivial. - status = CanonicalQuery::canonicalize(collection->ns().ns(), query, &cq, whereCallback); - if (!status.isOK()) { - return status; + statusWithCQ = CanonicalQuery::canonicalize(collection->ns().ns(), query, whereCallback); + if (!statusWithCQ.isOK()) { + return statusWithCQ.getStatus(); } - autoCq.reset(cq); + cq = std::move(statusWithCQ.getValue()); - // Takes ownership of 'autoCq'. - return getExecutor(txn, collection, autoCq.release(), yieldPolicy, out); + // Takes ownership of 'cq'. + return getExecutor(txn, collection, cq.release(), yieldPolicy, out); } } // namespace mongo diff --git a/src/mongo/db/query/get_executor_test.cpp b/src/mongo/db/query/get_executor_test.cpp index fdc04609df8..0e8740fa089 100644 --- a/src/mongo/db/query/get_executor_test.cpp +++ b/src/mongo/db/query/get_executor_test.cpp @@ -48,14 +48,15 @@ static const char* ns = "somebogusns"; /** * Utility functions to create a CanonicalQuery */ -CanonicalQuery* canonicalize(const char* queryStr, const char* sortStr, const char* projStr) { +unique_ptr<CanonicalQuery> canonicalize(const char* queryStr, + const char* sortStr, + const char* projStr) { BSONObj queryObj = fromjson(queryStr); BSONObj sortObj = fromjson(sortStr); BSONObj projObj = fromjson(projStr); - CanonicalQuery* cq; - Status result = CanonicalQuery::canonicalize(ns, queryObj, sortObj, projObj, &cq); - ASSERT_OK(result); - return cq; + auto statusWithCQ = CanonicalQuery::canonicalize(ns, queryObj, sortObj, projObj); + ASSERT_OK(statusWithCQ.getStatus()); + return std::move(statusWithCQ.getValue()); } // diff --git a/src/mongo/db/query/plan_cache_test.cpp b/src/mongo/db/query/plan_cache_test.cpp index da15528d243..2bee0f5f10b 100644 --- a/src/mongo/db/query/plan_cache_test.cpp +++ b/src/mongo/db/query/plan_cache_test.cpp @@ -59,90 +59,77 @@ static const char* ns = "somebogusns"; /** * Utility functions to create a CanonicalQuery */ -CanonicalQuery* canonicalize(const BSONObj& queryObj) { - CanonicalQuery* cq; - Status result = CanonicalQuery::canonicalize(ns, queryObj, &cq); - ASSERT_OK(result); - return cq; +unique_ptr<CanonicalQuery> canonicalize(const BSONObj& queryObj) { + auto statusWithCQ = CanonicalQuery::canonicalize(ns, queryObj); + ASSERT_OK(statusWithCQ.getStatus()); + return std::move(statusWithCQ.getValue()); } -CanonicalQuery* canonicalize(const char* queryStr) { +unique_ptr<CanonicalQuery> canonicalize(const char* queryStr) { BSONObj queryObj = fromjson(queryStr); return canonicalize(queryObj); } -CanonicalQuery* canonicalize(const char* queryStr, const char* sortStr, const char* projStr) { +unique_ptr<CanonicalQuery> canonicalize(const char* queryStr, + const char* sortStr, + const char* projStr) { BSONObj queryObj = fromjson(queryStr); BSONObj sortObj = fromjson(sortStr); BSONObj projObj = fromjson(projStr); - CanonicalQuery* cq; - Status result = CanonicalQuery::canonicalize(ns, queryObj, sortObj, projObj, &cq); - ASSERT_OK(result); - return cq; + auto statusWithCQ = CanonicalQuery::canonicalize(ns, queryObj, sortObj, projObj); + ASSERT_OK(statusWithCQ.getStatus()); + return std::move(statusWithCQ.getValue()); } -CanonicalQuery* canonicalize(const char* queryStr, - const char* sortStr, - const char* projStr, - long long skip, - long long limit, - const char* hintStr, - const char* minStr, - const char* maxStr) { +unique_ptr<CanonicalQuery> canonicalize(const char* queryStr, + const char* sortStr, + const char* projStr, + long long skip, + long long limit, + const char* hintStr, + const char* minStr, + const char* maxStr) { BSONObj queryObj = fromjson(queryStr); BSONObj sortObj = fromjson(sortStr); BSONObj projObj = fromjson(projStr); BSONObj hintObj = fromjson(hintStr); BSONObj minObj = fromjson(minStr); BSONObj maxObj = fromjson(maxStr); - CanonicalQuery* cq; - Status result = CanonicalQuery::canonicalize(ns, - queryObj, - sortObj, - projObj, - skip, - limit, - hintObj, - minObj, - maxObj, - false, // snapshot - false, // explain - &cq); - ASSERT_OK(result); - return cq; + auto statusWithCQ = CanonicalQuery::canonicalize(ns, + queryObj, + sortObj, + projObj, + skip, + limit, + hintObj, + minObj, + maxObj, + false, // snapshot + false); // explain + ASSERT_OK(statusWithCQ.getStatus()); + return std::move(statusWithCQ.getValue()); } -CanonicalQuery* canonicalize(const char* queryStr, - const char* sortStr, - const char* projStr, - long long skip, - long long limit, - const char* hintStr, - const char* minStr, - const char* maxStr, - bool snapshot, - bool explain) { +unique_ptr<CanonicalQuery> canonicalize(const char* queryStr, + const char* sortStr, + const char* projStr, + long long skip, + long long limit, + const char* hintStr, + const char* minStr, + const char* maxStr, + bool snapshot, + bool explain) { BSONObj queryObj = fromjson(queryStr); BSONObj sortObj = fromjson(sortStr); BSONObj projObj = fromjson(projStr); BSONObj hintObj = fromjson(hintStr); BSONObj minObj = fromjson(minStr); BSONObj maxObj = fromjson(maxStr); - CanonicalQuery* cq; - Status result = CanonicalQuery::canonicalize(ns, - queryObj, - sortObj, - projObj, - skip, - limit, - hintObj, - minObj, - maxObj, - snapshot, - explain, - &cq); - ASSERT_OK(result); - return cq; + auto statusWithCQ = CanonicalQuery::canonicalize( + ns, queryObj, sortObj, projObj, skip, limit, hintObj, minObj, maxObj, snapshot, explain); + ASSERT_OK(statusWithCQ.getStatus()); + return std::move(statusWithCQ.getValue()); } /** @@ -380,7 +367,7 @@ TEST(PlanCacheTest, AddEmptySolutions) { PlanCache planCache; unique_ptr<CanonicalQuery> cq(canonicalize("{a: 1}")); std::vector<QuerySolution*> solns; - std::unique_ptr<PlanRankingDecision> decision(createDecision(1U)); + unique_ptr<PlanRankingDecision> decision(createDecision(1U)); ASSERT_NOT_OK(planCache.add(*cq, solns, decision.get())); } @@ -467,14 +454,11 @@ TEST(PlanCacheTest, NotifyOfWriteOp) { class CachePlanSelectionTest : public mongo::unittest::Test { protected: void setUp() { - cq = NULL; params.options = QueryPlannerParams::INCLUDE_COLLSCAN; addIndex(BSON("_id" << 1)); } void tearDown() { - delete cq; - for (vector<QuerySolution*>::iterator it = solns.begin(); it != solns.end(); ++it) { delete *it; } @@ -556,9 +540,6 @@ protected: const BSONObj& maxObj, bool snapshot) { // Clean up any previous state from a call to runQueryFull - delete cq; - cq = NULL; - for (vector<QuerySolution*>::iterator it = solns.begin(); it != solns.end(); ++it) { delete *it; } @@ -566,23 +547,19 @@ protected: solns.clear(); - Status s = CanonicalQuery::canonicalize(ns, - query, - sort, - proj, - skip, - limit, - hint, - minObj, - maxObj, - snapshot, - false, // explain - &cq); - if (!s.isOK()) { - cq = NULL; - } - ASSERT_OK(s); - s = QueryPlanner::plan(*cq, params, &solns); + auto statusWithCQ = CanonicalQuery::canonicalize(ns, + query, + sort, + proj, + skip, + limit, + hint, + minObj, + maxObj, + snapshot, + false); // explain + ASSERT_OK(statusWithCQ.getStatus()); + Status s = QueryPlanner::plan(*statusWithCQ.getValue(), params, &solns); ASSERT_OK(s); } @@ -651,11 +628,9 @@ protected: const BSONObj& sort, const BSONObj& proj, const QuerySolution& soln) const { - CanonicalQuery* cq; - Status s = CanonicalQuery::canonicalize(ns, query, sort, proj, &cq); - ASSERT_OK(s); - unique_ptr<CanonicalQuery> scopedCq(cq); - cq = NULL; + auto statusWithCQ = CanonicalQuery::canonicalize(ns, query, sort, proj); + ASSERT_OK(statusWithCQ.getStatus()); + unique_ptr<CanonicalQuery> scopedCq = std::move(statusWithCQ.getValue()); // Create a CachedSolution the long way.. // QuerySolution -> PlanCacheEntry -> CachedSolution @@ -667,7 +642,7 @@ protected: CachedSolution cachedSoln(ck, entry); QuerySolution* out; - s = QueryPlanner::planFromCache(*scopedCq.get(), params, cachedSoln, &out); + Status s = QueryPlanner::planFromCache(*scopedCq, params, cachedSoln, &out); ASSERT_OK(s); return out; @@ -756,7 +731,6 @@ protected: static const PlanCacheKey ck; BSONObj queryObj; - CanonicalQuery* cq; QueryPlannerParams params; vector<QuerySolution*> solns; }; diff --git a/src/mongo/db/query/query_planner_test.cpp b/src/mongo/db/query/query_planner_test.cpp index 2c0ca9167a5..4768eed4312 100644 --- a/src/mongo/db/query/query_planner_test.cpp +++ b/src/mongo/db/query/query_planner_test.cpp @@ -3761,10 +3761,9 @@ TEST(BadInputTest, CacheDataFromTaggedTree) { // No relevant index matching the index tag. relevantIndices.push_back(IndexEntry(BSON("a" << 1))); - CanonicalQuery* cq; - Status cqStatus = CanonicalQuery::canonicalize("ns", BSON("a" << 3), &cq); - ASSERT_OK(cqStatus); - std::unique_ptr<CanonicalQuery> scopedCq(cq); + auto statusWithCQ = CanonicalQuery::canonicalize("ns", BSON("a" << 3)); + ASSERT_OK(statusWithCQ.getStatus()); + std::unique_ptr<CanonicalQuery> scopedCq = std::move(statusWithCQ.getValue()); scopedCq->root()->setTag(new IndexTag(1)); s = QueryPlanner::cacheDataFromTaggedTree(scopedCq->root(), relevantIndices, &indexTree); @@ -3773,10 +3772,9 @@ TEST(BadInputTest, CacheDataFromTaggedTree) { } TEST(BadInputTest, TagAccordingToCache) { - CanonicalQuery* cq; - Status cqStatus = CanonicalQuery::canonicalize("ns", BSON("a" << 3), &cq); - ASSERT_OK(cqStatus); - std::unique_ptr<CanonicalQuery> scopedCq(cq); + auto statusWithCQ = CanonicalQuery::canonicalize("ns", BSON("a" << 3)); + ASSERT_OK(statusWithCQ.getStatus()); + std::unique_ptr<CanonicalQuery> scopedCq = std::move(statusWithCQ.getValue()); std::unique_ptr<PlanCacheIndexTree> indexTree(new PlanCacheIndexTree()); indexTree->setIndexEntry(IndexEntry(BSON("a" << 1))); @@ -3801,9 +3799,9 @@ TEST(BadInputTest, TagAccordingToCache) { ASSERT_OK(s); // Regenerate canonical query in order to clear tags. - cqStatus = CanonicalQuery::canonicalize("ns", BSON("a" << 3), &cq); - ASSERT_OK(cqStatus); - scopedCq.reset(cq); + statusWithCQ = CanonicalQuery::canonicalize("ns", BSON("a" << 3)); + ASSERT_OK(statusWithCQ.getStatus()); + scopedCq = std::move(statusWithCQ.getValue()); // Mismatched tree topology. PlanCacheIndexTree* child = new PlanCacheIndexTree(); diff --git a/src/mongo/db/query/query_planner_test_fixture.cpp b/src/mongo/db/query/query_planner_test_fixture.cpp index 1b876c5296b..496b6e37128 100644 --- a/src/mongo/db/query/query_planner_test_fixture.cpp +++ b/src/mongo/db/query/query_planner_test_fixture.cpp @@ -155,25 +155,20 @@ void QueryPlannerTest::runQueryFull(const BSONObj& query, // Clean up any previous state from a call to runQueryFull solns.clear(); - { - CanonicalQuery* rawCq; - Status s = CanonicalQuery::canonicalize(ns, - query, - sort, - proj, - skip, - limit, - hint, - minObj, - maxObj, - snapshot, - false, // explain - &rawCq); - ASSERT_OK(s); - cq.reset(rawCq); - } - - ASSERT_OK(QueryPlanner::plan(*cq, params, &solns.mutableVector())); + auto statusWithCQ = CanonicalQuery::canonicalize(ns, + query, + sort, + proj, + skip, + limit, + hint, + minObj, + maxObj, + snapshot, + false); // explain + ASSERT_OK(statusWithCQ.getStatus()); + + ASSERT_OK(QueryPlanner::plan(*statusWithCQ.getValue(), params, &solns.mutableVector())); } void QueryPlannerTest::runInvalidQuery(const BSONObj& query) { @@ -225,25 +220,20 @@ void QueryPlannerTest::runInvalidQueryFull(const BSONObj& query, bool snapshot) { solns.clear(); - { - CanonicalQuery* rawCq; - Status s = CanonicalQuery::canonicalize(ns, - query, - sort, - proj, - skip, - limit, - hint, - minObj, - maxObj, - snapshot, - false, // explain - &rawCq); - ASSERT_OK(s); - cq.reset(rawCq); - } - - Status s = QueryPlanner::plan(*cq, params, &solns.mutableVector()); + auto statusWithCQ = CanonicalQuery::canonicalize(ns, + query, + sort, + proj, + skip, + limit, + hint, + minObj, + maxObj, + snapshot, + false); // explain + ASSERT_OK(statusWithCQ.getStatus()); + + Status s = QueryPlanner::plan(*statusWithCQ.getValue(), params, &solns.mutableVector()); ASSERT_NOT_OK(s); } @@ -257,13 +247,11 @@ void QueryPlannerTest::runQueryAsCommand(const BSONObj& cmdObj) { std::unique_ptr<LiteParsedQuery> lpq( assertGet(LiteParsedQuery::makeFromFindCommand(nss, cmdObj, isExplain))); - CanonicalQuery* rawCq; WhereCallbackNoop whereCallback; - Status canonStatus = CanonicalQuery::canonicalize(lpq.release(), &rawCq, whereCallback); - ASSERT_OK(canonStatus); - cq.reset(rawCq); + auto statusWithCQ = CanonicalQuery::canonicalize(lpq.release(), whereCallback); + ASSERT_OK(statusWithCQ.getStatus()); - Status s = QueryPlanner::plan(*cq, params, &solns.mutableVector()); + Status s = QueryPlanner::plan(*statusWithCQ.getValue(), params, &solns.mutableVector()); ASSERT_OK(s); } diff --git a/src/mongo/dbtests/documentsourcetests.cpp b/src/mongo/dbtests/documentsourcetests.cpp index 834e38ddaed..971acb64317 100644 --- a/src/mongo/dbtests/documentsourcetests.cpp +++ b/src/mongo/dbtests/documentsourcetests.cpp @@ -51,6 +51,7 @@ using std::shared_ptr; using std::map; using std::set; using std::string; +using std::unique_ptr; using std::vector; static const char* const ns = "unittests.documentsourcetests"; @@ -178,11 +179,12 @@ protected: _exec.reset(); OldClientWriteContext ctx(&_opCtx, ns); - CanonicalQuery* cq; - uassertStatusOK(CanonicalQuery::canonicalize(ns, /*query=*/BSONObj(), &cq)); + auto statusWithCQ = CanonicalQuery::canonicalize(ns, /*query=*/BSONObj()); + uassertStatusOK(statusWithCQ.getStatus()); + unique_ptr<CanonicalQuery> cq = std::move(statusWithCQ.getValue()); PlanExecutor* execBare; - uassertStatusOK( - getExecutor(&_opCtx, ctx.getCollection(), cq, PlanExecutor::YIELD_MANUAL, &execBare)); + uassertStatusOK(getExecutor( + &_opCtx, ctx.getCollection(), cq.release(), PlanExecutor::YIELD_MANUAL, &execBare)); _exec.reset(execBare); _exec->saveState(); diff --git a/src/mongo/dbtests/executor_registry.cpp b/src/mongo/dbtests/executor_registry.cpp index 5a3a5649930..3601ecc80e0 100644 --- a/src/mongo/dbtests/executor_registry.cpp +++ b/src/mongo/dbtests/executor_registry.cpp @@ -72,14 +72,16 @@ public: unique_ptr<CollectionScan> scan(new CollectionScan(&_opCtx, params, ws.get(), NULL)); // Create a plan executor to hold it - CanonicalQuery* cq; - ASSERT(CanonicalQuery::canonicalize(ns(), BSONObj(), &cq).isOK()); + auto statusWithCQ = CanonicalQuery::canonicalize(ns(), BSONObj()); + ASSERT_OK(statusWithCQ.getStatus()); + std::unique_ptr<CanonicalQuery> cq = std::move(statusWithCQ.getValue()); + PlanExecutor* exec; // Takes ownership of 'ws', 'scan', and 'cq'. Status status = PlanExecutor::make(&_opCtx, ws.release(), scan.release(), - cq, + cq.release(), _ctx->db()->getCollection(ns()), PlanExecutor::YIELD_MANUAL, &exec); diff --git a/src/mongo/dbtests/oplogstarttests.cpp b/src/mongo/dbtests/oplogstarttests.cpp index 1c73c533e6b..2d685959a24 100644 --- a/src/mongo/dbtests/oplogstarttests.cpp +++ b/src/mongo/dbtests/oplogstarttests.cpp @@ -83,10 +83,9 @@ protected: } void setupFromQuery(const BSONObj& query) { - CanonicalQuery* cq; - Status s = CanonicalQuery::canonicalize(ns(), query, &cq); - ASSERT(s.isOK()); - _cq.reset(cq); + auto statusWithCQ = CanonicalQuery::canonicalize(ns(), query); + ASSERT_OK(statusWithCQ.getStatus()); + _cq = std::move(statusWithCQ.getValue()); _oplogws.reset(new WorkingSet()); _stage.reset(new OplogStart(&_txn, collection(), _cq->root(), _oplogws.get())); } diff --git a/src/mongo/dbtests/plan_ranking.cpp b/src/mongo/dbtests/plan_ranking.cpp index 77e34fc95f3..ef9e72bfdeb 100644 --- a/src/mongo/dbtests/plan_ranking.cpp +++ b/src/mongo/dbtests/plan_ranking.cpp @@ -97,7 +97,7 @@ public: * Use the MultiPlanRunner to pick the best plan for the query 'cq'. Goes through * normal planning to generate solutions and feeds them to the MPR. * - * Takes ownership of 'cq'. Caller DOES NOT own the returned QuerySolution*. + * Does NOT take ownership of 'cq'. Caller DOES NOT own the returned QuerySolution*. */ QuerySolution* pickBestPlan(CanonicalQuery* cq) { AutoGetCollectionForRead ctx(&_txn, ns); @@ -117,7 +117,7 @@ public: // Fill out the MPR. _mps.reset(new MultiPlanStage(&_txn, collection, cq)); - std::unique_ptr<WorkingSet> ws(new WorkingSet()); + unique_ptr<WorkingSet> ws(new WorkingSet()); // Put each solution from the planner into the MPR. for (size_t i = 0; i < solutions.size(); ++i) { PlanStage* root; @@ -185,15 +185,18 @@ public: addIndex(BSON("a" << 1)); addIndex(BSON("b" << 1)); + unique_ptr<CanonicalQuery> cq; + // Run the query {a:4, b:1}. - CanonicalQuery* cq; - verify(CanonicalQuery::canonicalize(ns, BSON("a" << 100 << "b" << 1), &cq).isOK()); - ASSERT(NULL != cq); - std::unique_ptr<CanonicalQuery> killCq(cq); + { + auto statusWithCQ = CanonicalQuery::canonicalize(ns, BSON("a" << 100 << "b" << 1)); + verify(statusWithCQ.isOK()); + cq = std::move(statusWithCQ.getValue()); + ASSERT(cq.get()); + } // {a:100} is super selective so choose that. - // Takes ownership of cq. - QuerySolution* soln = pickBestPlan(cq); + QuerySolution* soln = pickBestPlan(cq.get()); ASSERT(QueryPlannerTestLib::solutionMatches( "{fetch: {filter: {b:1}, node: {ixscan: {pattern: {a: 1}}}}}", soln->root.get())); @@ -202,14 +205,16 @@ public: internalQueryForceIntersectionPlans = true; // And run the same query again. - ASSERT(CanonicalQuery::canonicalize(ns, BSON("a" << 100 << "b" << 1), &cq).isOK()); - std::unique_ptr<CanonicalQuery> killCq2(cq); + { + auto statusWithCQ = CanonicalQuery::canonicalize(ns, BSON("a" << 100 << "b" << 1)); + verify(statusWithCQ.isOK()); + cq = std::move(statusWithCQ.getValue()); + } // With the "ranking picks ixisect always" option we pick an intersection plan that uses // both the {a:1} and {b:1} indices even though it performs poorly. - // Takes ownership of cq. - soln = pickBestPlan(cq); + soln = pickBestPlan(cq.get()); ASSERT(QueryPlannerTestLib::solutionMatches( "{fetch: {node: {andSorted: {nodes: [" "{ixscan: {filter: null, pattern: {a:1}}}," @@ -234,18 +239,17 @@ public: addIndex(BSON("b" << 1)); // Run the query {a:1, b:{$gt:1}. - CanonicalQuery* cq; - verify(CanonicalQuery::canonicalize(ns, BSON("a" << 1 << "b" << BSON("$gt" << 1)), &cq) - .isOK()); - ASSERT(NULL != cq); - std::unique_ptr<CanonicalQuery> killCq(cq); + auto statusWithCQ = + CanonicalQuery::canonicalize(ns, BSON("a" << 1 << "b" << BSON("$gt" << 1))); + verify(statusWithCQ.isOK()); + unique_ptr<CanonicalQuery> cq = std::move(statusWithCQ.getValue()); + ASSERT(NULL != cq.get()); // Turn on the "force intersect" option. // This will be reverted by PlanRankingTestBase's destructor when the test completes. internalQueryForceIntersectionPlans = true; - // Takes ownership of cq. - QuerySolution* soln = pickBestPlan(cq); + QuerySolution* soln = pickBestPlan(cq.get()); ASSERT(QueryPlannerTestLib::solutionMatches( "{fetch: {node: {andHash: {nodes: [" "{ixscan: {filter: null, pattern: {a:1}}}," @@ -274,15 +278,13 @@ public: addIndex(BSON("a" << 1 << "b" << 1)); // Query for a==27 with projection that wants 'a' and 'b'. BSONObj() is for sort. - CanonicalQuery* cq; - ASSERT(CanonicalQuery::canonicalize( - ns, BSON("a" << 27), BSONObj(), BSON("_id" << 0 << "a" << 1 << "b" << 1), &cq) - .isOK()); - ASSERT(NULL != cq); - std::unique_ptr<CanonicalQuery> killCq(cq); + auto statusWithCQ = CanonicalQuery::canonicalize( + ns, BSON("a" << 27), BSONObj(), BSON("_id" << 0 << "a" << 1 << "b" << 1)); + ASSERT_OK(statusWithCQ.getStatus()); + unique_ptr<CanonicalQuery> cq = std::move(statusWithCQ.getValue()); + ASSERT(NULL != cq.get()); - // Takes ownership of cq. - QuerySolution* soln = pickBestPlan(cq); + QuerySolution* soln = pickBestPlan(cq.get()); // Prefer the fully covered plan. ASSERT(QueryPlannerTestLib::solutionMatches( @@ -309,14 +311,13 @@ public: addIndex(BSON("b" << 1)); // There is no data that matches this query but we don't know that until EOF. - CanonicalQuery* cq; BSONObj queryObj = BSON("a" << 1 << "b" << 1 << "c" << 99); - ASSERT(CanonicalQuery::canonicalize(ns, queryObj, &cq).isOK()); - ASSERT(NULL != cq); - std::unique_ptr<CanonicalQuery> killCq(cq); + auto statusWithCQ = CanonicalQuery::canonicalize(ns, queryObj); + ASSERT_OK(statusWithCQ.getStatus()); + unique_ptr<CanonicalQuery> cq = std::move(statusWithCQ.getValue()); + ASSERT(NULL != cq.get()); - // Takes ownership of cq. - QuerySolution* soln = pickBestPlan(cq); + QuerySolution* soln = pickBestPlan(cq.get()); // Anti-prefer the intersection plan. bool bestIsScanOverA = QueryPlannerTestLib::solutionMatches( @@ -347,15 +348,13 @@ public: // There is no data that matches this query ({a:2}). Both scans will hit EOF before // returning any data. - CanonicalQuery* cq; - ASSERT(CanonicalQuery::canonicalize( - ns, BSON("a" << 2), BSONObj(), BSON("_id" << 0 << "a" << 1 << "b" << 1), &cq) - .isOK()); - ASSERT(NULL != cq); - std::unique_ptr<CanonicalQuery> killCq(cq); + auto statusWithCQ = CanonicalQuery::canonicalize( + ns, BSON("a" << 2), BSONObj(), BSON("_id" << 0 << "a" << 1 << "b" << 1)); + ASSERT_OK(statusWithCQ.getStatus()); + unique_ptr<CanonicalQuery> cq = std::move(statusWithCQ.getValue()); + ASSERT(NULL != cq.get()); - // Takes ownership of cq. - QuerySolution* soln = pickBestPlan(cq); + QuerySolution* soln = pickBestPlan(cq.get()); // Prefer the fully covered plan. ASSERT(QueryPlannerTestLib::solutionMatches( "{proj: {spec: {_id:0, a:1, b:1}, node: {ixscan: {pattern: {a: 1, b:1}}}}}", @@ -381,14 +380,13 @@ public: addIndex(BSON("b" << 1)); // Run the query {a:N+1, b:1}. (No such document.) - CanonicalQuery* cq; - verify(CanonicalQuery::canonicalize(ns, BSON("a" << N + 1 << "b" << 1), &cq).isOK()); - ASSERT(NULL != cq); - std::unique_ptr<CanonicalQuery> killCq(cq); + auto statusWithCQ = CanonicalQuery::canonicalize(ns, BSON("a" << N + 1 << "b" << 1)); + verify(statusWithCQ.isOK()); + unique_ptr<CanonicalQuery> cq = std::move(statusWithCQ.getValue()); + ASSERT(NULL != cq.get()); // {a: 100} is super selective so choose that. - // Takes ownership of cq. - QuerySolution* soln = pickBestPlan(cq); + QuerySolution* soln = pickBestPlan(cq.get()); ASSERT(QueryPlannerTestLib::solutionMatches( "{fetch: {filter: {b:1}, node: {ixscan: {pattern: {a: 1}}}}}", soln->root.get())); } @@ -416,15 +414,14 @@ public: addIndex(BSON("b" << 1)); // Run the query {a:N+1, b:1}. (No such document.) - CanonicalQuery* cq; - verify(CanonicalQuery::canonicalize(ns, BSON("a" << BSON("$gte" << N + 1) << "b" << 1), &cq) - .isOK()); - ASSERT(NULL != cq); - std::unique_ptr<CanonicalQuery> killCq(cq); + auto statusWithCQ = + CanonicalQuery::canonicalize(ns, BSON("a" << BSON("$gte" << N + 1) << "b" << 1)); + verify(statusWithCQ.isOK()); + unique_ptr<CanonicalQuery> cq = std::move(statusWithCQ.getValue()); + ASSERT(NULL != cq.get()); // {a: 100} is super selective so choose that. - // Takes ownership of cq. - QuerySolution* soln = pickBestPlan(cq); + QuerySolution* soln = pickBestPlan(cq.get()); ASSERT(QueryPlannerTestLib::solutionMatches( "{fetch: {filter: {b:1}, node: {ixscan: {pattern: {a: 1}}}}}", soln->root.get())); } @@ -445,15 +442,14 @@ public: // Run a query with a sort. The blocking sort won't produce any data during the // evaluation period. - CanonicalQuery* cq; BSONObj queryObj = BSON("_id" << BSON("$gte" << 20 << "$lte" << 200)); BSONObj sortObj = BSON("c" << 1); BSONObj projObj = BSONObj(); - ASSERT(CanonicalQuery::canonicalize(ns, queryObj, sortObj, projObj, &cq).isOK()); - std::unique_ptr<CanonicalQuery> killCq(cq); + auto statusWithCQ = CanonicalQuery::canonicalize(ns, queryObj, sortObj, projObj); + ASSERT_OK(statusWithCQ.getStatus()); + unique_ptr<CanonicalQuery> cq = std::move(statusWithCQ.getValue()); - // Takes ownership of cq. - QuerySolution* soln = pickBestPlan(cq); + QuerySolution* soln = pickBestPlan(cq.get()); // The best must not be a collscan. ASSERT(QueryPlannerTestLib::solutionMatches( @@ -476,13 +472,12 @@ public: } // Look for A Space Odyssey. - CanonicalQuery* cq; - verify(CanonicalQuery::canonicalize(ns, BSON("foo" << 2001), &cq).isOK()); - ASSERT(NULL != cq); - std::unique_ptr<CanonicalQuery> killCq(cq); + auto statusWithCQ = CanonicalQuery::canonicalize(ns, BSON("foo" << 2001)); + verify(statusWithCQ.isOK()); + unique_ptr<CanonicalQuery> cq = std::move(statusWithCQ.getValue()); + ASSERT(NULL != cq.get()); - // Takes ownership of cq. - QuerySolution* soln = pickBestPlan(cq); + QuerySolution* soln = pickBestPlan(cq.get()); // The best must be a collscan. ASSERT(QueryPlannerTestLib::solutionMatches("{cscan: {dir: 1, filter: {foo: 2001}}}", @@ -508,19 +503,18 @@ public: addIndex(BSON("d" << 1 << "e" << 1)); // Query: find({a: 1}).sort({d: 1}) - CanonicalQuery* cq; - ASSERT(CanonicalQuery::canonicalize(ns, - BSON("a" << 1), - BSON("d" << 1), // sort - BSONObj(), // projection - &cq).isOK()); - ASSERT(NULL != cq); - std::unique_ptr<CanonicalQuery> killCq(cq); + auto statusWithCQ = CanonicalQuery::canonicalize(ns, + BSON("a" << 1), + BSON("d" << 1), // sort + BSONObj()); // projection + ASSERT_OK(statusWithCQ.getStatus()); + unique_ptr<CanonicalQuery> cq = std::move(statusWithCQ.getValue()); + ASSERT(NULL != cq.get()); // No results will be returned during the trial period, // so we expect to choose {d: 1, e: 1}, as it allows us // to avoid the sort stage. - QuerySolution* soln = pickBestPlan(cq); + QuerySolution* soln = pickBestPlan(cq.get()); ASSERT(QueryPlannerTestLib::solutionMatches( "{fetch: {filter: {a:1}, node: " "{ixscan: {filter: null, pattern: {d:1,e:1}}}}}", @@ -547,14 +541,14 @@ public: // Solutions using either 'a' or 'b' will take a long time to start producing // results. However, an index scan on 'b' will start producing results sooner // than an index scan on 'a'. - CanonicalQuery* cq; - ASSERT(CanonicalQuery::canonicalize(ns, fromjson("{a: 1, b: 1, c: {$gte: 5000}}"), &cq) - .isOK()); - ASSERT(NULL != cq); - std::unique_ptr<CanonicalQuery> killCq(cq); + auto statusWithCQ = + CanonicalQuery::canonicalize(ns, fromjson("{a: 1, b: 1, c: {$gte: 5000}}")); + ASSERT_OK(statusWithCQ.getStatus()); + unique_ptr<CanonicalQuery> cq = std::move(statusWithCQ.getValue()); + ASSERT(NULL != cq.get()); // Use index on 'b'. - QuerySolution* soln = pickBestPlan(cq); + QuerySolution* soln = pickBestPlan(cq.get()); ASSERT(QueryPlannerTestLib::solutionMatches("{fetch: {node: {ixscan: {pattern: {b: 1}}}}}", soln->root.get())); } @@ -578,14 +572,14 @@ public: addIndex(BSON("b" << 1 << "c" << 1)); addIndex(BSON("a" << 1)); - CanonicalQuery* cq; - ASSERT( - CanonicalQuery::canonicalize(ns, fromjson("{a: 9, b: {$ne: 10}, c: 9}"), &cq).isOK()); - ASSERT(NULL != cq); - std::unique_ptr<CanonicalQuery> killCq(cq); + auto statusWithCQ = + CanonicalQuery::canonicalize(ns, fromjson("{a: 9, b: {$ne: 10}, c: 9}")); + ASSERT_OK(statusWithCQ.getStatus()); + unique_ptr<CanonicalQuery> cq = std::move(statusWithCQ.getValue()); + ASSERT(NULL != cq.get()); // Expect to use index {a: 1, b: 1}. - QuerySolution* soln = pickBestPlan(cq); + QuerySolution* soln = pickBestPlan(cq.get()); ASSERT(QueryPlannerTestLib::solutionMatches("{fetch: {node: {ixscan: {pattern: {a: 1}}}}}", soln->root.get())); } diff --git a/src/mongo/dbtests/query_multi_plan_runner.cpp b/src/mongo/dbtests/query_multi_plan_runner.cpp index 50b957f4731..02e66609dc2 100644 --- a/src/mongo/dbtests/query_multi_plan_runner.cpp +++ b/src/mongo/dbtests/query_multi_plan_runner.cpp @@ -63,7 +63,7 @@ using std::vector; * Create query solution. */ QuerySolution* createQuerySolution() { - std::unique_ptr<QuerySolution> soln(new QuerySolution()); + unique_ptr<QuerySolution> soln(new QuerySolution()); soln->cacheData.reset(new SolutionCacheData()); soln->cacheData->solnType = SolutionCacheData::COLLSCAN_SOLN; soln->cacheData->tree.reset(new PlanCacheIndexTree()); @@ -152,11 +152,12 @@ public: new CollectionScan(&_txn, csparams, sharedWs.get(), filter.get())); // Hand the plans off to the runner. - CanonicalQuery* cq = NULL; - verify(CanonicalQuery::canonicalize(ns(), BSON("foo" << 7), &cq).isOK()); - verify(NULL != cq); + auto statusWithCQ = CanonicalQuery::canonicalize(ns(), BSON("foo" << 7)); + verify(statusWithCQ.isOK()); + unique_ptr<CanonicalQuery> cq = std::move(statusWithCQ.getValue()); + verify(NULL != cq.get()); - MultiPlanStage* mps = new MultiPlanStage(&_txn, ctx.getCollection(), cq); + MultiPlanStage* mps = new MultiPlanStage(&_txn, ctx.getCollection(), cq.get()); mps->addPlan(createQuerySolution(), firstRoot.release(), sharedWs.get()); mps->addPlan(createQuerySolution(), secondRoot.release(), sharedWs.get()); @@ -168,10 +169,15 @@ public: // Takes ownership of arguments other than 'collection'. PlanExecutor* rawExec; - Status status = PlanExecutor::make( - &_txn, sharedWs.release(), mps, cq, coll, PlanExecutor::YIELD_MANUAL, &rawExec); + Status status = PlanExecutor::make(&_txn, + sharedWs.release(), + mps, + cq.release(), + coll, + PlanExecutor::YIELD_MANUAL, + &rawExec); ASSERT_OK(status); - std::unique_ptr<PlanExecutor> exec(rawExec); + unique_ptr<PlanExecutor> exec(rawExec); // Get all our results out. int results = 0; @@ -201,14 +207,13 @@ public: Collection* collection = ctx.getCollection(); // Query for both 'a' and 'b' and sort on 'b'. - CanonicalQuery* cq; - verify(CanonicalQuery::canonicalize(ns(), - BSON("a" << 1 << "b" << 1), // query - BSON("b" << 1), // sort - BSONObj(), // proj - &cq).isOK()); - ASSERT(NULL != cq); - std::unique_ptr<CanonicalQuery> killCq(cq); + auto statusWithCQ = CanonicalQuery::canonicalize(ns(), + BSON("a" << 1 << "b" << 1), // query + BSON("b" << 1), // sort + BSONObj()); // proj + verify(statusWithCQ.isOK()); + unique_ptr<CanonicalQuery> cq = std::move(statusWithCQ.getValue()); + ASSERT(NULL != cq.get()); // Force index intersection. bool forceIxisectOldValue = internalQueryForceIntersectionPlans; @@ -216,7 +221,7 @@ public: // Get planner params. QueryPlannerParams plannerParams; - fillOutPlannerParams(&_txn, collection, cq, &plannerParams); + fillOutPlannerParams(&_txn, collection, cq.get(), &plannerParams); // Turn this off otherwise it pops up in some plans. plannerParams.options &= ~QueryPlannerParams::KEEP_MUTATIONS; @@ -230,7 +235,7 @@ public: ASSERT_EQUALS(solutions.size(), 3U); // Fill out the MultiPlanStage. - unique_ptr<MultiPlanStage> mps(new MultiPlanStage(&_txn, collection, cq)); + unique_ptr<MultiPlanStage> mps(new MultiPlanStage(&_txn, collection, cq.get())); unique_ptr<WorkingSet> ws(new WorkingSet()); // Put each solution from the planner into the MPR. for (size_t i = 0; i < solutions.size(); ++i) { diff --git a/src/mongo/dbtests/query_plan_executor.cpp b/src/mongo/dbtests/query_plan_executor.cpp index 9c977bf5336..7cbd86ab9cb 100644 --- a/src/mongo/dbtests/query_plan_executor.cpp +++ b/src/mongo/dbtests/query_plan_executor.cpp @@ -95,17 +95,23 @@ public: unique_ptr<WorkingSet> ws(new WorkingSet()); // Canonicalize the query - CanonicalQuery* cq; - verify(CanonicalQuery::canonicalize(ns(), filterObj, &cq).isOK()); - verify(NULL != cq); + auto statusWithCQ = CanonicalQuery::canonicalize(ns(), filterObj); + verify(statusWithCQ.isOK()); + unique_ptr<CanonicalQuery> cq = std::move(statusWithCQ.getValue()); + verify(NULL != cq.get()); // Make the stage. - unique_ptr<PlanStage> root(new CollectionScan(&_txn, csparams, ws.get(), cq->root())); + unique_ptr<PlanStage> root(new CollectionScan(&_txn, csparams, ws.get(), cq.get()->root())); PlanExecutor* exec; // Hand the plan off to the executor. - Status stat = PlanExecutor::make( - &_txn, ws.release(), root.release(), cq, coll, PlanExecutor::YIELD_MANUAL, &exec); + Status stat = PlanExecutor::make(&_txn, + ws.release(), + root.release(), + cq.release(), + coll, + PlanExecutor::YIELD_MANUAL, + &exec); ASSERT_OK(stat); return exec; } @@ -139,14 +145,20 @@ public: IndexScan* ix = new IndexScan(&_txn, ixparams, ws.get(), NULL); unique_ptr<PlanStage> root(new FetchStage(&_txn, ws.get(), ix, NULL, coll)); - CanonicalQuery* cq; - verify(CanonicalQuery::canonicalize(ns(), BSONObj(), &cq).isOK()); - verify(NULL != cq); + auto statusWithCQ = CanonicalQuery::canonicalize(ns(), BSONObj()); + verify(statusWithCQ.isOK()); + unique_ptr<CanonicalQuery> cq = std::move(statusWithCQ.getValue()); + verify(NULL != cq.get()); PlanExecutor* exec; // Hand the plan off to the executor. - Status stat = PlanExecutor::make( - &_txn, ws.release(), root.release(), cq, coll, PlanExecutor::YIELD_MANUAL, &exec); + Status stat = PlanExecutor::make(&_txn, + ws.release(), + root.release(), + cq.release(), + coll, + PlanExecutor::YIELD_MANUAL, + &exec); ASSERT_OK(stat); return exec; } diff --git a/src/mongo/dbtests/query_stage_cached_plan.cpp b/src/mongo/dbtests/query_stage_cached_plan.cpp index 821a329e91d..a0b7fd14b4a 100644 --- a/src/mongo/dbtests/query_stage_cached_plan.cpp +++ b/src/mongo/dbtests/query_stage_cached_plan.cpp @@ -116,9 +116,9 @@ public: ASSERT(collection); // Query can be answered by either index on "a" or index on "b". - CanonicalQuery* rawCq; - ASSERT_OK(CanonicalQuery::canonicalize(ns(), fromjson("{a: {$gte: 8}, b: 1}"), &rawCq)); - const std::unique_ptr<CanonicalQuery> cq(rawCq); + auto statusWithCQ = CanonicalQuery::canonicalize(ns(), fromjson("{a: {$gte: 8}, b: 1}")); + ASSERT_OK(statusWithCQ.getStatus()); + const std::unique_ptr<CanonicalQuery> cq = std::move(statusWithCQ.getValue()); // We shouldn't have anything in the plan cache for this shape yet. PlanCache* cache = collection->infoCache()->getPlanCache(); @@ -180,9 +180,9 @@ public: ASSERT(collection); // Query can be answered by either index on "a" or index on "b". - CanonicalQuery* rawCq; - ASSERT_OK(CanonicalQuery::canonicalize(ns(), fromjson("{a: {$gte: 8}, b: 1}"), &rawCq)); - const std::unique_ptr<CanonicalQuery> cq(rawCq); + auto statusWithCQ = CanonicalQuery::canonicalize(ns(), fromjson("{a: {$gte: 8}, b: 1}")); + ASSERT_OK(statusWithCQ.getStatus()); + const std::unique_ptr<CanonicalQuery> cq = std::move(statusWithCQ.getValue()); // We shouldn't have anything in the plan cache for this shape yet. PlanCache* cache = collection->infoCache()->getPlanCache(); diff --git a/src/mongo/dbtests/query_stage_delete.cpp b/src/mongo/dbtests/query_stage_delete.cpp index 49a2d8709ae..e3d3ace2054 100644 --- a/src/mongo/dbtests/query_stage_delete.cpp +++ b/src/mongo/dbtests/query_stage_delete.cpp @@ -97,11 +97,10 @@ public: } } - CanonicalQuery* canonicalize(const BSONObj& query) { - CanonicalQuery* cq; - Status status = CanonicalQuery::canonicalize(ns(), query, &cq); - ASSERT_OK(status); - return cq; + unique_ptr<CanonicalQuery> canonicalize(const BSONObj& query) { + auto statusWithCQ = CanonicalQuery::canonicalize(ns(), query); + ASSERT_OK(statusWithCQ.getStatus()); + return std::move(statusWithCQ.getValue()); } static size_t numObj() { @@ -195,8 +194,8 @@ public: const NamespaceString nss(ns()); const int targetDocIndex = 0; const BSONObj query = BSON("foo" << BSON("$gte" << targetDocIndex)); - const std::unique_ptr<WorkingSet> ws(stdx::make_unique<WorkingSet>()); - const std::unique_ptr<CanonicalQuery> cq(canonicalize(query)); + const unique_ptr<WorkingSet> ws(stdx::make_unique<WorkingSet>()); + const unique_ptr<CanonicalQuery> cq(canonicalize(query)); // Get the RecordIds that would be returned by an in-order scan. vector<RecordId> locs; @@ -204,7 +203,7 @@ public: // Configure a QueuedDataStage to pass the first object in the collection back in a // LOC_AND_UNOWNED_OBJ state. - std::unique_ptr<QueuedDataStage> qds(stdx::make_unique<QueuedDataStage>(ws.get())); + unique_ptr<QueuedDataStage> qds(stdx::make_unique<QueuedDataStage>(ws.get())); WorkingSetMember member; member.loc = locs[targetDocIndex]; member.state = WorkingSetMember::LOC_AND_UNOWNED_OBJ; @@ -217,7 +216,7 @@ public: deleteParams.returnDeleted = true; deleteParams.canonicalQuery = cq.get(); - const std::unique_ptr<DeleteStage> deleteStage( + const unique_ptr<DeleteStage> deleteStage( stdx::make_unique<DeleteStage>(&_txn, deleteParams, ws.get(), coll, qds.release())); const DeleteStats* stats = static_cast<const DeleteStats*>(deleteStage->getSpecificStats()); @@ -260,11 +259,11 @@ public: OldClientWriteContext ctx(&_txn, ns()); Collection* coll = ctx.getCollection(); const BSONObj query = BSONObj(); - const std::unique_ptr<WorkingSet> ws(stdx::make_unique<WorkingSet>()); - const std::unique_ptr<CanonicalQuery> cq(canonicalize(query)); + const unique_ptr<WorkingSet> ws(stdx::make_unique<WorkingSet>()); + const unique_ptr<CanonicalQuery> cq(canonicalize(query)); // Configure a QueuedDataStage to pass an OWNED_OBJ to the delete stage. - std::unique_ptr<QueuedDataStage> qds(stdx::make_unique<QueuedDataStage>(ws.get())); + unique_ptr<QueuedDataStage> qds(stdx::make_unique<QueuedDataStage>(ws.get())); WorkingSetMember member; member.state = WorkingSetMember::OWNED_OBJ; member.obj = Snapshotted<BSONObj>(SnapshotId(), fromjson("{x: 1}")); @@ -276,7 +275,7 @@ public: deleteParams.returnDeleted = true; deleteParams.canonicalQuery = cq.get(); - const std::unique_ptr<DeleteStage> deleteStage( + const unique_ptr<DeleteStage> deleteStage( stdx::make_unique<DeleteStage>(&_txn, deleteParams, ws.get(), coll, qds.release())); const DeleteStats* stats = static_cast<const DeleteStats*>(deleteStage->getSpecificStats()); diff --git a/src/mongo/dbtests/query_stage_subplan.cpp b/src/mongo/dbtests/query_stage_subplan.cpp index 244eee99283..2231240f2c4 100644 --- a/src/mongo/dbtests/query_stage_subplan.cpp +++ b/src/mongo/dbtests/query_stage_subplan.cpp @@ -88,9 +88,9 @@ public: "{$or: [{a: {$geoWithin: {$centerSphere: [[0,0],10]}}}," "{a: {$geoWithin: {$centerSphere: [[1,1],10]}}}]}"); - CanonicalQuery* rawCq; - ASSERT_OK(CanonicalQuery::canonicalize(ns(), query, &rawCq)); - std::unique_ptr<CanonicalQuery> cq(rawCq); + auto statusWithCQ = CanonicalQuery::canonicalize(ns(), query); + ASSERT_OK(statusWithCQ.getStatus()); + std::unique_ptr<CanonicalQuery> cq = std::move(statusWithCQ.getValue()); Collection* collection = ctx.getCollection(); @@ -131,9 +131,9 @@ public: Collection* collection = ctx.getCollection(); - CanonicalQuery* rawCq; - ASSERT_OK(CanonicalQuery::canonicalize(ns(), query, &rawCq)); - std::unique_ptr<CanonicalQuery> cq(rawCq); + auto statusWithCQ = CanonicalQuery::canonicalize(ns(), query); + ASSERT_OK(statusWithCQ.getStatus()); + std::unique_ptr<CanonicalQuery> cq = std::move(statusWithCQ.getValue()); // Get planner params. QueryPlannerParams plannerParams; diff --git a/src/mongo/dbtests/query_stage_update.cpp b/src/mongo/dbtests/query_stage_update.cpp index c4e33eb6802..2e80ef886aa 100644 --- a/src/mongo/dbtests/query_stage_update.cpp +++ b/src/mongo/dbtests/query_stage_update.cpp @@ -82,11 +82,10 @@ public: return _client.count(ns(), query, 0, 0, 0); } - CanonicalQuery* canonicalize(const BSONObj& query) { - CanonicalQuery* cq; - Status status = CanonicalQuery::canonicalize(ns(), query, &cq); - ASSERT_OK(status); - return cq; + unique_ptr<CanonicalQuery> canonicalize(const BSONObj& query) { + auto statusWithCQ = CanonicalQuery::canonicalize(ns(), query); + ASSERT_OK(statusWithCQ.getStatus()); + return std::move(statusWithCQ.getValue()); } /** @@ -377,8 +376,8 @@ public: UpdateDriver driver((UpdateDriver::Options())); const int targetDocIndex = 0; // We'll be working with the first doc in the collection. const BSONObj query = BSON("foo" << BSON("$gte" << targetDocIndex)); - const std::unique_ptr<WorkingSet> ws(stdx::make_unique<WorkingSet>()); - const std::unique_ptr<CanonicalQuery> cq(canonicalize(query)); + const unique_ptr<WorkingSet> ws(stdx::make_unique<WorkingSet>()); + const unique_ptr<CanonicalQuery> cq(canonicalize(query)); // Get the RecordIds that would be returned by an in-order scan. vector<RecordId> locs; @@ -396,7 +395,7 @@ public: // Configure a QueuedDataStage to pass the first object in the collection back in a // LOC_AND_UNOWNED_OBJ state. - std::unique_ptr<QueuedDataStage> qds(stdx::make_unique<QueuedDataStage>(ws.get())); + unique_ptr<QueuedDataStage> qds(stdx::make_unique<QueuedDataStage>(ws.get())); WorkingSetMember member; member.loc = locs[targetDocIndex]; member.state = WorkingSetMember::LOC_AND_UNOWNED_OBJ; @@ -408,7 +407,7 @@ public: UpdateStageParams updateParams(&request, &driver, opDebug); updateParams.canonicalQuery = cq.get(); - const std::unique_ptr<UpdateStage> updateStage( + const unique_ptr<UpdateStage> updateStage( stdx::make_unique<UpdateStage>(&_txn, updateParams, ws.get(), coll, qds.release())); // Should return advanced. @@ -464,8 +463,8 @@ public: UpdateDriver driver((UpdateDriver::Options())); const int targetDocIndex = 10; const BSONObj query = BSON("foo" << BSON("$gte" << targetDocIndex)); - const std::unique_ptr<WorkingSet> ws(stdx::make_unique<WorkingSet>()); - const std::unique_ptr<CanonicalQuery> cq(canonicalize(query)); + const unique_ptr<WorkingSet> ws(stdx::make_unique<WorkingSet>()); + const unique_ptr<CanonicalQuery> cq(canonicalize(query)); // Get the RecordIds that would be returned by an in-order scan. vector<RecordId> locs; @@ -483,7 +482,7 @@ public: // Configure a QueuedDataStage to pass the first object in the collection back in a // LOC_AND_UNOWNED_OBJ state. - std::unique_ptr<QueuedDataStage> qds(stdx::make_unique<QueuedDataStage>(ws.get())); + unique_ptr<QueuedDataStage> qds(stdx::make_unique<QueuedDataStage>(ws.get())); WorkingSetMember member; member.loc = locs[targetDocIndex]; member.state = WorkingSetMember::LOC_AND_UNOWNED_OBJ; @@ -495,7 +494,7 @@ public: UpdateStageParams updateParams(&request, &driver, opDebug); updateParams.canonicalQuery = cq.get(); - std::unique_ptr<UpdateStage> updateStage( + unique_ptr<UpdateStage> updateStage( stdx::make_unique<UpdateStage>(&_txn, updateParams, ws.get(), coll, qds.release())); // Should return advanced. @@ -544,8 +543,8 @@ public: UpdateRequest request(nsString()); UpdateDriver driver((UpdateDriver::Options())); const BSONObj query = BSONObj(); - const std::unique_ptr<WorkingSet> ws(stdx::make_unique<WorkingSet>()); - const std::unique_ptr<CanonicalQuery> cq(canonicalize(query)); + const unique_ptr<WorkingSet> ws(stdx::make_unique<WorkingSet>()); + const unique_ptr<CanonicalQuery> cq(canonicalize(query)); // Populate the request. request.setQuery(query); @@ -558,7 +557,7 @@ public: ASSERT_OK(driver.parse(request.getUpdates(), request.isMulti())); // Configure a QueuedDataStage to pass an OWNED_OBJ to the update stage. - std::unique_ptr<QueuedDataStage> qds(stdx::make_unique<QueuedDataStage>(ws.get())); + unique_ptr<QueuedDataStage> qds(stdx::make_unique<QueuedDataStage>(ws.get())); WorkingSetMember member; member.state = WorkingSetMember::OWNED_OBJ; member.obj = Snapshotted<BSONObj>(SnapshotId(), fromjson("{x: 1}")); @@ -568,7 +567,7 @@ public: UpdateStageParams updateParams(&request, &driver, opDebug); updateParams.canonicalQuery = cq.get(); - const std::unique_ptr<UpdateStage> updateStage( + const unique_ptr<UpdateStage> updateStage( stdx::make_unique<UpdateStage>(&_txn, updateParams, ws.get(), coll, qds.release())); const UpdateStats* stats = static_cast<const UpdateStats*>(updateStage->getSpecificStats()); diff --git a/src/mongo/s/chunk_manager.cpp b/src/mongo/s/chunk_manager.cpp index 73c1461c92f..e663a5dd640 100644 --- a/src/mongo/s/chunk_manager.cpp +++ b/src/mongo/s/chunk_manager.cpp @@ -55,14 +55,14 @@ namespace mongo { -using std::shared_ptr; - using std::make_pair; using std::map; using std::max; using std::pair; using std::set; +using std::shared_ptr; using std::string; +using std::unique_ptr; using std::vector; namespace { @@ -452,15 +452,13 @@ ChunkPtr ChunkManager::findIntersectingChunk(const BSONObj& shardKey) const { } void ChunkManager::getShardIdsForQuery(set<ShardId>& shardIds, const BSONObj& query) const { - CanonicalQuery* canonicalQuery = NULL; - Status status = CanonicalQuery::canonicalize(_ns, query, &canonicalQuery, WhereCallbackNoop()); + auto statusWithCQ = CanonicalQuery::canonicalize(_ns, query, WhereCallbackNoop()); - std::unique_ptr<CanonicalQuery> canonicalQueryPtr(canonicalQuery); - - uassert(status.code(), status.reason(), status.isOK()); + uassertStatusOK(statusWithCQ.getStatus()); + unique_ptr<CanonicalQuery> cq = std::move(statusWithCQ.getValue()); // Query validation - if (QueryPlannerCommon::hasNode(canonicalQuery->root(), MatchExpression::GEO_NEAR)) { + if (QueryPlannerCommon::hasNode(cq->root(), MatchExpression::GEO_NEAR)) { uassert(13501, "use geoNear command rather than $near query", false); } @@ -470,7 +468,7 @@ void ChunkManager::getShardIdsForQuery(set<ShardId>& shardIds, const BSONObj& qu // Query { a : { $gte : 1, $lt : 2 }, // b : { $gte : 3, $lt : 4 } } // => Bounds { a : [1, 2), b : [3, 4) } - IndexBounds bounds = getIndexBoundsForQuery(_keyPattern.toBSON(), canonicalQuery); + IndexBounds bounds = getIndexBoundsForQuery(_keyPattern.toBSON(), *cq); // Transforms bounds for each shard key field into full shard key ranges // for example : @@ -525,12 +523,12 @@ void ChunkManager::getAllShardIds(set<ShardId>* all) const { } IndexBounds ChunkManager::getIndexBoundsForQuery(const BSONObj& key, - const CanonicalQuery* canonicalQuery) { + const CanonicalQuery& canonicalQuery) { // $text is not allowed in planning since we don't have text index on mongos. // // TODO: Treat $text query as a no-op in planning. So with shard key {a: 1}, // the query { a: 2, $text: { ... } } will only target to {a: 2}. - if (QueryPlannerCommon::hasNode(canonicalQuery->root(), MatchExpression::TEXT)) { + if (QueryPlannerCommon::hasNode(canonicalQuery.root(), MatchExpression::TEXT)) { IndexBounds bounds; IndexBoundsBuilder::allValuesBounds(key, &bounds); // [minKey, maxKey] return bounds; @@ -555,7 +553,7 @@ IndexBounds ChunkManager::getIndexBoundsForQuery(const BSONObj& key, plannerParams.indices.push_back(indexEntry); OwnedPointerVector<QuerySolution> solutions; - Status status = QueryPlanner::plan(*canonicalQuery, plannerParams, &solutions.mutableVector()); + Status status = QueryPlanner::plan(canonicalQuery, plannerParams, &solutions.mutableVector()); uassert(status.code(), status.reason(), status.isOK()); IndexBounds bounds; diff --git a/src/mongo/s/chunk_manager.h b/src/mongo/s/chunk_manager.h index 63e563e7d67..cb68b42b858 100644 --- a/src/mongo/s/chunk_manager.h +++ b/src/mongo/s/chunk_manager.h @@ -208,7 +208,7 @@ public: // b : { $gte : 3, $lt : 4 } } // => Bounds { a : [1, 2), b : [3, 4) } static IndexBounds getIndexBoundsForQuery(const BSONObj& key, - const CanonicalQuery* canonicalQuery); + const CanonicalQuery& canonicalQuery); // Collapse query solution tree. // diff --git a/src/mongo/s/chunk_manager_targeter_test.cpp b/src/mongo/s/chunk_manager_targeter_test.cpp index 4f43580d40c..ff6d83cf0ea 100644 --- a/src/mongo/s/chunk_manager_targeter_test.cpp +++ b/src/mongo/s/chunk_manager_targeter_test.cpp @@ -53,12 +53,11 @@ using std::make_pair; */ // Utility function to create a CanonicalQuery -CanonicalQuery* canonicalize(const char* queryStr) { +unique_ptr<CanonicalQuery> canonicalize(const char* queryStr) { BSONObj queryObj = fromjson(queryStr); - CanonicalQuery* cq; - Status result = CanonicalQuery::canonicalize("test.foo", queryObj, &cq, WhereCallbackNoop()); - ASSERT_OK(result); - return cq; + auto statusWithCQ = CanonicalQuery::canonicalize("test.foo", queryObj, WhereCallbackNoop()); + ASSERT_OK(statusWithCQ.getStatus()); + return std::move(statusWithCQ.getValue()); } void checkIndexBoundsWithKey(const char* keyStr, @@ -69,7 +68,7 @@ void checkIndexBoundsWithKey(const char* keyStr, BSONObj key = fromjson(keyStr); - IndexBounds indexBounds = ChunkManager::getIndexBoundsForQuery(key, query.get()); + IndexBounds indexBounds = ChunkManager::getIndexBoundsForQuery(key, *query.get()); ASSERT_EQUALS(indexBounds.size(), expectedBounds.size()); for (size_t i = 0; i < indexBounds.size(); i++) { const OrderedIntervalList& oil = indexBounds.fields[i]; @@ -92,7 +91,7 @@ void checkIndexBounds(const char* queryStr, const OrderedIntervalList& expectedO BSONObj key = fromjson("{a: 1}"); - IndexBounds indexBounds = ChunkManager::getIndexBoundsForQuery(key, query.get()); + IndexBounds indexBounds = ChunkManager::getIndexBoundsForQuery(key, *query.get()); ASSERT_EQUALS(indexBounds.size(), 1U); const OrderedIntervalList& oil = indexBounds.fields.front(); @@ -279,7 +278,7 @@ TEST(CMCollapseTreeTest, BasicAllElemMatch) { BSONObj key = fromjson("{'foo.a': 1}"); - IndexBounds indexBounds = ChunkManager::getIndexBoundsForQuery(key, query.get()); + IndexBounds indexBounds = ChunkManager::getIndexBoundsForQuery(key, *query.get()); ASSERT_EQUALS(indexBounds.size(), 1U); const OrderedIntervalList& oil = indexBounds.fields.front(); ASSERT_EQUALS(oil.intervals.size(), 1U); @@ -359,7 +358,7 @@ TEST(CMCollapseTreeTest, HashedSinglePoint) { BSONObj key = fromjson("{a: 'hashed'}"); - IndexBounds indexBounds = ChunkManager::getIndexBoundsForQuery(key, query.get()); + IndexBounds indexBounds = ChunkManager::getIndexBoundsForQuery(key, *query.get()); ASSERT_EQUALS(indexBounds.size(), 1U); const OrderedIntervalList& oil = indexBounds.fields.front(); ASSERT_EQUALS(oil.intervals.size(), 1U); diff --git a/src/mongo/s/shard_key_pattern.cpp b/src/mongo/s/shard_key_pattern.cpp index f411461b925..24485148810 100644 --- a/src/mongo/s/shard_key_pattern.cpp +++ b/src/mongo/s/shard_key_pattern.cpp @@ -265,12 +265,11 @@ StatusWith<BSONObj> ShardKeyPattern::extractShardKeyFromQuery(const BSONObj& bas return StatusWith<BSONObj>(BSONObj()); // Extract equalities from query - CanonicalQuery* rawQuery; - Status queryStatus = - CanonicalQuery::canonicalize("", basicQuery, &rawQuery, WhereCallbackNoop()); - if (!queryStatus.isOK()) - return StatusWith<BSONObj>(queryStatus); - unique_ptr<CanonicalQuery> query(rawQuery); + auto statusWithCQ = CanonicalQuery::canonicalize("", basicQuery, WhereCallbackNoop()); + if (!statusWithCQ.isOK()) { + return StatusWith<BSONObj>(statusWithCQ.getStatus()); + } + unique_ptr<CanonicalQuery> query = std::move(statusWithCQ.getValue()); EqualityMatches equalities; // TODO: Build the path set initially? |