summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--jstests/core/profile_agg.js6
-rw-r--r--jstests/core/views/views_find.js4
-rw-r--r--jstests/noPassthrough/currentop_query.js8
-rw-r--r--jstests/sharding/views.js18
-rw-r--r--src/mongo/db/pipeline/aggregation_request.cpp12
-rw-r--r--src/mongo/db/pipeline/aggregation_request.h12
-rw-r--r--src/mongo/db/pipeline/aggregation_request_test.cpp17
-rw-r--r--src/mongo/db/query/query_request.cpp7
-rw-r--r--src/mongo/db/query/query_request_test.cpp9
-rw-r--r--src/mongo/db/views/resolved_view.cpp1
-rw-r--r--src/mongo/db/views/resolved_view_test.cpp9
11 files changed, 91 insertions, 12 deletions
diff --git a/jstests/core/profile_agg.js b/jstests/core/profile_agg.js
index 39f4fd788ed..262fa29495b 100644
--- a/jstests/core/profile_agg.js
+++ b/jstests/core/profile_agg.js
@@ -21,7 +21,10 @@
}
assert.commandWorked(coll.createIndex({a: 1}));
- assert.eq(8, coll.aggregate([{$match: {a: {$gte: 2}}}], {collation: {locale: "fr"}}).itcount());
+ assert.eq(8,
+ coll.aggregate([{$match: {a: {$gte: 2}}}],
+ {collation: {locale: "fr"}, comment: "agg_comment"})
+ .itcount());
var profileObj = getLatestProfilerEntry(testDB);
assert.eq(profileObj.ns, coll.getFullName(), tojson(profileObj));
@@ -35,6 +38,7 @@
tojson(profileObj));
assert.eq(profileObj.command.aggregate, coll.getName(), tojson(profileObj));
assert.eq(profileObj.command.collation, {locale: "fr"}, tojson(profileObj));
+ assert.eq(profileObj.command.comment, "agg_comment", tojson(profileObj));
assert(profileObj.hasOwnProperty("responseLength"), tojson(profileObj));
assert(profileObj.hasOwnProperty("millis"), tojson(profileObj));
assert(profileObj.hasOwnProperty("numYield"), tojson(profileObj));
diff --git a/jstests/core/views/views_find.js b/jstests/core/views/views_find.js
index da555e454b7..428457b7822 100644
--- a/jstests/core/views/views_find.js
+++ b/jstests/core/views/views_find.js
@@ -69,6 +69,10 @@
assert.commandFailed(viewsDB.runCommand({find: "identityView", batchSize: -1}));
assert.commandFailed(viewsDB.runCommand({find: "identityView", limit: -1}));
+ // Comment should succeed.
+ assert.commandWorked(
+ viewsDB.runCommand({find: "identityView", filter: {}, comment: "views_find"}));
+
// Views support find with explain.
assert.commandWorked(viewsDB.identityView.find().explain());
diff --git a/jstests/noPassthrough/currentop_query.js b/jstests/noPassthrough/currentop_query.js
index dd5c0992dbb..7137a9bdd18 100644
--- a/jstests/noPassthrough/currentop_query.js
+++ b/jstests/noPassthrough/currentop_query.js
@@ -99,8 +99,11 @@
{
test: function() {
assert.eq(db.currentop_query
- .aggregate([{$match: {a: 1, $comment: "currentop_query"}}],
- {collation: {locale: "fr"}, hint: {_id: 1}})
+ .aggregate([{$match: {a: 1, $comment: "currentop_query"}}], {
+ collation: {locale: "fr"},
+ hint: {_id: 1},
+ comment: "currentop_query_2"
+ })
.itcount(),
1);
},
@@ -108,6 +111,7 @@
planSummary: "IXSCAN { _id: 1 }",
currentOpFilter: {
"query.pipeline.0.$match.$comment": "currentop_query",
+ "query.comment": "currentop_query_2",
"query.collation": {locale: "fr"},
"query.hint": {_id: 1}
}
diff --git a/jstests/sharding/views.js b/jstests/sharding/views.js
index a120eb32157..09196702ed0 100644
--- a/jstests/sharding/views.js
+++ b/jstests/sharding/views.js
@@ -5,6 +5,9 @@
(function() {
"use strict";
+ // For profilerHasSingleMatchingEntryOrThrow.
+ load("jstests/libs/profiler.js");
+
// Given sharded explain output in 'shardedExplain', verifies that the explain mode 'verbosity'
// affected the output verbosity appropriately, and that the response has the expected format.
function verifyExplainResult(shardedExplain, verbosity) {
@@ -133,5 +136,20 @@
assert.commandFailedWithCode(db.adminCommand({getShardVersion: view.getFullName()}),
ErrorCodes.NamespaceNotSharded);
+ //
+ // Confirm that the comment parameter on a find command is retained when rewritten as an
+ // expanded aggregation on the view.
+ //
+ let sdb = st.shard0.getDB(jsTestName());
+ assert.commandWorked(sdb.setProfilingLevel(2));
+
+ assert.eq(5, view.find({a: {$lte: 8}}).comment("agg_comment").itcount());
+
+ profilerHasSingleMatchingEntryOrThrow(sdb, {
+ "command.aggregate": coll.getName(),
+ "command.fromRouter": true,
+ "command.comment": "agg_comment"
+ });
+
st.stop();
})();
diff --git a/src/mongo/db/pipeline/aggregation_request.cpp b/src/mongo/db/pipeline/aggregation_request.cpp
index 920073c6558..214c134837f 100644
--- a/src/mongo/db/pipeline/aggregation_request.cpp
+++ b/src/mongo/db/pipeline/aggregation_request.cpp
@@ -56,6 +56,7 @@ const StringData AggregationRequest::kCollationName = "collation"_sd;
const StringData AggregationRequest::kExplainName = "explain"_sd;
const StringData AggregationRequest::kAllowDiskUseName = "allowDiskUse"_sd;
const StringData AggregationRequest::kHintName = "hint"_sd;
+const StringData AggregationRequest::kCommentName = "comment"_sd;
const long long AggregationRequest::kDefaultBatchSize = 101;
@@ -136,6 +137,13 @@ StatusWith<AggregationRequest> AggregationRequest::parseFromBSON(
<< " must be specified as a string representing an index"
<< " name, or an object representing an index's key pattern");
}
+ } else if (kCommentName == fieldName) {
+ if (elem.type() != BSONType::String) {
+ return {ErrorCodes::TypeMismatch,
+ str::stream() << kCommentName << " must be a string, not a "
+ << typeName(elem.type())};
+ }
+ request.setComment(elem.str());
} else if (kExplainName == fieldName) {
if (elem.type() != BSONType::Bool) {
return {ErrorCodes::TypeMismatch,
@@ -222,7 +230,9 @@ Document AggregationRequest::serializeToCommandObj() const {
// Only serialize batchSize when explain is false.
{kCursorName, _explainMode ? Value() : Value(Document{{kBatchSizeName, _batchSize}})},
// Only serialize a hint if one was specified.
- {kHintName, _hint.isEmpty() ? Value() : Value(_hint)}};
+ {kHintName, _hint.isEmpty() ? Value() : Value(_hint)},
+ // Only serialize a comment if one was specified.
+ {kCommentName, _comment.empty() ? Value() : Value(_comment)}};
}
} // namespace mongo
diff --git a/src/mongo/db/pipeline/aggregation_request.h b/src/mongo/db/pipeline/aggregation_request.h
index 156d397cbb0..935b98fed97 100644
--- a/src/mongo/db/pipeline/aggregation_request.h
+++ b/src/mongo/db/pipeline/aggregation_request.h
@@ -56,6 +56,7 @@ public:
static const StringData kExplainName;
static const StringData kAllowDiskUseName;
static const StringData kHintName;
+ static const StringData kCommentName;
static const long long kDefaultBatchSize;
@@ -131,6 +132,10 @@ public:
return _hint;
}
+ const std::string& getComment() const {
+ return _comment;
+ }
+
boost::optional<ExplainOptions::Verbosity> getExplain() const {
return _explainMode;
}
@@ -155,6 +160,10 @@ public:
_hint = hint.getOwned();
}
+ void setComment(const std::string& comment) {
+ _comment = comment;
+ }
+
void setExplain(boost::optional<ExplainOptions::Verbosity> verbosity) {
_explainMode = verbosity;
}
@@ -191,6 +200,9 @@ private:
// {$hint: <String>}, where <String> is the index name hinted.
BSONObj _hint;
+ // The comment parameter attached to this aggregation.
+ std::string _comment;
+
// The explain mode to use, or boost::none if this is not a request for an aggregation explain.
boost::optional<ExplainOptions::Verbosity> _explainMode;
diff --git a/src/mongo/db/pipeline/aggregation_request_test.cpp b/src/mongo/db/pipeline/aggregation_request_test.cpp
index a40bca8d8d1..e148cef9b25 100644
--- a/src/mongo/db/pipeline/aggregation_request_test.cpp
+++ b/src/mongo/db/pipeline/aggregation_request_test.cpp
@@ -56,7 +56,7 @@ TEST(AggregationRequestTest, ShouldParseAllKnownOptions) {
const BSONObj inputBson = fromjson(
"{pipeline: [{$match: {a: 'abc'}}], explain: true, allowDiskUse: true, fromRouter: true, "
"bypassDocumentValidation: true, collation: {locale: 'en_US'}, cursor: {batchSize: 10}, "
- "hint: {a: 1}}");
+ "hint: {a: 1}, comment: 'agg_comment'}");
auto request = unittest::assertGet(AggregationRequest::parseFromBSON(nss, inputBson));
ASSERT_TRUE(request.getExplain());
ASSERT(*request.getExplain() == ExplainOptions::Verbosity::kQueryPlanner);
@@ -65,6 +65,7 @@ TEST(AggregationRequestTest, ShouldParseAllKnownOptions) {
ASSERT_TRUE(request.shouldBypassDocumentValidation());
ASSERT_EQ(request.getBatchSize(), 10);
ASSERT_BSONOBJ_EQ(request.getHint(), BSON("a" << 1));
+ ASSERT_EQ(request.getComment(), "agg_comment");
ASSERT_BSONOBJ_EQ(request.getCollation(),
BSON("locale"
<< "en_US"));
@@ -121,6 +122,7 @@ TEST(AggregationRequestTest, ShouldNotSerializeOptionalValuesIfEquivalentToDefau
request.setBypassDocumentValidation(false);
request.setCollation(BSONObj());
request.setHint(BSONObj());
+ request.setComment("");
auto expectedSerialization =
Document{{AggregationRequest::kCommandName, nss.coll()},
@@ -138,6 +140,8 @@ TEST(AggregationRequestTest, ShouldSerializeOptionalValuesIfSet) {
request.setBatchSize(10);
const auto hintObj = BSON("a" << 1);
request.setHint(hintObj);
+ const auto comment = std::string("agg_comment");
+ request.setComment(comment);
const auto collationObj = BSON("locale"
<< "en_US");
request.setCollation(collationObj);
@@ -151,7 +155,8 @@ TEST(AggregationRequestTest, ShouldSerializeOptionalValuesIfSet) {
{AggregationRequest::kCollationName, collationObj},
{AggregationRequest::kCursorName,
Value(Document({{AggregationRequest::kBatchSizeName, 10}}))},
- {AggregationRequest::kHintName, hintObj}};
+ {AggregationRequest::kHintName, hintObj},
+ {AggregationRequest::kCommentName, comment}};
ASSERT_DOCUMENT_EQ(request.serializeToCommandObj(), expectedSerialization);
}
@@ -241,6 +246,14 @@ TEST(AggregationRequestTest, ShouldRejectHintAsArray) {
AggregationRequest::parseFromBSON(NamespaceString("a.collection"), inputBson).getStatus());
}
+TEST(AggregationRequestTest, ShouldRejectNonStringComment) {
+ NamespaceString nss("a.collection");
+ const BSONObj inputBson =
+ fromjson("{pipeline: [{$match: {a: 'abc'}}], cursor: {}, comment: 1}");
+ ASSERT_NOT_OK(
+ AggregationRequest::parseFromBSON(NamespaceString("a.collection"), inputBson).getStatus());
+}
+
TEST(AggregationRequestTest, ShouldRejectExplainIfNumber) {
NamespaceString nss("a.collection");
const BSONObj inputBson =
diff --git a/src/mongo/db/query/query_request.cpp b/src/mongo/db/query/query_request.cpp
index dc8188ab96a..04c4154f741 100644
--- a/src/mongo/db/query/query_request.cpp
+++ b/src/mongo/db/query/query_request.cpp
@@ -964,10 +964,6 @@ StatusWith<BSONObj> QueryRequest::asAggregationCommand() const {
return {ErrorCodes::InvalidPipelineOperator,
str::stream() << "Option " << kReturnKeyField << " not supported in aggregation."};
}
- if (!_comment.empty()) {
- return {ErrorCodes::InvalidPipelineOperator,
- str::stream() << "Option " << kCommentField << " not supported in aggregation."};
- }
if (_showRecordId) {
return {ErrorCodes::InvalidPipelineOperator,
str::stream() << "Option " << kShowRecordIdField
@@ -1064,6 +1060,9 @@ StatusWith<BSONObj> QueryRequest::asAggregationCommand() const {
if (!_hint.isEmpty()) {
aggregationBuilder.append("hint", _hint);
}
+ if (!_comment.empty()) {
+ aggregationBuilder.append("comment", _comment);
+ }
return StatusWith<BSONObj>(aggregationBuilder.obj());
}
} // namespace mongo
diff --git a/src/mongo/db/query/query_request_test.cpp b/src/mongo/db/query/query_request_test.cpp
index 8906d5c2a7d..0834cd6e4c8 100644
--- a/src/mongo/db/query/query_request_test.cpp
+++ b/src/mongo/db/query/query_request_test.cpp
@@ -1178,10 +1178,15 @@ TEST(QueryRequestTest, ConvertToAggregationWithReturnKeyFails) {
ASSERT_NOT_OK(qr.asAggregationCommand());
}
-TEST(QueryRequestTest, ConvertToAggregationWithCommentFails) {
+TEST(QueryRequestTest, ConvertToAggregationWithCommentSucceeds) {
QueryRequest qr(testns);
qr.setComment("test");
- ASSERT_NOT_OK(qr.asAggregationCommand());
+ const auto aggCmd = qr.asAggregationCommand();
+ ASSERT_OK(aggCmd);
+
+ auto ar = AggregationRequest::parseFromBSON(testns, aggCmd.getValue());
+ ASSERT_OK(ar.getStatus());
+ ASSERT_EQ(qr.getComment(), ar.getValue().getComment());
}
TEST(QueryRequestTest, ConvertToAggregationWithShowRecordIdFails) {
diff --git a/src/mongo/db/views/resolved_view.cpp b/src/mongo/db/views/resolved_view.cpp
index 9012e5489e4..3376c05570d 100644
--- a/src/mongo/db/views/resolved_view.cpp
+++ b/src/mongo/db/views/resolved_view.cpp
@@ -84,6 +84,7 @@ AggregationRequest ResolvedView::asExpandedViewAggregation(
}
expandedRequest.setHint(request.getHint());
+ expandedRequest.setComment(request.getComment());
expandedRequest.setBypassDocumentValidation(request.shouldBypassDocumentValidation());
expandedRequest.setAllowDiskUse(request.shouldAllowDiskUse());
expandedRequest.setCollation(request.getCollation());
diff --git a/src/mongo/db/views/resolved_view_test.cpp b/src/mongo/db/views/resolved_view_test.cpp
index e25c5b263b7..cd34a8e6cf4 100644
--- a/src/mongo/db/views/resolved_view_test.cpp
+++ b/src/mongo/db/views/resolved_view_test.cpp
@@ -125,6 +125,15 @@ TEST(ResolvedViewTest, ExpandingAggRequestPreservesCollation) {
<< "fr_CA"));
}
+TEST(ResolvedViewTest, ExpandingAggRequestPreservesComment) {
+ const ResolvedView resolvedView{backingNss, emptyPipeline};
+ AggregationRequest aggRequest(viewNss, {});
+ aggRequest.setComment("agg_comment");
+
+ auto result = resolvedView.asExpandedViewAggregation(aggRequest);
+ ASSERT_EQ(result.getComment(), "agg_comment");
+}
+
TEST(ResolvedViewTest, FromBSONFailsIfMissingResolvedView) {
BSONObj badCmdResponse = BSON("x" << 1);
ASSERT_THROWS_CODE(ResolvedView::fromBSON(badCmdResponse), UserException, 40248);
View Lorry Controller Status