summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorArun Banala <arun.banala@mongodb.com>2021-01-29 19:10:10 +0000
committerEvergreen Agent <no-reply@evergreen.mongodb.com>2021-02-19 22:48:35 +0000
commit1544f419f57da5fe5aba2094afff6e8f663feb18 (patch)
tree9145e30d6a5e76831734c7debe7d90ba724ee415
parent0a3dc2dbe61349734ca98c5caf19fd8d28fd02eb (diff)
downloadmongo-1544f419f57da5fe5aba2094afff6e8f663feb18.tar.gz
SERVER-51622 Convert find and aggregate commands' output to IDL
-rw-r--r--jstests/core/api_version_unstable_fields.js4
-rw-r--r--src/mongo/db/commands/find_cmd.cpp1
-rw-r--r--src/mongo/db/commands/pipeline_command.cpp6
-rw-r--r--src/mongo/db/commands/run_aggregate.cpp1
-rw-r--r--src/mongo/db/pipeline/aggregate_command.idl11
-rw-r--r--src/mongo/db/query/SConscript14
-rw-r--r--src/mongo/db/query/cursor_response.cpp22
-rw-r--r--src/mongo/db/query/cursor_response.h11
-rw-r--r--src/mongo/db/query/cursor_response.idl71
-rw-r--r--src/mongo/db/query/cursor_response_test.cpp27
-rw-r--r--src/mongo/db/query/find_command.idl17
-rw-r--r--src/mongo/db/query/query_request_helper.cpp7
-rw-r--r--src/mongo/db/query/query_request_helper.h5
-rw-r--r--src/mongo/db/query/query_request_test.cpp14
14 files changed, 145 insertions, 66 deletions
diff --git a/jstests/core/api_version_unstable_fields.js b/jstests/core/api_version_unstable_fields.js
index f2e54bebf83..ee3c6bf8d34 100644
--- a/jstests/core/api_version_unstable_fields.js
+++ b/jstests/core/api_version_unstable_fields.js
@@ -4,6 +4,8 @@
* @tags: [
* requires_fcv_47,
* uses_api_parameters,
+ * # 'explain' does not support stepdowns.
+ * does_not_support_stepdowns,
* ]
*/
@@ -16,7 +18,7 @@ assert.commandWorked(db[collName].insert({a: 1}));
const unstableFieldsForAggregate = {
isMapReduceCommand: false,
$_requestReshardingResumeToken: false,
- exchange: {policy: "roundrobin", consumers: NumberInt(10), bufferSize: NumberInt(1024)},
+ explain: true,
runtimeConstants: {a: 1},
collectionUUID: UUID(),
use44SortKeys: false,
diff --git a/src/mongo/db/commands/find_cmd.cpp b/src/mongo/db/commands/find_cmd.cpp
index e315f1c314f..f9b858d0b49 100644
--- a/src/mongo/db/commands/find_cmd.cpp
+++ b/src/mongo/db/commands/find_cmd.cpp
@@ -570,6 +570,7 @@ public:
// documents.
auto& metricsCollector = ResourceConsumption::MetricsCollector::get(opCtx);
metricsCollector.incrementDocUnitsReturned(docUnitsReturned);
+ query_request_helper::validateCursorResponse(result->getBodyBuilder().asTempObj());
}
void appendMirrorableRequest(BSONObjBuilder* bob) const override {
diff --git a/src/mongo/db/commands/pipeline_command.cpp b/src/mongo/db/commands/pipeline_command.cpp
index 5cd32640290..be5927ef955 100644
--- a/src/mongo/db/commands/pipeline_command.cpp
+++ b/src/mongo/db/commands/pipeline_command.cpp
@@ -146,6 +146,12 @@ public:
_request.body,
_privileges,
reply));
+
+ // The aggregate command's response is unstable when 'explain' or 'exchange' fields are
+ // set.
+ if (!_aggregationRequest.getExplain() && !_aggregationRequest.getExchange()) {
+ query_request_helper::validateCursorResponse(reply->getBodyBuilder().asTempObj());
+ }
}
NamespaceString ns() const override {
diff --git a/src/mongo/db/commands/run_aggregate.cpp b/src/mongo/db/commands/run_aggregate.cpp
index 5632d906234..344e3462d75 100644
--- a/src/mongo/db/commands/run_aggregate.cpp
+++ b/src/mongo/db/commands/run_aggregate.cpp
@@ -881,7 +881,6 @@ Status runAggregate(OperationContext* opCtx,
curOp->setNS_inlock(origNss.ns());
}
- // Any code that needs the cursor pinned must be inside the try block, above.
return Status::OK();
}
diff --git a/src/mongo/db/pipeline/aggregate_command.idl b/src/mongo/db/pipeline/aggregate_command.idl
index e904d2c6b31..7d1f8f6b355 100644
--- a/src/mongo/db/pipeline/aggregate_command.idl
+++ b/src/mongo/db/pipeline/aggregate_command.idl
@@ -38,6 +38,7 @@ imports:
- "mongo/db/pipeline/exchange_spec.idl"
- "mongo/db/pipeline/legacy_runtime_constants.idl"
- "mongo/db/query/hint.idl"
+ - "mongo/db/query/cursor_response.idl"
- "mongo/db/write_concern_options.idl"
types:
@@ -68,6 +69,11 @@ commands:
namespace: concatenate_with_db
allow_global_collection_name: true
api_version: "1"
+ # Note that the 'CursorInitialReply' is not the only response that an aggregate command
+ # could return. With 'explain' or 'exchange', the response would not include the fields in
+ # 'CursorInitialReply'. But using 'explain' or 'exchange' is unstable, but otherwise the
+ # aggregate response is guaranteed to be stable.
+ reply_type: CursorInitialReply
fields:
pipeline:
description: "An unparsed version of the pipeline."
@@ -76,6 +82,7 @@ commands:
description: "Specifies to return the information on the processing of the pipeline."
type: explainVerbosity
optional: true
+ unstable: true
allowDiskUse:
description: "Enables writing to temporary files."
type: optionalBool
@@ -118,7 +125,6 @@ commands:
fromMongos:
description: "True if this request originated from a mongoS."
type: optionalBool
-
$queryOptions:
description: "The unwrapped readPreference object, if one was given to us by the mongos command processor. This object will be empty when no readPreference is specified or if the request does not originate from mongos."
cpp_name: unwrappedReadPref
@@ -167,6 +173,3 @@ commands:
type: bool
ignore: true
unstable: true
- # TODO SERVER-51622: This OkReply is just used as default to be able to mark "unstable"
- # fields, and should be replaced when converting the output of aggregate command to IDL.
- reply_type: OkReply
diff --git a/src/mongo/db/query/SConscript b/src/mongo/db/query/SConscript
index 6672d91e65d..c12394340b0 100644
--- a/src/mongo/db/query/SConscript
+++ b/src/mongo/db/query/SConscript
@@ -146,6 +146,18 @@ env.Library(
)
env.Library(
+ target='cursor_response_idl',
+ source=[
+ 'cursor_response.idl',
+ ],
+ LIBDEPS=[
+ '$BUILD_DIR/mongo/base',
+ '$BUILD_DIR/mongo/idl/basic_types',
+ '$BUILD_DIR/mongo/idl/idl_parser',
+ ],
+)
+
+env.Library(
target='command_request_response',
source=[
"count_command_as_aggregation_command.cpp",
@@ -168,6 +180,7 @@ env.Library(
'$BUILD_DIR/mongo/idl/idl_parser',
'$BUILD_DIR/mongo/rpc/command_status',
'$BUILD_DIR/mongo/rpc/rpc',
+ 'cursor_response_idl',
'query_request',
]
)
@@ -191,6 +204,7 @@ env.Library(
"$BUILD_DIR/mongo/db/commands/test_commands_enabled",
"$BUILD_DIR/mongo/db/pipeline/runtime_constants_idl",
"$BUILD_DIR/mongo/db/repl/read_concern_args",
+ "cursor_response_idl",
"hint_parser",
],
)
diff --git a/src/mongo/db/query/cursor_response.cpp b/src/mongo/db/query/cursor_response.cpp
index 4b3eea43ee8..fe5b3bc0175 100644
--- a/src/mongo/db/query/cursor_response.cpp
+++ b/src/mongo/db/query/cursor_response.cpp
@@ -57,26 +57,16 @@ const char kPartialResultsReturnedField[] = "partialResultsReturned";
CursorResponseBuilder::CursorResponseBuilder(rpc::ReplyBuilderInterface* replyBuilder,
Options options = Options())
: _options(options), _replyBuilder(replyBuilder) {
- if (_options.useDocumentSequences) {
- _docSeqBuilder.emplace(_replyBuilder->getDocSequenceBuilder(
- _options.isInitialResponse ? kBatchDocSequenceFieldInitial : kBatchDocSequenceField));
- } else {
- _bodyBuilder.emplace(_replyBuilder->getBodyBuilder());
- _cursorObject.emplace(_bodyBuilder->subobjStart(kCursorField));
- _batch.emplace(_cursorObject->subarrayStart(_options.isInitialResponse ? kBatchFieldInitial
- : kBatchField));
- }
+ _bodyBuilder.emplace(_replyBuilder->getBodyBuilder());
+ _cursorObject.emplace(_bodyBuilder->subobjStart(kCursorField));
+ _batch.emplace(_cursorObject->subarrayStart(_options.isInitialResponse ? kBatchFieldInitial
+ : kBatchField));
}
void CursorResponseBuilder::done(CursorId cursorId, StringData cursorNamespace) {
invariant(_active);
- if (_options.useDocumentSequences) {
- _docSeqBuilder.reset();
- _bodyBuilder.emplace(_replyBuilder->getBodyBuilder());
- _cursorObject.emplace(_bodyBuilder->subobjStart(kCursorField));
- } else {
- _batch.reset();
- }
+
+ _batch.reset();
if (!_postBatchResumeToken.isEmpty()) {
_cursorObject->append(kPostBatchResumeTokenField, _postBatchResumeToken);
}
diff --git a/src/mongo/db/query/cursor_response.h b/src/mongo/db/query/cursor_response.h
index 7e7633b14c1..6c7081fa1ec 100644
--- a/src/mongo/db/query/cursor_response.h
+++ b/src/mongo/db/query/cursor_response.h
@@ -56,7 +56,6 @@ public:
*/
struct Options {
bool isInitialResponse = false;
- bool useDocumentSequences = false;
boost::optional<LogicalTime> atClusterTime = boost::none;
};
@@ -78,16 +77,13 @@ public:
size_t bytesUsed() const {
invariant(_active);
- return _options.useDocumentSequences ? _docSeqBuilder->len() : _batch->len();
+ return _batch->len();
}
void append(const BSONObj& obj) {
invariant(_active);
- if (_options.useDocumentSequences) {
- _docSeqBuilder->append(obj);
- } else {
- _batch->append(obj);
- }
+
+ _batch->append(obj);
_numDocs++;
}
@@ -123,7 +119,6 @@ private:
boost::optional<BSONObjBuilder> _bodyBuilder;
boost::optional<BSONObjBuilder> _cursorObject;
boost::optional<BSONArrayBuilder> _batch;
- boost::optional<OpMsgBuilder::DocSequenceBuilder> _docSeqBuilder;
bool _active = true;
long long _numDocs = 0;
diff --git a/src/mongo/db/query/cursor_response.idl b/src/mongo/db/query/cursor_response.idl
new file mode 100644
index 00000000000..6521d4a0e60
--- /dev/null
+++ b/src/mongo/db/query/cursor_response.idl
@@ -0,0 +1,71 @@
+# Copyright(C) 2021 - present MongoDB, Inc.
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the Server Side Public License, version 1,
+# as published by MongoDB, Inc.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# Server Side Public License for more details.
+#
+# You should have received a copy of the Server Side Public License
+# along with this program. If not, see
+# <http://www.mongodb.com/licensing/server-side-public-license>.
+#
+# As a special exception, the copyright holders give permission to link the
+# code of portions of this program with the OpenSSL library under certain
+# conditions as described in each individual source file and distribute
+# linked combinations including the program with the OpenSSL library. You
+# must comply with the Server Side Public License in all respects for
+# all of the code used other than as permitted herein. If you modify file(s)
+# with this exception, you may extend this exception to your version of the
+# file(s), but you are not obligated to do so. If you do not wish to do so,
+# delete this exception statement from your version. If you delete this
+# exception statement from all source files in the program, then also delete
+# it in the license file.
+#
+
+# This IDL file describes the BSON format for cursor response object.
+
+global:
+ cpp_namespace: "mongo"
+ cpp_includes:
+ - "mongo/db/namespace_string.h"
+ - "mongo/idl/basic_types.h"
+
+imports:
+ - "mongo/idl/basic_types.idl"
+
+structs:
+ InitialResponseCursor:
+ description: "A struct representing an initial response cursor."
+ fields:
+ firstBatch:
+ description: "The first batch of the cursor."
+ type: array<object>
+ id:
+ cpp_name: "cursorId"
+ description: "The cursor id of the cursor."
+ type: long
+ ns:
+ description: "The namespace of the cursor."
+ type: namespacestring
+ postBatchResumeToken:
+ description: "An optional resume token object."
+ type: object
+ optional: true
+ atClusterTime:
+ description: "The time stamp at which the response is being returned."
+ type: timestamp
+ optional: true
+ partialResultsReturned:
+ description: "Boolean represents whether partial results are being returned."
+ type: optionalBool
+
+ CursorInitialReply:
+ description: "A struct representing a initial cursor reply."
+ fields:
+ cursor:
+ description: "A response cursor object."
+ type: InitialResponseCursor
diff --git a/src/mongo/db/query/cursor_response_test.cpp b/src/mongo/db/query/cursor_response_test.cpp
index a3bc5449ac7..2a3424f2029 100644
--- a/src/mongo/db/query/cursor_response_test.cpp
+++ b/src/mongo/db/query/cursor_response_test.cpp
@@ -371,32 +371,5 @@ TEST(CursorResponseTest, serializePostBatchResumeToken) {
ASSERT_BSONOBJ_EQ(*reparsedResponse.getPostBatchResumeToken(), postBatchResumeToken);
}
-TEST(CursorResponseTest, cursorReturnDocumentSequences) {
- CursorResponseBuilder::Options options;
- options.isInitialResponse = true;
- options.useDocumentSequences = true;
- rpc::OpMsgReplyBuilder builder;
- BSONObj expectedDoc = BSON("_id" << 1 << "test"
- << "123");
- BSONObj expectedBody = BSON("cursor" << BSON("id" << CursorId(123) << "ns"
- << "db.coll"));
-
- CursorResponseBuilder crb(&builder, options);
- crb.append(expectedDoc);
- ASSERT_EQ(crb.numDocs(), 1U);
- crb.done(CursorId(123), "db.coll");
-
- auto msg = builder.done();
- auto opMsg = OpMsg::parse(msg);
- const auto& docSeqs = opMsg.sequences;
- ASSERT_EQ(docSeqs.size(), 1U);
- const auto& documentSequence = docSeqs[0];
- ASSERT_EQ(documentSequence.name, "cursor.firstBatch");
- ASSERT_EQ(documentSequence.objs.size(), 1U);
- ASSERT_BSONOBJ_EQ(documentSequence.objs[0], expectedDoc);
- ASSERT_BSONOBJ_EQ(opMsg.body, expectedBody);
-}
-
} // namespace
-
} // namespace mongo
diff --git a/src/mongo/db/query/find_command.idl b/src/mongo/db/query/find_command.idl
index fe2cc20be23..6bacd5988cb 100644
--- a/src/mongo/db/query/find_command.idl
+++ b/src/mongo/db/query/find_command.idl
@@ -39,18 +39,19 @@ imports:
- "mongo/db/logical_session_id.idl"
- "mongo/db/pipeline/legacy_runtime_constants.idl"
- "mongo/idl/basic_types.idl"
+ - "mongo/db/query/cursor_response.idl"
- "mongo/db/query/hint.idl"
types:
boolNoOpSerializer:
- bson_serialization_type: any
- description: "Bool data type field which doesn't produce any data when serialized."
- cpp_type: "bool"
- deserializer: "::mongo::parseBoolean"
- serializer: "::mongo::noOpSerializer"
+ bson_serialization_type: any
+ description: "Bool data type field which doesn't produce any data when serialized."
+ cpp_type: "bool"
+ deserializer: "::mongo::parseBoolean"
+ serializer: "::mongo::noOpSerializer"
object_owned_nonempty_serialize:
bson_serialization_type: any
- description: "An owned BSONObj, which gets serialized only when the BSON is not empty.
+ description: "An owned BSONObj, which gets serialized only when the BSON is not empty.
The object is ignored if empty, null or missing."
cpp_type: "mongo::BSONObj"
serializer: "::mongo::serializeBSONWhenNotEmpty"
@@ -71,6 +72,7 @@ commands:
namespace: concatenate_with_db_or_uuid
non_const_getter: true
api_version: "1"
+ reply_type: CursorInitialReply
fields:
filter:
description: "The query predicate. If unspecified, then all documents in the collection
@@ -214,6 +216,3 @@ commands:
cpp_name: legacyRuntimeConstants
type: LegacyRuntimeConstants
optional: true
- # TODO SERVER-51622: This OkReply is just used as default to be able to mark "unstable" fields,
- # and should be replaced when converting the output of find command to IDL.
- reply_type: OkReply
diff --git a/src/mongo/db/query/query_request_helper.cpp b/src/mongo/db/query/query_request_helper.cpp
index ec12a47c821..6f7147d6b2b 100644
--- a/src/mongo/db/query/query_request_helper.cpp
+++ b/src/mongo/db/query/query_request_helper.cpp
@@ -36,6 +36,7 @@
#include "mongo/base/status.h"
#include "mongo/base/status_with.h"
#include "mongo/bson/simple_bsonobj_comparator.h"
+#include "mongo/db/commands/test_commands_enabled.h"
#include "mongo/db/dbmessage.h"
namespace mongo {
@@ -425,6 +426,12 @@ TailableModeEnum getTailableMode(const FindCommand& findCommand) {
tailableModeFromBools(findCommand.getTailable(), findCommand.getAwaitData()));
}
+void validateCursorResponse(const BSONObj& outputAsBson) {
+ if (getTestCommandsEnabled()) {
+ CursorInitialReply::parse(IDLParserErrorContext("CursorInitialReply"), outputAsBson);
+ }
+}
+
//
// Old QueryRequest parsing code: SOON TO BE DEPRECATED.
//
diff --git a/src/mongo/db/query/query_request_helper.h b/src/mongo/db/query/query_request_helper.h
index 00497369379..c4c3b4245fd 100644
--- a/src/mongo/db/query/query_request_helper.h
+++ b/src/mongo/db/query/query_request_helper.h
@@ -133,6 +133,11 @@ void setTailableMode(TailableModeEnum tailableMode, FindCommand* findCommand);
TailableModeEnum getTailableMode(const FindCommand& findCommand);
+/**
+ * Asserts whether the cursor response adhere to the format defined in IDL.
+ */
+void validateCursorResponse(const BSONObj& outputAsBson);
+
//
// Old parsing code: SOON TO BE DEPRECATED.
//
diff --git a/src/mongo/db/query/query_request_test.cpp b/src/mongo/db/query/query_request_test.cpp
index adb40fa212f..c8aae9f0a6b 100644
--- a/src/mongo/db/query/query_request_test.cpp
+++ b/src/mongo/db/query/query_request_test.cpp
@@ -1576,6 +1576,20 @@ TEST(QueryRequestTest, ParseFromLegacyQueryUnwrapped) {
ASSERT_BSONOBJ_EQ(findCommand->getFilter(), fromjson("{foo: 1}"));
}
+TEST(QueryRequestHelperTest, ValidateResponseMissingFields) {
+ BSONObjBuilder builder;
+ ASSERT_THROWS_CODE(
+ query_request_helper::validateCursorResponse(builder.asTempObj()), DBException, 40414);
+}
+
+TEST(QueryRequestHelperTest, ValidateResponseWrongDataType) {
+ BSONObjBuilder builder;
+ builder.append("cursor", 1);
+ ASSERT_THROWS_CODE(query_request_helper::validateCursorResponse(builder.asTempObj()),
+ DBException,
+ ErrorCodes::TypeMismatch);
+}
+
TEST(QueryRequestTest, ParseFromLegacyQueryTooNegativeNToReturn) {
BSONObj queryObj = fromjson(R"({
foo: 1