SERVER-46716: Add tests for let parameters for sharded update

Bring back the earlier commit.

12 files changed, 105 insertions, 51 deletions

diff --git a/jstests/core/command_let_variables.js b/jstests/core/command_let_variables.js
index 08ad58131ab..cd0a63425e6 100644
--- a/jstests/core/command_let_variables.js
+++ b/jstests/core/command_let_variables.js
@@ -341,4 +341,65 @@ expectedResults = {
     suspect: "dino"
 };
 assert.eq(expectedResults, result.value, result);
+
+// Test that update respects different parameters in both the query and update part.
+result = assert.commandWorked(testDB.runCommand({
+    update: coll.getName(),
+    updates: [
+        {q: {$expr: {$eq: ["$Species", "$$target_species"]}}, u: [{$set: {Species: "$$new_name"}}]}
+    ],
+    let : {target_species: "Chaffinch (Fringilla coelebs)", new_name: "Chaffinch"}
+}));
+assert.eq(result.n, 1);
+assert.eq(result.nModified, 1);
+
+result = assert.commandWorked(testDB.runCommand(
+    {find: coll.getName(), filter: {$expr: {$eq: ["$Species", "Chaffinch (Fringilla coelebs)"]}}}));
+assert.eq(result.cursor.firstBatch.length, 0);
+
+result = assert.commandWorked(
+    testDB.runCommand({find: coll.getName(), filter: {$expr: {$eq: ["$Species", "Chaffinch"]}}}));
+assert.eq(result.cursor.firstBatch.length, 1);
+
+// Test that update respects runtime constants and parameters.
+result = assert.commandWorked(testDB.runCommand({
+    update: coll.getName(),
+    updates: [{
+        q: {$expr: {$eq: ["$Species", "$$target_species"]}},
+        u: [{$set: {Timestamp: "$$NOW"}}, {$set: {Species: "$$new_name"}}]
+    }],
+    let : {target_species: "Chaffinch", new_name: "Pied Piper"}
+}));
+assert.eq(result.n, 1);
+assert.eq(result.nModified, 1);
+
+result = assert.commandWorked(
+    testDB.runCommand({find: coll.getName(), filter: {$expr: {$eq: ["$Species", "Chaffinch"]}}}));
+assert.eq(result.cursor.firstBatch.length, 0, result);
+
+result = assert.commandWorked(
+    testDB.runCommand({find: coll.getName(), filter: {$expr: {$eq: ["$Species", "Pied Piper"]}}}));
+assert.eq(result.cursor.firstBatch.length, 1, result);
+
+// Test that undefined let params in the update's query part fail gracefully.
+assert.commandFailedWithCode(testDB.runCommand({
+    update: coll.getName(),
+    updates: [{
+        q: {$expr: {$eq: ["$Species", "$$target_species"]}},
+        u: [{$set: {Species: "Homo Erectus"}}]
+    }],
+    let : {cat: "not_a_bird"}
+}),
+                             17276);
+
+// Test that undefined let params in the update's update part fail gracefully.
+assert.commandFailedWithCode(testDB.runCommand({
+    update: coll.getName(),
+    updates: [{
+        q: {$expr: {$eq: ["$Species", "Chaffinch (Fringilla coelebs)"]}},
+        u: [{$set: {Species: "$$new_name"}}]
+    }],
+    let : {cat: "not_a_bird"}
+}),
+                             17276);
 }());
diff --git a/jstests/noPassthroughWithMongod/command_let_variables.js b/jstests/noPassthroughWithMongod/command_let_variables.js
index 7cfc6c1ad5e..7ae405cade6 100644
--- a/jstests/noPassthroughWithMongod/command_let_variables.js
+++ b/jstests/noPassthroughWithMongod/command_let_variables.js
@@ -195,23 +195,4 @@ assert.commandWorked(db.runCommand({
     let : {variable: "OUTER"}
 }));
 assert.eq(targetColl.aggregate({$match: {$expr: {$eq: ["$var", "INNER"]}}}).toArray().length, 1);
-
-// Update
-assert.commandWorked(db.runCommand({
-    update: coll.getName(),
-    let : {target_species: "Song Thrush (Turdus philomelos)", new_name: "Song Thrush"},
-    updates: [
-        {q: {$expr: {$eq: ["$Species", "$$target_species"]}}, u: [{$set: {Species: "$$new_name"}}]}
-    ]
-}));
-
-assert.commandWorked(db.runCommand({
-    update: coll.getName(),
-    let : {target_species: "Song Thrush (Turdus philomelos)"},
-    updates: [{
-        q: {$expr: {$eq: ["$Species", "$$target_species"]}},
-        u: [{$set: {Location: "$$place"}}],
-        c: {place: "North America"}
-    }]
-}));
 }());
diff --git a/src/mongo/db/ops/update_request.h b/src/mongo/db/ops/update_request.h
index a18fe293690..5c917fe277f 100644
--- a/src/mongo/db/ops/update_request.h
+++ b/src/mongo/db/ops/update_request.h
@@ -272,6 +272,10 @@ public:
             builder << " runtimeConstants: " << _runtimeConstants->toBSON().toString();
         }
 
+        if (_letParameters) {
+            builder << " letParameters: " << _letParameters;
+        }
+
         builder << " god: " << _god;
         builder << " upsert: " << isUpsert();
         builder << " multi: " << isMulti();
diff --git a/src/mongo/db/pipeline/aggregation_request.cpp b/src/mongo/db/pipeline/aggregation_request.cpp
index b75fb66a055..d2a7b9d0fbb 100644
--- a/src/mongo/db/pipeline/aggregation_request.cpp
+++ b/src/mongo/db/pipeline/aggregation_request.cpp
@@ -189,7 +189,7 @@ StatusWith<AggregationRequest> AggregationRequest::parseFromBSON(
 
             auto writeConcern = uassertStatusOK(WriteConcernOptions::parse(elem.embeddedObject()));
             request.setWriteConcern(writeConcern);
-        } else if (kRuntimeConstants == fieldName) {
+        } else if (kRuntimeConstantsName == fieldName) {
             // TODO SERVER-46384: Remove 'runtimeConstants' in 4.5 since it is redundant with 'let'
             try {
                 IDLParserErrorContext ctx("internalRuntimeConstants");
@@ -197,18 +197,18 @@ StatusWith<AggregationRequest> AggregationRequest::parseFromBSON(
             } catch (const DBException& ex) {
                 return ex.toStatus();
             }
-        } else if (kLet == fieldName) {
+        } else if (kLetName == fieldName) {
             if (elem.type() != BSONType::Object)
                 return {ErrorCodes::TypeMismatch,
                         str::stream()
                             << fieldName << " must be an object, not a " << typeName(elem.type())};
-            auto bob = BSONObjBuilder{request.letParameters};
+            auto bob = BSONObjBuilder{request.getLetParameters()};
             bob.appendElementsUnique(elem.embeddedObject());
-            request.letParameters = bob.obj();
-        } else if (fieldName == kUse44SortKeys) {
+            request._letParameters = bob.obj();
+        } else if (fieldName == kUse44SortKeysName) {
             if (elem.type() != BSONType::Bool) {
                 return {ErrorCodes::TypeMismatch,
-                        str::stream() << kUse44SortKeys << " must be a boolean, not a "
+                        str::stream() << kUse44SortKeysName << " must be a boolean, not a "
                                       << typeName(elem.type())};
             }
             // TODO SERVER-47065: A 4.6 node still has to accept the 'use44SortKeys' field, since it
@@ -219,10 +219,10 @@ StatusWith<AggregationRequest> AggregationRequest::parseFromBSON(
             // 4.6 upgrade purposes, since a 4.4 mongoS will always send {useNewUpsert:true} to the
             // shards. We do nothing with it because useNewUpsert will be automatically used in 4.6
             // when appropriate. Remove this final vestige of useNewUpsert during the 4.7 dev cycle.
-        } else if (fieldName == kIsMapReduceCommand) {
+        } else if (fieldName == kIsMapReduceCommandName) {
             if (elem.type() != BSONType::Bool) {
                 return {ErrorCodes::TypeMismatch,
-                        str::stream() << kIsMapReduceCommand << " must be a boolean, not a "
+                        str::stream() << kIsMapReduceCommandName << " must be a boolean, not a "
                                       << typeName(elem.type())};
             }
             request.setIsMapReduceCommand(elem.boolean());
@@ -327,9 +327,9 @@ Document AggregationRequest::serializeToCommandObj() const {
         {WriteConcernOptions::kWriteConcernField,
          _writeConcern ? Value(_writeConcern->toBSON()) : Value()},
         // Only serialize runtime constants if any were specified.
-        {kRuntimeConstants, _runtimeConstants ? Value(_runtimeConstants->toBSON()) : Value()},
-        {kIsMapReduceCommand, _isMapReduceCommand ? Value(true) : Value()},
-        {kLet, !letParameters.isEmpty() ? Value(letParameters) : Value()},
+        {kRuntimeConstantsName, _runtimeConstants ? Value(_runtimeConstants->toBSON()) : Value()},
+        {kIsMapReduceCommandName, _isMapReduceCommand ? Value(true) : Value()},
+        {kLetName, !_letParameters.isEmpty() ? Value(_letParameters) : Value()},
     };
 }
 }  // namespace mongo
diff --git a/src/mongo/db/pipeline/aggregation_request.h b/src/mongo/db/pipeline/aggregation_request.h
index 3e1de8412f6..7788ec962aa 100644
--- a/src/mongo/db/pipeline/aggregation_request.h
+++ b/src/mongo/db/pipeline/aggregation_request.h
@@ -62,10 +62,10 @@ public:
     static constexpr StringData kAllowDiskUseName = "allowDiskUse"_sd;
     static constexpr StringData kHintName = "hint"_sd;
     static constexpr StringData kExchangeName = "exchange"_sd;
-    static constexpr StringData kRuntimeConstants = "runtimeConstants"_sd;
-    static constexpr StringData kUse44SortKeys = "use44SortKeys"_sd;
-    static constexpr StringData kIsMapReduceCommand = "isMapReduceCommand"_sd;
-    static constexpr StringData kLet = "let"_sd;
+    static constexpr StringData kRuntimeConstantsName = "runtimeConstants"_sd;
+    static constexpr StringData kUse44SortKeysName = "use44SortKeys"_sd;
+    static constexpr StringData kIsMapReduceCommandName = "isMapReduceCommand"_sd;
+    static constexpr StringData kLetName = "let"_sd;
 
     static constexpr long long kDefaultBatchSize = 101;
 
@@ -219,6 +219,10 @@ public:
         return _runtimeConstants;
     }
 
+    const auto& getLetParameters() const {
+        return _letParameters;
+    }
+
     bool getIsMapReduceCommand() const {
         return _isMapReduceCommand;
     }
@@ -287,14 +291,14 @@ public:
         _runtimeConstants = std::move(runtimeConstants);
     }
 
+    void setLetParameters(BSONObj letParameters) {
+        _letParameters = letParameters.getOwned();
+    }
+
     void setIsMapReduceCommand(bool isMapReduce) {
         _isMapReduceCommand = isMapReduce;
     }
 
-    // A document containing user-specified let parameter constants; i.e. values that do not change
-    // once computed.
-    BSONObj letParameters;
-
 private:
     // Required fields.
     const NamespaceString _nss;
@@ -345,6 +349,10 @@ private:
     // $$NOW).
     boost::optional<RuntimeConstants> _runtimeConstants;
 
+    // A document containing user-specified let parameter constants; i.e. values that do not change
+    // once computed.
+    BSONObj _letParameters;
+
     // True when an aggregation was invoked by the MapReduce command.
     bool _isMapReduceCommand = false;
 };
diff --git a/src/mongo/db/pipeline/aggregation_request_test.cpp b/src/mongo/db/pipeline/aggregation_request_test.cpp
index 4618f9035bf..f16b7a5891b 100644
--- a/src/mongo/db/pipeline/aggregation_request_test.cpp
+++ b/src/mongo/db/pipeline/aggregation_request_test.cpp
@@ -192,7 +192,7 @@ TEST(AggregationRequestTest, ShouldSerializeOptionalValuesIfSet) {
     request.setIsMapReduceCommand(true);
     const auto letParamsObj = BSON("foo"
                                    << "bar");
-    request.letParameters = letParamsObj;
+    request.setLetParameters(letParamsObj);
 
     auto expectedSerialization =
         Document{{AggregationRequest::kCommandName, nss.coll()},
@@ -208,8 +208,8 @@ TEST(AggregationRequestTest, ShouldSerializeOptionalValuesIfSet) {
                  {repl::ReadConcernArgs::kReadConcernFieldName, readConcernObj},
                  {QueryRequest::kUnwrappedReadPrefField, readPrefObj},
                  {QueryRequest::cmdOptionMaxTimeMS, 10},
-                 {AggregationRequest::kIsMapReduceCommand, true},
-                 {AggregationRequest::kLet, letParamsObj}};
+                 {AggregationRequest::kIsMapReduceCommandName, true},
+                 {AggregationRequest::kLetName, letParamsObj}};
     ASSERT_DOCUMENT_EQ(request.serializeToCommandObj(), expectedSerialization);
 }
 
diff --git a/src/mongo/db/pipeline/expression_context.cpp b/src/mongo/db/pipeline/expression_context.cpp
index f33d6a16e06..bc5b3d26608 100644
--- a/src/mongo/db/pipeline/expression_context.cpp
+++ b/src/mongo/db/pipeline/expression_context.cpp
@@ -65,7 +65,7 @@ ExpressionContext::ExpressionContext(OperationContext* opCtx,
                         std::move(processInterface),
                         std::move(resolvedNamespaces),
                         std::move(collUUID),
-                        request.letParameters,
+                        request.getLetParameters(),
                         mayDbProfile) {
 
     if (request.getIsMapReduceCommand()) {
diff --git a/src/mongo/db/pipeline/sharded_agg_helpers.cpp b/src/mongo/db/pipeline/sharded_agg_helpers.cpp
index 7e02ec30fe3..1058c618cfd 100644
--- a/src/mongo/db/pipeline/sharded_agg_helpers.cpp
+++ b/src/mongo/db/pipeline/sharded_agg_helpers.cpp
@@ -127,7 +127,7 @@ BSONObj genericTransformForShards(MutableDocument&& cmdForShards,
                                   const boost::optional<RuntimeConstants>& constants,
                                   BSONObj collationObj) {
     if (constants) {
-        cmdForShards[AggregationRequest::kRuntimeConstants] = Value(constants.get().toBSON());
+        cmdForShards[AggregationRequest::kRuntimeConstantsName] = Value(constants.get().toBSON());
     }
 
     cmdForShards[AggregationRequest::kFromMongosName] = Value(expCtx->inMongos);
diff --git a/src/mongo/db/query/find_and_modify_request.cpp b/src/mongo/db/query/find_and_modify_request.cpp
index 660cc9c95a5..582f4753d13 100644
--- a/src/mongo/db/query/find_and_modify_request.cpp
+++ b/src/mongo/db/query/find_and_modify_request.cpp
@@ -48,7 +48,7 @@ const char kHintField[] = "hint";
 const char kCollationField[] = "collation";
 const char kArrayFiltersField[] = "arrayFilters";
 const char kRuntimeConstantsField[] = "runtimeConstants";
-const char kLet[] = "let";
+const char kLetField[] = "let";
 const char kRemoveField[] = "remove";
 const char kUpdateField[] = "update";
 const char kNewField[] = "new";
@@ -138,7 +138,7 @@ BSONObj FindAndModifyRequest::toBSON(const BSONObj& commandPassthroughFields) co
 
     if (_letParameters) {
         if (auto letParams = _letParameters.get(); !letParams.isEmpty()) {
-            builder.append(kLet, _letParameters.get());
+            builder.append(kLetField, _letParameters.get());
         }
     }
 
@@ -253,10 +253,10 @@ StatusWith<FindAndModifyRequest> FindAndModifyRequest::parseFromBSON(NamespaceSt
             runtimeConstants =
                 RuntimeConstants::parse(IDLParserErrorContext(kRuntimeConstantsField),
                                         cmdObj.getObjectField(kRuntimeConstantsField));
-        } else if (field == kLet) {
+        } else if (field == kLetField) {
             BSONElement letElt;
             if (Status letEltStatus =
-                    bsonExtractTypedField(cmdObj, kLet, BSONType::Object, &letElt);
+                    bsonExtractTypedField(cmdObj, kLetField, BSONType::Object, &letElt);
                 !letEltStatus.isOK()) {
                 return letEltStatus;
             }
diff --git a/src/mongo/db/views/resolved_view.cpp b/src/mongo/db/views/resolved_view.cpp
index 714644ecca3..5bb17093d21 100644
--- a/src/mongo/db/views/resolved_view.cpp
+++ b/src/mongo/db/views/resolved_view.cpp
@@ -112,7 +112,7 @@ AggregationRequest ResolvedView::asExpandedViewAggregation(
     expandedRequest.setBypassDocumentValidation(request.shouldBypassDocumentValidation());
     expandedRequest.setAllowDiskUse(request.shouldAllowDiskUse());
     expandedRequest.setIsMapReduceCommand(request.getIsMapReduceCommand());
-    expandedRequest.letParameters = request.letParameters;
+    expandedRequest.setLetParameters(request.getLetParameters());
 
     // Operations on a view must always use the default collation of the view. We must have already
     // checked that if the user's request specifies a collation, it matches the collation of the
diff --git a/src/mongo/s/commands/cluster_map_reduce_agg.cpp b/src/mongo/s/commands/cluster_map_reduce_agg.cpp
index 224e82dc6ca..7908cdec77b 100644
--- a/src/mongo/s/commands/cluster_map_reduce_agg.cpp
+++ b/src/mongo/s/commands/cluster_map_reduce_agg.cpp
@@ -118,9 +118,9 @@ Document serializeToCommand(BSONObj originalCmd, const MapReduce& parsedMr, Pipe
         Value(Document{{"batchSize", std::numeric_limits<long long>::max()}});
     translatedCmd[AggregationRequest::kAllowDiskUseName] = Value(true);
     translatedCmd[AggregationRequest::kFromMongosName] = Value(true);
-    translatedCmd[AggregationRequest::kRuntimeConstants] =
+    translatedCmd[AggregationRequest::kRuntimeConstantsName] =
         Value(pipeline->getContext()->getRuntimeConstants().toBSON());
-    translatedCmd[AggregationRequest::kIsMapReduceCommand] = Value(true);
+    translatedCmd[AggregationRequest::kIsMapReduceCommandName] = Value(true);
 
     if (shouldBypassDocumentValidationForCommand(originalCmd)) {
         translatedCmd[bypassDocumentValidationCommandOption()] = Value(true);
diff --git a/src/mongo/s/query/cluster_aggregation_planner.cpp b/src/mongo/s/query/cluster_aggregation_planner.cpp
index 5c4716e229f..5b7ea00286f 100644
--- a/src/mongo/s/query/cluster_aggregation_planner.cpp
+++ b/src/mongo/s/query/cluster_aggregation_planner.cpp
@@ -126,7 +126,7 @@ BSONObj createCommandForMergingShard(Document serializedCommand,
     mergeCmd["pipeline"] = Value(pipelineForMerging->serialize());
     mergeCmd[AggregationRequest::kFromMongosName] = Value(true);
 
-    mergeCmd[AggregationRequest::kRuntimeConstants] =
+    mergeCmd[AggregationRequest::kRuntimeConstantsName] =
         Value(mergeCtx->getRuntimeConstants().toBSON());
 
     // If the user didn't specify a collation already, make sure there's a collation attached to