From 160001354901fb46fff1e6f1b4bc0a7db0407cf6 Mon Sep 17 00:00:00 2001 From: Qingyang Chen Date: Mon, 29 Jun 2015 14:10:06 -0400 Subject: SERVER-18932 LiteParsedQuery::_ns use const NamespaceString rather than std::string --- src/mongo/db/query/canonical_query.cpp | 17 ++++++++++--- src/mongo/db/query/canonical_query.h | 6 +++-- src/mongo/db/query/canonical_query_test.cpp | 24 +++++++++--------- src/mongo/db/query/lite_parsed_query.cpp | 35 ++++++++++++--------------- src/mongo/db/query/lite_parsed_query.h | 23 ++++++++++-------- src/mongo/db/query/lite_parsed_query_test.cpp | 28 +++++++++++---------- 6 files changed, 74 insertions(+), 59 deletions(-) (limited to 'src/mongo/db') diff --git a/src/mongo/db/query/canonical_query.cpp b/src/mongo/db/query/canonical_query.cpp index 9392c62c09b..145b65e0fad 100644 --- a/src/mongo/db/query/canonical_query.cpp +++ b/src/mongo/db/query/canonical_query.cpp @@ -33,6 +33,7 @@ #include "mongo/db/query/canonical_query.h" #include "mongo/db/jsobj.h" +#include "mongo/db/namespace_string.h" #include "mongo/db/query/query_planner_common.h" #include "mongo/util/log.h" @@ -239,7 +240,7 @@ StatusWith> CanonicalQuery::canonicalize( // 0, 0, 0 is 'ntoskip', 'ntoreturn', and 'queryoptions' // false, false is 'snapshot' and 'explain' - auto lpqStatus = LiteParsedQuery::makeAsOpQuery(baseQuery.ns(), + auto lpqStatus = LiteParsedQuery::makeAsOpQuery(baseQuery.nss(), 0, 0, 0, @@ -283,8 +284,18 @@ StatusWith> CanonicalQuery::canonicalize( // Pass empty sort and projection. BSONObj emptyObj; - auto lpqStatus = LiteParsedQuery::makeAsOpQuery( - ns, skip, limit, 0, query, proj, sort, hint, minObj, maxObj, snapshot, explain); + auto lpqStatus = LiteParsedQuery::makeAsOpQuery(NamespaceString(ns), + skip, + limit, + 0, + query, + proj, + sort, + hint, + minObj, + maxObj, + snapshot, + explain); if (!lpqStatus.isOK()) { return lpqStatus.getStatus(); } diff --git a/src/mongo/db/query/canonical_query.h b/src/mongo/db/query/canonical_query.h index 56f06a20c9d..81df734f1f9 100644 --- a/src/mongo/db/query/canonical_query.h +++ b/src/mongo/db/query/canonical_query.h @@ -152,9 +152,11 @@ public: */ static bool isSimpleIdQuery(const BSONObj& query); - // What namespace is this query over? + const NamespaceString& nss() const { + return _pq->nss(); + } const std::string& ns() const { - return _pq->ns(); + return _pq->nss().ns(); } // diff --git a/src/mongo/db/query/canonical_query_test.cpp b/src/mongo/db/query/canonical_query_test.cpp index ff4df2026d9..38f1e3144d4 100644 --- a/src/mongo/db/query/canonical_query_test.cpp +++ b/src/mongo/db/query/canonical_query_test.cpp @@ -38,7 +38,7 @@ using std::string; using std::unique_ptr; using unittest::assertGet; -static const char* ns = "somebogusns"; +static const NamespaceString nss("testdb.testcoll"); /** * Helper function to parse the given BSON object as a MatchExpression, checks the status, @@ -97,7 +97,7 @@ void assertNotEquivalent(const char* queryStr, TEST(CanonicalQueryTest, IsValidText) { // Passes in default values for LiteParsedQuery. // Filter inside LiteParsedQuery is not used. - unique_ptr lpq(assertGet(LiteParsedQuery::makeAsOpQuery(ns, + unique_ptr lpq(assertGet(LiteParsedQuery::makeAsOpQuery(nss, 0, 0, 0, @@ -164,7 +164,7 @@ TEST(CanonicalQueryTest, IsValidText) { TEST(CanonicalQueryTest, IsValidGeo) { // Passes in default values for LiteParsedQuery. // Filter inside LiteParsedQuery is not used. - unique_ptr lpq(assertGet(LiteParsedQuery::makeAsOpQuery(ns, + unique_ptr lpq(assertGet(LiteParsedQuery::makeAsOpQuery(nss, 0, 0, 0, @@ -241,7 +241,7 @@ TEST(CanonicalQueryTest, IsValidGeo) { TEST(CanonicalQueryTest, IsValidTextAndGeo) { // Passes in default values for LiteParsedQuery. // Filter inside LiteParsedQuery is not used. - unique_ptr lpq(assertGet(LiteParsedQuery::makeAsOpQuery(ns, + unique_ptr lpq(assertGet(LiteParsedQuery::makeAsOpQuery(nss, 0, 0, 0, @@ -274,7 +274,7 @@ TEST(CanonicalQueryTest, IsValidTextAndNaturalAscending) { // Passes in default values for LiteParsedQuery except for sort order. // Filter inside LiteParsedQuery is not used. BSONObj sort = fromjson("{$natural: 1}"); - unique_ptr lpq(assertGet(LiteParsedQuery::makeAsOpQuery(ns, + unique_ptr lpq(assertGet(LiteParsedQuery::makeAsOpQuery(nss, 0, 0, 0, @@ -295,7 +295,7 @@ TEST(CanonicalQueryTest, IsValidTextAndNaturalDescending) { // Passes in default values for LiteParsedQuery except for sort order. // Filter inside LiteParsedQuery is not used. BSONObj sort = fromjson("{$natural: -1}"); - unique_ptr lpq(assertGet(LiteParsedQuery::makeAsOpQuery(ns, + unique_ptr lpq(assertGet(LiteParsedQuery::makeAsOpQuery(nss, 0, 0, 0, @@ -316,7 +316,7 @@ TEST(CanonicalQueryTest, IsValidTextAndHint) { // Passes in default values for LiteParsedQuery except for hint. // Filter inside LiteParsedQuery is not used. BSONObj hint = fromjson("{a: 1}"); - unique_ptr lpq(assertGet(LiteParsedQuery::makeAsOpQuery(ns, + unique_ptr lpq(assertGet(LiteParsedQuery::makeAsOpQuery(nss, 0, 0, 0, @@ -338,7 +338,7 @@ TEST(CanonicalQueryTest, IsValidGeoNearNaturalSort) { // Passes in default values for LiteParsedQuery except for sort order. // Filter inside LiteParsedQuery is not used. BSONObj sort = fromjson("{$natural: 1}"); - unique_ptr lpq(assertGet(LiteParsedQuery::makeAsOpQuery(ns, + unique_ptr lpq(assertGet(LiteParsedQuery::makeAsOpQuery(nss, 0, 0, 0, @@ -360,7 +360,7 @@ TEST(CanonicalQueryTest, IsValidGeoNearNaturalHint) { // Passes in default values for LiteParsedQuery except for the hint. // Filter inside LiteParsedQuery is not used. BSONObj hint = fromjson("{$natural: 1}"); - unique_ptr lpq(assertGet(LiteParsedQuery::makeAsOpQuery(ns, + unique_ptr lpq(assertGet(LiteParsedQuery::makeAsOpQuery(nss, 0, 0, 0, @@ -381,7 +381,7 @@ TEST(CanonicalQueryTest, IsValidTextAndSnapshot) { // Passes in default values for LiteParsedQuery except for snapshot. // Filter inside LiteParsedQuery is not used. bool snapshot = true; - unique_ptr lpq(assertGet(LiteParsedQuery::makeAsOpQuery(ns, + unique_ptr lpq(assertGet(LiteParsedQuery::makeAsOpQuery(nss, 0, 0, 0, @@ -463,7 +463,7 @@ TEST(CanonicalQueryTest, SortTreeNumChildrenComparison) { */ unique_ptr canonicalize(const char* queryStr) { BSONObj queryObj = fromjson(queryStr); - auto statusWithCQ = CanonicalQuery::canonicalize(ns, queryObj); + auto statusWithCQ = CanonicalQuery::canonicalize(nss, queryObj); ASSERT_OK(statusWithCQ.getStatus()); return std::move(statusWithCQ.getValue()); } @@ -474,7 +474,7 @@ std::unique_ptr canonicalize(const char* queryStr, BSONObj queryObj = fromjson(queryStr); BSONObj sortObj = fromjson(sortStr); BSONObj projObj = fromjson(projStr); - auto statusWithCQ = CanonicalQuery::canonicalize(ns, queryObj, sortObj, projObj); + auto statusWithCQ = CanonicalQuery::canonicalize(nss, queryObj, sortObj, projObj); ASSERT_OK(statusWithCQ.getStatus()); return std::move(statusWithCQ.getValue()); } diff --git a/src/mongo/db/query/lite_parsed_query.cpp b/src/mongo/db/query/lite_parsed_query.cpp index c7c8e5e933c..43fea7af3c0 100644 --- a/src/mongo/db/query/lite_parsed_query.cpp +++ b/src/mongo/db/query/lite_parsed_query.cpp @@ -41,8 +41,8 @@ namespace mongo { -using std::unique_ptr; using std::string; +using std::unique_ptr; const string LiteParsedQuery::cmdOptionMaxTimeMS("maxTimeMS"); const string LiteParsedQuery::queryOptionMaxTimeMS("$maxTimeMS"); @@ -93,11 +93,13 @@ const char kPartialField[] = "partial"; } // namespace +LiteParsedQuery::LiteParsedQuery(NamespaceString nss) : _nss(std::move(nss)) {} + // static -StatusWith> LiteParsedQuery::makeFromFindCommand( - const NamespaceString& nss, const BSONObj& cmdObj, bool isExplain) { - unique_ptr pq(new LiteParsedQuery()); - pq->_ns = nss.ns(); +StatusWith> LiteParsedQuery::makeFromFindCommand(NamespaceString nss, + const BSONObj& cmdObj, + bool isExplain) { + unique_ptr pq(new LiteParsedQuery(std::move(nss))); pq->_fromCommand = true; pq->_explain = isExplain; @@ -350,7 +352,7 @@ StatusWith> LiteParsedQuery::makeFromFindCommand( } // static -StatusWith> LiteParsedQuery::makeAsOpQuery(const string& ns, +StatusWith> LiteParsedQuery::makeAsOpQuery(NamespaceString nss, int ntoskip, int ntoreturn, int queryOptions, @@ -362,7 +364,7 @@ StatusWith> LiteParsedQuery::makeAsOpQuery(const str const BSONObj& maxObj, bool snapshot, bool explain) { - unique_ptr pq(new LiteParsedQuery()); + unique_ptr pq(new LiteParsedQuery(std::move(nss))); pq->_sort = sort.getOwned(); pq->_hint = hint.getOwned(); pq->_min = minObj.getOwned(); @@ -370,7 +372,7 @@ StatusWith> LiteParsedQuery::makeAsOpQuery(const str pq->_snapshot = snapshot; pq->_explain = explain; - Status status = pq->init(ns, ntoskip, ntoreturn, queryOptions, query, proj, false); + Status status = pq->init(ntoskip, ntoreturn, queryOptions, query, proj, false); if (!status.isOK()) { return status; } @@ -379,14 +381,13 @@ StatusWith> LiteParsedQuery::makeAsOpQuery(const str } // static -StatusWith> LiteParsedQuery::makeAsFindCmd(const NamespaceString& ns, +StatusWith> LiteParsedQuery::makeAsFindCmd(NamespaceString nss, const BSONObj& query, const BSONObj& sort, boost::optional limit) { - unique_ptr pq(new LiteParsedQuery()); + unique_ptr pq(new LiteParsedQuery(std::move(nss))); pq->_fromCommand = true; - pq->_ns = ns.ns(); pq->_filter = query.getOwned(); pq->_sort = sort.getOwned(); @@ -411,8 +412,7 @@ StatusWith> LiteParsedQuery::makeAsFindCmd(const Nam BSONObj LiteParsedQuery::asFindCommand() const { BSONObjBuilder bob; - const NamespaceString nss(_ns); - bob.append(kCmdName, nss.coll()); + bob.append(kCmdName, _nss.coll()); if (!_filter.isEmpty()) { bob.append(kFilterField, _filter); @@ -696,10 +696,9 @@ bool LiteParsedQuery::isQueryIsolated(const BSONObj& query) { // static StatusWith> LiteParsedQuery::fromLegacyQueryMessage( const QueryMessage& qm) { - unique_ptr pq(new LiteParsedQuery()); + unique_ptr pq(new LiteParsedQuery(NamespaceString(qm.ns))); - Status status = - pq->init(qm.ns, qm.ntoskip, qm.ntoreturn, qm.queryOptions, qm.query, qm.fields, true); + Status status = pq->init(qm.ntoskip, qm.ntoreturn, qm.queryOptions, qm.query, qm.fields, true); if (!status.isOK()) { return status; } @@ -707,14 +706,12 @@ StatusWith> LiteParsedQuery::fromLegacyQueryMessage( return std::move(pq); } -Status LiteParsedQuery::init(const string& ns, - int ntoskip, +Status LiteParsedQuery::init(int ntoskip, int ntoreturn, int queryOptions, const BSONObj& queryObj, const BSONObj& proj, bool fromQueryMessage) { - _ns = ns; _skip = ntoskip; _proj = proj.getOwned(); diff --git a/src/mongo/db/query/lite_parsed_query.h b/src/mongo/db/query/lite_parsed_query.h index 776377ce6b8..92a8c958d67 100644 --- a/src/mongo/db/query/lite_parsed_query.h +++ b/src/mongo/db/query/lite_parsed_query.h @@ -32,10 +32,10 @@ #include #include "mongo/db/jsobj.h" +#include "mongo/db/namespace_string.h" namespace mongo { -class NamespaceString; class QueryMessage; class Status; template @@ -54,13 +54,14 @@ public: * Returns a heap allocated LiteParsedQuery on success or an error if 'cmdObj' is not well * formed. */ - static StatusWith> makeFromFindCommand( - const NamespaceString& nss, const BSONObj& cmdObj, bool isExplain); + static StatusWith> makeFromFindCommand(NamespaceString nss, + const BSONObj& cmdObj, + bool isExplain); /** * Constructs a LiteParseQuery object as though it is from a legacy QueryMessage. */ - static StatusWith> makeAsOpQuery(const std::string& ns, + static StatusWith> makeAsOpQuery(NamespaceString nss, int ntoskip, int ntoreturn, int queryoptions, @@ -77,7 +78,7 @@ public: * Constructs a LiteParseQuery object that can be used to serialize to find command * BSON object. */ - static StatusWith> makeAsFindCmd(const NamespaceString& ns, + static StatusWith> makeAsFindCmd(NamespaceString nss, const BSONObj& query, const BSONObj& sort, boost::optional limit); @@ -138,8 +139,11 @@ public: static const std::string metaRecordId; static const std::string metaIndexKey; + const NamespaceString& nss() const { + return _nss; + } const std::string& ns() const { - return _ns; + return _nss.ns(); } const BSONObj& getFilter() const { @@ -247,7 +251,7 @@ public: const QueryMessage& qm); private: - LiteParsedQuery() = default; + LiteParsedQuery(NamespaceString nss); /** * Parsing code calls this after construction of the LPQ is complete. There are additional @@ -255,8 +259,7 @@ private: */ Status validate() const; - Status init(const std::string& ns, - int ntoskip, + Status init(int ntoskip, int ntoreturn, int queryOptions, const BSONObj& queryObj, @@ -294,7 +297,7 @@ private: */ Status validateFindCmd(); - std::string _ns; + const NamespaceString _nss; BSONObj _filter; BSONObj _proj; diff --git a/src/mongo/db/query/lite_parsed_query_test.cpp b/src/mongo/db/query/lite_parsed_query_test.cpp index 69813e9ed39..a79fb6d593c 100644 --- a/src/mongo/db/query/lite_parsed_query_test.cpp +++ b/src/mongo/db/query/lite_parsed_query_test.cpp @@ -42,8 +42,10 @@ namespace { using std::unique_ptr; using unittest::assertGet; +static const NamespaceString testns("testdb.testcoll"); + TEST(LiteParsedQueryTest, InitSortOrder) { - ASSERT_OK(LiteParsedQuery::makeAsOpQuery("testns", + ASSERT_OK(LiteParsedQuery::makeAsOpQuery(testns, 0, 1, 0, @@ -59,7 +61,7 @@ TEST(LiteParsedQueryTest, InitSortOrder) { } TEST(LiteParsedQueryTest, InitSortOrderString) { - ASSERT_NOT_OK(LiteParsedQuery::makeAsOpQuery("testns", + ASSERT_NOT_OK(LiteParsedQuery::makeAsOpQuery(testns, 0, 1, 0, @@ -75,7 +77,7 @@ TEST(LiteParsedQueryTest, InitSortOrderString) { } TEST(LiteParsedQueryTest, GetFilter) { - unique_ptr lpq(assertGet(LiteParsedQuery::makeAsOpQuery("testns", + unique_ptr lpq(assertGet(LiteParsedQuery::makeAsOpQuery(testns, 5, 6, 9, @@ -92,7 +94,7 @@ TEST(LiteParsedQueryTest, GetFilter) { } TEST(LiteParsedQueryTest, NumToReturn) { - unique_ptr lpq(assertGet(LiteParsedQuery::makeAsOpQuery("testns", + unique_ptr lpq(assertGet(LiteParsedQuery::makeAsOpQuery(testns, 5, 6, 9, @@ -110,7 +112,7 @@ TEST(LiteParsedQueryTest, NumToReturn) { } TEST(LiteParsedQueryTest, NumToReturnNegative) { - unique_ptr lpq(assertGet(LiteParsedQuery::makeAsOpQuery("testns", + unique_ptr lpq(assertGet(LiteParsedQuery::makeAsOpQuery(testns, 5, -6, 9, @@ -128,7 +130,7 @@ TEST(LiteParsedQueryTest, NumToReturnNegative) { } TEST(LiteParsedQueryTest, MinFieldsNotPrefixOfMax) { - ASSERT_NOT_OK(LiteParsedQuery::makeAsOpQuery("testns", + ASSERT_NOT_OK(LiteParsedQuery::makeAsOpQuery(testns, 0, 0, 0, @@ -144,7 +146,7 @@ TEST(LiteParsedQueryTest, MinFieldsNotPrefixOfMax) { } TEST(LiteParsedQueryTest, MinFieldsMoreThanMax) { - ASSERT_NOT_OK(LiteParsedQuery::makeAsOpQuery("testns", + ASSERT_NOT_OK(LiteParsedQuery::makeAsOpQuery(testns, 0, 0, 0, @@ -160,7 +162,7 @@ TEST(LiteParsedQueryTest, MinFieldsMoreThanMax) { } TEST(LiteParsedQueryTest, MinFieldsLessThanMax) { - ASSERT_NOT_OK(LiteParsedQuery::makeAsOpQuery("testns", + ASSERT_NOT_OK(LiteParsedQuery::makeAsOpQuery(testns, 0, 0, 0, @@ -178,7 +180,7 @@ TEST(LiteParsedQueryTest, MinFieldsLessThanMax) { // Helper function which returns the Status of creating a LiteParsedQuery object with the given // parameters. void assertLiteParsedQuerySuccess(const BSONObj& query, const BSONObj& proj, const BSONObj& sort) { - unique_ptr lpq(assertGet(LiteParsedQuery::makeAsOpQuery("testns", + unique_ptr lpq(assertGet(LiteParsedQuery::makeAsOpQuery(testns, 0, 0, 0, @@ -205,7 +207,7 @@ TEST(LiteParsedQueryTest, ValidSortProj) { } TEST(LiteParsedQueryTest, ForbidNonMetaSortOnFieldWithMetaProject) { - ASSERT_NOT_OK(LiteParsedQuery::makeAsOpQuery("testns", + ASSERT_NOT_OK(LiteParsedQuery::makeAsOpQuery(testns, 0, 0, 0, @@ -224,7 +226,7 @@ TEST(LiteParsedQueryTest, ForbidNonMetaSortOnFieldWithMetaProject) { } TEST(LiteParsedQueryTest, ForbidMetaSortOnFieldWithoutMetaProject) { - ASSERT_NOT_OK(LiteParsedQuery::makeAsOpQuery("testns", + ASSERT_NOT_OK(LiteParsedQuery::makeAsOpQuery(testns, 0, 0, 0, @@ -238,7 +240,7 @@ TEST(LiteParsedQueryTest, ForbidMetaSortOnFieldWithoutMetaProject) { false) // explain .getStatus()); - ASSERT_NOT_OK(LiteParsedQuery::makeAsOpQuery("testns", + ASSERT_NOT_OK(LiteParsedQuery::makeAsOpQuery(testns, 0, 0, 0, @@ -938,7 +940,7 @@ TEST(LiteParsedQueryTest, ParseCommandIsFromFindCommand) { TEST(LiteParsedQueryTest, ParseCommandNotFromFindCommand) { std::unique_ptr lpq( - assertGet(LiteParsedQuery::makeAsOpQuery("testns", + assertGet(LiteParsedQuery::makeAsOpQuery(testns, 5, 6, 9, -- cgit v1.2.1