diff options
author | Qingyang Chen <qingyang.chen@10gen.com> | 2015-06-16 16:16:12 -0400 |
---|---|---|
committer | Qingyang Chen <qingyang.chen@10gen.com> | 2015-06-22 13:17:31 -0400 |
commit | 3f6f66daac840fe2b2dc26eeeacbf015479567df (patch) | |
tree | 3b8f907c22f9ec41f1a72d1ea71f1dff8376a77d /src/mongo/db/query | |
parent | 1e786585ae83e55e13016a6761a121b502c887f8 (diff) | |
download | mongo-3f6f66daac840fe2b2dc26eeeacbf015479567df.tar.gz |
SERVER-16889 query subsystem CanonicalQuery::canonicalize use StatusWith<unique_ptr> for ownership transfer
Diffstat (limited to 'src/mongo/db/query')
-rw-r--r-- | src/mongo/db/query/canonical_query.cpp | 142 | ||||
-rw-r--r-- | src/mongo/db/query/canonical_query.h | 162 | ||||
-rw-r--r-- | src/mongo/db/query/canonical_query_test.cpp | 25 | ||||
-rw-r--r-- | src/mongo/db/query/find.cpp | 31 | ||||
-rw-r--r-- | src/mongo/db/query/get_executor.cpp | 84 | ||||
-rw-r--r-- | src/mongo/db/query/get_executor_test.cpp | 11 | ||||
-rw-r--r-- | src/mongo/db/query/plan_cache_test.cpp | 154 | ||||
-rw-r--r-- | src/mongo/db/query/query_planner_test.cpp | 20 | ||||
-rw-r--r-- | src/mongo/db/query/query_planner_test_fixture.cpp | 74 |
9 files changed, 326 insertions, 377 deletions
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..8036dac3adb 100644 --- a/src/mongo/db/query/canonical_query_test.cpp +++ b/src/mongo/db/query/canonical_query_test.cpp @@ -35,7 +35,6 @@ namespace mongo { namespace { using std::string; -using std::unique_ptr; using unittest::assertGet; static const char* ns = "somebogusns"; @@ -52,6 +51,7 @@ MatchExpression* parseMatchExpression(const BSONObj& obj) { << ". Reason: " << status.getStatus().toString(); FAIL(ss); } + return status.getValue(); } @@ -62,8 +62,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 +460,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); } |