SERVER-28128 Add support for a "comment" parameter to the aggregate...

... command

7 files changed, 58 insertions, 9 deletions

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);