SERVER-63428 Robustify oplog applying code for update operation

(cherry picked from commit a3158ab422e4d8091203166c5f2601f9bfa0099d)

19 files changed, 180 insertions, 105 deletions

diff --git a/jstests/core/apply_ops1.js b/jstests/core/apply_ops1.js
index 9ecaef4c763..86754d7b9ba 100644
--- a/jstests/core/apply_ops1.js
+++ b/jstests/core/apply_ops1.js
@@ -457,7 +457,7 @@ res = assert.commandFailed(db.adminCommand({
         }
     ]
 }));
-assert.eq(res.code, 4772604);
+assert.eq(res.code, 4772600);
 
 // When we explicitly specify {$v: 1}, we should get 'UpdateNode' update semantics, and $set
 // operations get performed in lexicographic order.
@@ -503,7 +503,7 @@ res = assert.commandFailed(db.adminCommand({
         }
     ]
 }));
-assert.eq(res.code, 4772604);
+assert.eq(res.code, 4772600);
 
 var insert_op1 = {_id: 13, x: 'inserted apply ops1'};
 var insert_op2 = {_id: 14, x: 'inserted apply ops2'};
diff --git a/jstests/replsets/update_with_dollar_fields.js b/jstests/replsets/update_with_dollar_fields.js
new file mode 100644
index 00000000000..9ecb8a4399c
--- /dev/null
+++ b/jstests/replsets/update_with_dollar_fields.js
@@ -0,0 +1,50 @@
+/**
+ * Tests that replacement style update with $v field in the document is correctly applied.
+ * @tags: [
+ *  requires_fcv_60,
+ * ]
+ */
+
+(function() {
+"use strict";
+
+const rst = new ReplSetTest({nodes: 2});
+rst.startSet();
+rst.initiate();
+
+const dbName = 'testDb';
+const collName = 'testColl';
+const primary = rst.getPrimary();
+const coll = primary.getDB(dbName).getCollection(collName);
+const oplog = primary.getDB('local').getCollection('oplog.rs');
+
+function assertLastUpdateOplogEntryIsReplacement() {
+    const lastUpdate = oplog.find({op: 'u'}).sort({$natural: -1}).limit(1).next();
+    assert(lastUpdate.o._id);
+}
+
+[true].forEach($v => {
+    const _id = assert.commandWorked(coll.insertOne({$v})).insertedId;
+    assert.commandWorked(coll.update({_id}, [{$set: {p: 1, q: 1}}]));
+    assertLastUpdateOplogEntryIsReplacement();
+});
+
+[true, "hello", 0, 1, 2, 3].forEach($v => {
+    const _id = assert.commandWorked(coll.insertOne({})).insertedId;
+    assert.commandWorked(coll.update(
+        {_id},
+        [{$replaceWith: {"$setField": {field: {$literal: "$v"}, input: "$$ROOT", value: $v}}}]));
+    assertLastUpdateOplogEntryIsReplacement();
+});
+
+(function() {
+const _id = assert.commandWorked(coll.insertOne({})).insertedId;
+assert.commandWorked(coll.update(
+    {_id},
+    [{$replaceWith: {"$setField": {field: {$literal: "$set"}, input: "$$ROOT", value: {a: 1}}}}]));
+assertLastUpdateOplogEntryIsReplacement();
+})();
+
+rst.awaitReplication();
+rst.stopSet();
+}());
diff --git a/jstests/sharding/update_sharded.js b/jstests/sharding/update_sharded.js
index ee9ec595bb6..73da8316f4e 100644
--- a/jstests/sharding/update_sharded.js
+++ b/jstests/sharding/update_sharded.js
@@ -40,6 +40,14 @@ for (let i = 0; i < 2; i++) {
     assert.commandWorked(coll.update({_id: 2, key: 2}, {key: 2, foo: 'bar'}, {upsert: true}));
     assert.commandWorked(coll.update({_id: 3, key: 3}, {$set: {foo: 'bar'}}, {upsert: true}));
 
+    // Mixing operator & non-operator fields in updates is not allowed.
+    assert.commandFailedWithCode(
+        coll.update({_id: 4, key: 4}, {key: 4, $baz: {foo: 'bar'}}, {upsert: true}),
+        ErrorCodes.UnsupportedFormat);
+    assert.commandFailedWithCode(
+        coll.update({_id: 5, key: 5}, {$baz: {foo: 'bar'}, key: 5}, {upsert: true}),
+        ErrorCodes.UnsupportedFormat);
+
     assert.eq(coll.count(), 3, "count A");
     assert.eq(coll.findOne({_id: 3}).key, 3, "findOne 3 key A");
     assert.eq(coll.findOne({_id: 3}).foo, 'bar', "findOne 3 foo A");
diff --git a/src/mongo/db/ops/write_ops.cpp b/src/mongo/db/ops/write_ops.cpp
index b2b0758b284..c4b6d5bf54a 100644
--- a/src/mongo/db/ops/write_ops.cpp
+++ b/src/mongo/db/ops/write_ops.cpp
@@ -134,6 +134,11 @@ int32_t getStmtIdForWriteAt(const WriteCommandRequestBase& writeCommandBase, siz
     return kFirstStmtId + writePos;
 }
 
+bool isClassicalUpdateReplacement(const BSONObj& update) {
+    // An empty update object will be treated as replacement as firstElementFieldName() returns "".
+    return update.firstElementFieldName()[0] != '$';
+}
+
 }  // namespace write_ops
 
 write_ops::InsertCommandRequest InsertOp::parse(const OpMsgRequest& request) {
@@ -223,15 +228,20 @@ void DeleteOp::validate(const DeleteCommandRequest& deleteOp) {
 write_ops::UpdateModification write_ops::UpdateModification::parseFromOplogEntry(
     const BSONObj& oField, const DiffOptions& options) {
     BSONElement vField = oField[kUpdateOplogEntryVersionFieldName];
+    BSONElement idField = oField["_id"];
 
-    // If this field appears it should be an integer.
+    // If _id field is present, we're getting a replacement style update in which $v can be a user
+    // field. Otherwise, $v field has to be either missing or be one of the version flag $v:1 /
+    // $v:2.
     uassert(4772600,
-            str::stream() << "Expected $v field to be missing or an integer, but got type: "
-                          << vField.type(),
-            !vField.ok() ||
-                (vField.type() == BSONType::NumberInt || vField.type() == BSONType::NumberLong));
-
-    if (vField.ok() && vField.numberInt() == static_cast<int>(UpdateOplogEntryVersion::kDeltaV2)) {
+            str::stream() << "Expected _id field or $v field missing or $v:1/$v:2, but got: "
+                          << vField,
+            idField.ok() || !vField.ok() ||
+                vField.numberInt() == static_cast<int>(UpdateOplogEntryVersion::kUpdateNodeV1) ||
+                vField.numberInt() == static_cast<int>(UpdateOplogEntryVersion::kDeltaV2));
+
+    if (!idField.ok() && vField.ok() &&
+        vField.numberInt() == static_cast<int>(UpdateOplogEntryVersion::kDeltaV2)) {
         // Make sure there's a diff field.
         BSONElement diff = oField[update_oplog_entry::kDiffObjectFieldName];
         uassert(4772601,
@@ -240,18 +250,11 @@ write_ops::UpdateModification write_ops::UpdateModification::parseFromOplogEntry
                 diff.type() == BSONType::Object);
 
         return UpdateModification(doc_diff::Diff{diff.embeddedObject()}, options);
-    } else if (!vField.ok() ||
-               vField.numberInt() == static_cast<int>(UpdateOplogEntryVersion::kUpdateNodeV1)) {
+    } else {
         // Treat it as a "classic" update which can either be a full replacement or a
-        // modifier-style update. Which variant it is will be determined when the update driver is
-        // constructed.
-        return UpdateModification(oField, ClassicTag{});
+        // modifier-style update. Use "_id" field to determine whether which style it is.
+        return UpdateModification(oField, ClassicTag{}, idField.ok());
     }
-
-    // The $v field must be present, but have some unsupported value.
-    uasserted(4772604,
-              str::stream() << "Unrecognized value for '$v' (Version) field: "
-                            << vField.numberInt());
 }
 
 write_ops::UpdateModification::UpdateModification(doc_diff::Diff diff, DiffOptions options)
@@ -263,7 +266,7 @@ write_ops::UpdateModification::UpdateModification(TransformFunc transform)
 write_ops::UpdateModification::UpdateModification(BSONElement update) {
     const auto type = update.type();
     if (type == BSONType::Object) {
-        _update = ClassicUpdate{update.Obj()};
+        _update = UpdateModification(update.Obj(), ClassicTag{})._update;
         return;
     }
 
@@ -274,19 +277,24 @@ write_ops::UpdateModification::UpdateModification(BSONElement update) {
     _update = PipelineUpdate{parsePipelineFromBSON(update)};
 }
 
-write_ops::UpdateModification::UpdateModification(const BSONObj& update, ClassicTag) {
-    // Do a sanity check that the $v field is either not provided or has value of 1.
-    const auto versionElem = update["$v"];
-    uassert(4772602,
-            str::stream() << "Expected classic update either contain no '$v' field, or "
-                          << "'$v' field with value 1, but found: " << versionElem,
-            !versionElem.ok() ||
-                versionElem.numberInt() ==
-                    static_cast<int>(UpdateOplogEntryVersion::kUpdateNodeV1));
-
-    _update = ClassicUpdate{update};
+// If we know whether the update is a replacement, use that value. For example, when we're parsing
+// the oplog entry, we know if the update is a replacement by checking whether there's an _id field.
+write_ops::UpdateModification::UpdateModification(const BSONObj& update,
+                                                  ClassicTag,
+                                                  bool isReplacement) {
+    if (isReplacement) {
+        _update = ReplacementUpdate{update};
+    } else {
+        _update = ModifierUpdate{update};
+    }
 }
 
+// If we don't know whether the update is a replacement, for example while we are parsing a user
+// request, we infer this by checking whether the first element is a $-field to distinguish modifier
+// style updates.
+write_ops::UpdateModification::UpdateModification(const BSONObj& update, ClassicTag)
+    : UpdateModification(update, ClassicTag{}, isClassicalUpdateReplacement(update)) {}
+
 write_ops::UpdateModification::UpdateModification(std::vector<BSONObj> pipeline)
     : _update{PipelineUpdate{std::move(pipeline)}} {}
 
@@ -301,7 +309,8 @@ write_ops::UpdateModification write_ops::UpdateModification::parseFromBSON(BSONE
 int write_ops::UpdateModification::objsize() const {
     return stdx::visit(
         visit_helper::Overloaded{
-            [](const ClassicUpdate& classic) -> int { return classic.bson.objsize(); },
+            [](const ReplacementUpdate& replacement) -> int { return replacement.bson.objsize(); },
+            [](const ModifierUpdate& modifier) -> int { return modifier.bson.objsize(); },
             [](const PipelineUpdate& pipeline) -> int {
                 int size = 0;
                 std::for_each(pipeline.begin(), pipeline.end(), [&size](const BSONObj& obj) {
@@ -319,7 +328,8 @@ int write_ops::UpdateModification::objsize() const {
 write_ops::UpdateModification::Type write_ops::UpdateModification::type() const {
     return stdx::visit(
         visit_helper::Overloaded{
-            [](const ClassicUpdate& classic) { return Type::kClassic; },
+            [](const ReplacementUpdate& replacement) { return Type::kReplacement; },
+            [](const ModifierUpdate& modifier) { return Type::kModifier; },
             [](const PipelineUpdate& pipelineUpdate) { return Type::kPipeline; },
             [](const DeltaUpdate& delta) { return Type::kDelta; },
             [](const TransformUpdate& transform) { return Type::kTransform; }},
@@ -335,7 +345,12 @@ void write_ops::UpdateModification::serializeToBSON(StringData fieldName,
 
     stdx::visit(
         visit_helper::Overloaded{
-            [fieldName, bob](const ClassicUpdate& classic) { *bob << fieldName << classic.bson; },
+            [fieldName, bob](const ReplacementUpdate& replacement) {
+                *bob << fieldName << replacement.bson;
+            },
+            [fieldName, bob](const ModifierUpdate& modifier) {
+                *bob << fieldName << modifier.bson;
+            },
             [fieldName, bob](const PipelineUpdate& pipeline) {
                 BSONArrayBuilder arrayBuilder(bob->subarrayStart(fieldName));
                 for (auto&& stage : pipeline) {
diff --git a/src/mongo/db/ops/write_ops_parsers.h b/src/mongo/db/ops/write_ops_parsers.h
index 4a70bfdc0da..797af279df7 100644
--- a/src/mongo/db/ops/write_ops_parsers.h
+++ b/src/mongo/db/ops/write_ops_parsers.h
@@ -79,7 +79,7 @@ repl::OpTime opTimeParser(BSONElement elem);
 
 class UpdateModification {
 public:
-    enum class Type { kClassic, kPipeline, kDelta, kTransform };
+    enum class Type { kReplacement, kModifier, kPipeline, kDelta, kTransform };
     using TransformFunc = std::function<boost::optional<BSONObj>(const BSONObj&)>;
 
     /**
@@ -109,6 +109,7 @@ public:
     // Creates an transform-style update. The transform function MUST preserve the _id element.
     UpdateModification(TransformFunc transform);
     // This constructor exists only to provide a fast-path for constructing classic-style updates.
+    UpdateModification(const BSONObj& update, ClassicTag, bool isReplacement);
     UpdateModification(const BSONObj& update, ClassicTag);
 
     /**
@@ -129,9 +130,14 @@ public:
 
     Type type() const;
 
-    BSONObj getUpdateClassic() const {
-        invariant(type() == Type::kClassic);
-        return stdx::get<ClassicUpdate>(_update).bson;
+    BSONObj getUpdateReplacement() const {
+        invariant(type() == Type::kReplacement);
+        return stdx::get<ReplacementUpdate>(_update).bson;
+    }
+
+    BSONObj getUpdateModifier() const {
+        invariant(type() == Type::kModifier);
+        return stdx::get<ModifierUpdate>(_update).bson;
     }
 
     const std::vector<BSONObj>& getUpdatePipeline() const {
@@ -159,8 +165,11 @@ public:
 
         stdx::visit(
             visit_helper::Overloaded{
-                [&sb](const ClassicUpdate& classic) {
-                    sb << "{type: Classic, update: " << classic.bson << "}";
+                [&sb](const ReplacementUpdate& replacement) {
+                    sb << "{type: Replacement, update: " << replacement.bson << "}";
+                },
+                [&sb](const ModifierUpdate& modifier) {
+                    sb << "{type: Modifier, update: " << modifier.bson << "}";
                 },
                 [&sb](const PipelineUpdate& pipeline) {
                     sb << "{type: Pipeline, update: " << Value(pipeline).toString() << "}";
@@ -176,7 +185,10 @@ public:
 
 private:
     // Wrapper class used to avoid having a variant where multiple alternatives have the same type.
-    struct ClassicUpdate {
+    struct ReplacementUpdate {
+        BSONObj bson;
+    };
+    struct ModifierUpdate {
         BSONObj bson;
     };
     using PipelineUpdate = std::vector<BSONObj>;
@@ -187,7 +199,8 @@ private:
     struct TransformUpdate {
         TransformFunc transform;
     };
-    stdx::variant<ClassicUpdate, PipelineUpdate, DeltaUpdate, TransformUpdate> _update;
+    stdx::variant<ReplacementUpdate, ModifierUpdate, PipelineUpdate, DeltaUpdate, TransformUpdate>
+        _update;
 };
 
 }  // namespace write_ops
diff --git a/src/mongo/db/ops/write_ops_parsers_test.cpp b/src/mongo/db/ops/write_ops_parsers_test.cpp
index 7fd55f1ef32..dff6ab2cb0b 100644
--- a/src/mongo/db/ops/write_ops_parsers_test.cpp
+++ b/src/mongo/db/ops/write_ops_parsers_test.cpp
@@ -311,8 +311,8 @@ TEST(CommandWriteOpsParsers, UpdateCommandRequest) {
                 ASSERT_BSONOBJ_EQ(op.getUpdates()[0].getQ(), query);
 
                 const auto& updateMod = op.getUpdates()[0].getU();
-                ASSERT(updateMod.type() == write_ops::UpdateModification::Type::kClassic);
-                ASSERT_BSONOBJ_EQ(updateMod.getUpdateClassic(), update);
+                ASSERT(updateMod.type() == write_ops::UpdateModification::Type::kModifier);
+                ASSERT_BSONOBJ_EQ(updateMod.getUpdateModifier(), update);
 
                 ASSERT_BSONOBJ_EQ(write_ops::collationOf(op.getUpdates()[0]), collation);
                 ASSERT_EQ(write_ops::arrayFiltersOf(op.getUpdates()[0]).size(), 1u);
diff --git a/src/mongo/db/pipeline/document_source_merge.cpp b/src/mongo/db/pipeline/document_source_merge.cpp
index 0715ac8e65a..80daf31c772 100644
--- a/src/mongo/db/pipeline/document_source_merge.cpp
+++ b/src/mongo/db/pipeline/document_source_merge.cpp
@@ -136,7 +136,7 @@ MergeStrategy makeInsertStrategy() {
         // The batch stores replacement style updates, but for this "insert" style of $merge we'd
         // like to just insert the new document without attempting any sort of replacement.
         std::transform(batch.begin(), batch.end(), objectsToInsert.begin(), [](const auto& obj) {
-            return std::get<UpdateModification>(obj).getUpdateClassic();
+            return std::get<UpdateModification>(obj).getUpdateReplacement();
         });
         uassertStatusOK(expCtx->mongoProcessInterface->insert(
             expCtx, ns, std::move(objectsToInsert), wc, epoch));
@@ -151,7 +151,7 @@ BatchTransform makeUpdateTransform(const std::string& updateOp) {
     return [updateOp](auto& batch) {
         for (auto&& obj : batch) {
             std::get<UpdateModification>(obj) = UpdateModification::parseFromClassicUpdate(
-                BSON(updateOp << std::get<UpdateModification>(obj).getUpdateClassic()));
+                BSON(updateOp << std::get<UpdateModification>(obj).getUpdateReplacement()));
         }
     };
 }
diff --git a/src/mongo/db/s/config/sharding_catalog_manager_add_shard_test.cpp b/src/mongo/db/s/config/sharding_catalog_manager_add_shard_test.cpp
index 0d1f3189c66..cf5dbc04b55 100644
--- a/src/mongo/db/s/config/sharding_catalog_manager_add_shard_test.cpp
+++ b/src/mongo/db/s/config/sharding_catalog_manager_add_shard_test.cpp
@@ -226,8 +226,8 @@ protected:
                 ASSERT_EQ(itExpected->getUpsert(), itActual->getUpsert());
                 ASSERT_EQ(itExpected->getMulti(), itActual->getMulti());
                 ASSERT_BSONOBJ_EQ(itExpected->getQ(), itActual->getQ());
-                ASSERT_BSONOBJ_EQ(itExpected->getU().getUpdateClassic(),
-                                  itActual->getU().getUpdateClassic());
+                ASSERT_BSONOBJ_EQ(itExpected->getU().getUpdateReplacement(),
+                                  itActual->getU().getUpdateReplacement());
             }
 
             BatchedCommandResponse response;
@@ -268,8 +268,8 @@ protected:
                 ASSERT_EQ(itExpected->getUpsert(), itActual->getUpsert());
                 ASSERT_EQ(itExpected->getMulti(), itActual->getMulti());
                 ASSERT_BSONOBJ_EQ(itExpected->getQ(), itActual->getQ());
-                ASSERT_BSONOBJ_EQ(itExpected->getU().getUpdateClassic(),
-                                  itActual->getU().getUpdateClassic());
+                ASSERT_BSONOBJ_EQ(itExpected->getU().getUpdateReplacement(),
+                                  itActual->getU().getUpdateReplacement());
             }
 
             return statusToReturn;
diff --git a/src/mongo/db/s/dist_lock_catalog_replset_test.cpp b/src/mongo/db/s/dist_lock_catalog_replset_test.cpp
index 780e82f6d88..aedba30df96 100644
--- a/src/mongo/db/s/dist_lock_catalog_replset_test.cpp
+++ b/src/mongo/db/s/dist_lock_catalog_replset_test.cpp
@@ -1246,7 +1246,7 @@ TEST_F(DistLockCatalogReplSetTest, BasicUnlockAll) {
         ASSERT(update.getMulti());
         ASSERT_BSONOBJ_EQ(BSON(LocksType::process("processID")), update.getQ());
         ASSERT_BSONOBJ_EQ(BSON("$set" << BSON(LocksType::state(LocksType::UNLOCKED))),
-                          update.getU().getUpdateClassic());
+                          update.getU().getUpdateModifier());
 
         return BSON("ok" << 1);
     });
diff --git a/src/mongo/db/timeseries/timeseries_update_delete_util.cpp b/src/mongo/db/timeseries/timeseries_update_delete_util.cpp
index 91bf4fccc4b..5ed466dfe01 100644
--- a/src/mongo/db/timeseries/timeseries_update_delete_util.cpp
+++ b/src/mongo/db/timeseries/timeseries_update_delete_util.cpp
@@ -34,12 +34,6 @@
 
 namespace mongo::timeseries {
 namespace {
-/**
- * Returns whether the given document is a replacement document.
- */
-bool isDocReplacement(const BSONObj& doc) {
-    return doc.isEmpty() || (doc.firstElementFieldNameStringData().find("$") == std::string::npos);
-}
 
 /**
  * Returns whether the given metaField is the first element of the dotted path in the given
@@ -157,10 +151,11 @@ write_ops::UpdateModification translateUpdate(const write_ops::UpdateModificatio
             "Cannot perform an update on a time-series collection using a pipeline update",
             updateMod.type() != write_ops::UpdateModification::Type::kPipeline);
 
-    const auto& document = updateMod.getUpdateClassic();
     uassert(ErrorCodes::InvalidOptions,
             "Cannot perform an update on a time-series collection using a replacement document",
-            !isDocReplacement(document));
+            updateMod.type() != write_ops::UpdateModification::Type::kReplacement);
+
+    const auto& document = updateMod.getUpdateModifier();
 
     // Make a mutable copy of the update document in order to replace all occurrences of the
     // metaField with "meta".
diff --git a/src/mongo/db/timeseries/timeseries_update_delete_util_test.cpp b/src/mongo/db/timeseries/timeseries_update_delete_util_test.cpp
index b13e9bc2c02..397e6cb7c41 100644
--- a/src/mongo/db/timeseries/timeseries_update_delete_util_test.cpp
+++ b/src/mongo/db/timeseries/timeseries_update_delete_util_test.cpp
@@ -60,7 +60,7 @@ protected:
     BSONObj _translateUpdate(const BSONObj& update) const {
         return timeseries::translateUpdate(
                    write_ops::UpdateModification::parseFromClassicUpdate(update), _metaField)
-            .getUpdateClassic();
+            .getUpdateModifier();
     }
 
     BSONObj _translateUpdate(const char* update) const {
diff --git a/src/mongo/db/transaction_participant.cpp b/src/mongo/db/transaction_participant.cpp
index 6d1e69e92a4..46ff893a3bb 100644
--- a/src/mongo/db/transaction_participant.cpp
+++ b/src/mongo/db/transaction_participant.cpp
@@ -266,7 +266,9 @@ void updateSessionEntry(OperationContext* opCtx,
                         const LogicalSessionId& sessionId,
                         TxnNumber txnNum) {
     // Current code only supports replacement update.
-    dassert(UpdateDriver::isDocReplacement(updateRequest.getUpdateModification()));
+    dassert(updateRequest.getUpdateModification().type() ==
+            write_ops::UpdateModification::Type::kReplacement);
+    const auto updateMod = updateRequest.getUpdateModification().getUpdateReplacement();
 
     // TODO SERVER-58243: evaluate whether this is safe or whether acquiring the lock can block.
     AllowLockAcquisitionOnTimestampedUnitOfWork allowLockAcquisition(opCtx->lockState());
@@ -297,7 +299,6 @@ void updateSessionEntry(OperationContext* opCtx,
     dassert(idToFetch.fieldNameStringData() == "_id"_sd);
     auto recordId = indexAccess->findSingle(opCtx, *collection, toUpdateIdDoc);
     auto startingSnapshotId = opCtx->recoveryUnit()->getSnapshotId();
-    const auto updateMod = updateRequest.getUpdateModification().getUpdateClassic();
 
     if (recordId.isNull()) {
         // Upsert case.
diff --git a/src/mongo/db/update/update_driver.cpp b/src/mongo/db/update/update_driver.cpp
index 5b4809342be..d76c787be22 100644
--- a/src/mongo/db/update/update_driver.cpp
+++ b/src/mongo/db/update/update_driver.cpp
@@ -154,12 +154,12 @@ void UpdateDriver::parse(
     uassert(51198, "Constant values may only be specified for pipeline updates", !constants);
 
     // Check if the update expression is a full object replacement.
-    if (isDocReplacement(updateMod)) {
+    if (updateMod.type() == write_ops::UpdateModification::Type::kReplacement) {
         uassert(ErrorCodes::FailedToParse,
                 "multi update is not supported for replacement-style update",
                 !multi);
 
-        _updateExecutor = std::make_unique<ObjectReplaceExecutor>(updateMod.getUpdateClassic());
+        _updateExecutor = std::make_unique<ObjectReplaceExecutor>(updateMod.getUpdateReplacement());
 
         // Register the fact that this driver will only do full object replacements.
         _updateType = UpdateType::kReplacement;
@@ -185,7 +185,7 @@ void UpdateDriver::parse(
     // checked whether this is a delta update so we check that the $v field isn't present, or has a
     // value of 1.
 
-    auto updateExpr = updateMod.getUpdateClassic();
+    auto updateExpr = updateMod.getUpdateModifier();
     BSONElement versionElement = updateExpr[kUpdateOplogEntryVersionFieldName];
     if (versionElement) {
         uassert(ErrorCodes::FailedToParse,
@@ -316,10 +316,4 @@ void UpdateDriver::setCollator(const CollatorInterface* collator) {
     }
 }
 
-bool UpdateDriver::isDocReplacement(const write_ops::UpdateModification& updateMod) {
-    return (updateMod.type() == write_ops::UpdateModification::Type::kClassic &&
-            *updateMod.getUpdateClassic().firstElementFieldName() != '$') ||
-        updateMod.type() == write_ops::UpdateModification::Type::kPipeline;
-}
-
 }  // namespace mongo
diff --git a/src/mongo/db/update/update_driver.h b/src/mongo/db/update/update_driver.h
index d512d89b1cf..94d1f73c8d4 100644
--- a/src/mongo/db/update/update_driver.h
+++ b/src/mongo/db/update/update_driver.h
@@ -150,8 +150,6 @@ public:
         return _updateType;
     }
 
-    static bool isDocReplacement(const write_ops::UpdateModification& updateMod);
-
     bool modsAffectIndices() const {
         return _affectIndices;
     }
diff --git a/src/mongo/s/catalog/sharding_catalog_client_test.cpp b/src/mongo/s/catalog/sharding_catalog_client_test.cpp
index f2b831e24db..63dd880b63f 100644
--- a/src/mongo/s/catalog/sharding_catalog_client_test.cpp
+++ b/src/mongo/s/catalog/sharding_catalog_client_test.cpp
@@ -1084,7 +1084,7 @@ TEST_F(ShardingCatalogClientTest, UpdateDatabase) {
         ASSERT(update.getUpsert());
         ASSERT(!update.getMulti());
         ASSERT_BSONOBJ_EQ(update.getQ(), BSON(DatabaseType::name(dbt.getName())));
-        ASSERT_BSONOBJ_EQ(update.getU().getUpdateClassic(), dbt.toBSON());
+        ASSERT_BSONOBJ_EQ(update.getU().getUpdateReplacement(), dbt.toBSON());
 
         BatchedCommandResponse response;
         response.setStatus(Status::OK());
diff --git a/src/mongo/s/chunk_manager_targeter.cpp b/src/mongo/s/chunk_manager_targeter.cpp
index f87abb745a9..bdb386420c6 100644
--- a/src/mongo/s/chunk_manager_targeter.cpp
+++ b/src/mongo/s/chunk_manager_targeter.cpp
@@ -69,7 +69,7 @@ constexpr auto kIdFieldName = "_id"_sd;
 
 const ShardKeyPattern kVirtualIdShardKey(BSON(kIdFieldName << 1));
 
-using UpdateType = ChunkManagerTargeter::UpdateType;
+using UpdateType = write_ops::UpdateModification::Type;
 
 // Tracks the number of {multi:false} updates with an exact match on _id that are broadcasted to
 // multiple shards.
@@ -85,37 +85,33 @@ ServerStatusMetricField<Counter64> updateOneOpStyleBroadcastWithExactIDStats(
  *            or
  *          coll.update({x: 1}, [{$addFields: {y: 2}}])
  */
-UpdateType getUpdateExprType(const write_ops::UpdateOpEntry& updateDoc) {
+void validateUpdateDoc(const write_ops::UpdateOpEntry& updateDoc) {
     const auto& updateMod = updateDoc.getU();
     if (updateMod.type() == write_ops::UpdateModification::Type::kPipeline) {
-        return UpdateType::kOpStyle;
+        return;
     }
 
-    const auto& updateExpr = updateMod.getUpdateClassic();
-
-    // Empty update is replacement-style by default.
-    auto updateType = (updateExpr.isEmpty() ? UpdateType::kReplacement : UpdateType::kUnknown);
+    const auto updateType = updateMod.type();
+    invariant(updateType == UpdateType::kReplacement || updateType == UpdateType::kModifier);
+    const auto& updateExpr = updateType == UpdateType::kReplacement
+        ? updateMod.getUpdateReplacement()
+        : updateMod.getUpdateModifier();
 
     // Make sure that the update expression does not mix $op and non-$op fields.
     for (const auto& curField : updateExpr) {
-        const auto curFieldType =
-            (curField.fieldNameStringData()[0] == '$' ? UpdateType::kOpStyle
-                                                      : UpdateType::kReplacement);
-
-        // If the current field's type does not match the existing updateType, abort.
-        if (updateType == UpdateType::kUnknown)
-            updateType = curFieldType;
+        const auto updateTypeFromField = curField.fieldNameStringData()[0] != '$'
+            ? UpdateType::kReplacement
+            : UpdateType::kModifier;
 
         uassert(ErrorCodes::UnsupportedFormat,
                 str::stream() << "update document " << updateExpr
                               << " has mixed $operator and non-$operator style fields",
-                updateType == curFieldType);
+                updateType == updateTypeFromField);
     }
 
     uassert(ErrorCodes::InvalidOptions,
             "Replacement-style updates cannot be {multi:true}",
-            updateType == UpdateType::kOpStyle || !updateDoc.getMulti());
-    return updateType;
+            updateType == UpdateType::kModifier || !updateDoc.getMulti());
 }
 
 /**
@@ -128,25 +124,21 @@ UpdateType getUpdateExprType(const write_ops::UpdateOpEntry& updateDoc) {
  */
 BSONObj getUpdateExprForTargeting(const boost::intrusive_ptr<ExpressionContext> expCtx,
                                   const ShardKeyPattern& shardKeyPattern,
-                                  UpdateType updateType,
                                   const BSONObj& updateQuery,
                                   const write_ops::UpdateModification& updateMod) {
-    // We should never see an invalid update type here.
-    invariant(updateType != UpdateType::kUnknown);
-
     // If this is not a replacement update, then the update expression remains unchanged.
-    if (updateType != UpdateType::kReplacement) {
+    if (updateMod.type() != UpdateType::kReplacement) {
         BSONObjBuilder objBuilder;
         updateMod.serializeToBSON("u", &objBuilder);
         return objBuilder.obj();
     }
 
     // Extract the raw update expression from the request.
-    invariant(updateMod.type() == write_ops::UpdateModification::Type::kClassic);
+    invariant(updateMod.type() == UpdateType::kReplacement);
 
     // Replace any non-existent shard key values with a null value.
     auto updateExpr =
-        shardKeyPattern.emplaceMissingShardKeyValuesForDocument(updateMod.getUpdateClassic());
+        shardKeyPattern.emplaceMissingShardKeyValuesForDocument(updateMod.getUpdateReplacement());
 
     // If we aren't missing _id, return the update expression as-is.
     if (updateExpr.hasField(kIdFieldName)) {
@@ -443,9 +435,9 @@ std::vector<ShardEndpoint> ChunkManagerTargeter::targetUpdate(OperationContext*
         }
     }
 
-    const auto updateType = getUpdateExprType(updateOp);
+    validateUpdateDoc(updateOp);
     const auto updateExpr =
-        getUpdateExprForTargeting(expCtx, shardKeyPattern, updateType, query, updateOp.getU());
+        getUpdateExprForTargeting(expCtx, shardKeyPattern, query, updateOp.getU());
 
     // Utility function to target an update by shard key, and to handle any potential error results.
     auto targetByShardKey = [this, &collation](StatusWith<BSONObj> swShardKey, std::string msg) {
@@ -473,7 +465,7 @@ std::vector<ShardEndpoint> ChunkManagerTargeter::targetUpdate(OperationContext*
 
     // Replacement-style updates must always target a single shard. If we were unable to do so using
     // the query, we attempt to extract the shard key from the replacement and target based on it.
-    if (updateType == UpdateType::kReplacement) {
+    if (updateOp.getU().type() == write_ops::UpdateModification::Type::kReplacement) {
         return targetByShardKey(shardKeyPattern.extractShardKeyFromDoc(updateExpr),
                                 "Failed to target update by replacement document");
     }
diff --git a/src/mongo/s/chunk_manager_targeter.h b/src/mongo/s/chunk_manager_targeter.h
index 5bee40f821f..5dfb08dc878 100644
--- a/src/mongo/s/chunk_manager_targeter.h
+++ b/src/mongo/s/chunk_manager_targeter.h
@@ -58,7 +58,6 @@ using StaleShardVersionMap = std::map<ShardId, ChunkVersion>;
  */
 class ChunkManagerTargeter : public NSTargeter {
 public:
-    enum class UpdateType { kReplacement, kOpStyle, kUnknown };
     enum class LastErrorType { kCouldNotTarget, kStaleShardVersion, kStaleDbVersion };
     /**
      * Initializes the targeter with the latest routing information for the namespace, which means
diff --git a/src/mongo/s/sharding_router_test_fixture.cpp b/src/mongo/s/sharding_router_test_fixture.cpp
index 1597ca75e8e..f3348801a4e 100644
--- a/src/mongo/s/sharding_router_test_fixture.cpp
+++ b/src/mongo/s/sharding_router_test_fixture.cpp
@@ -322,7 +322,11 @@ void ShardingTestFixture::expectUpdateCollection(const HostAndPort& expectedHost
         ASSERT(!update.getMulti());
         ASSERT_BSONOBJ_EQ(BSON(CollectionType::kNssFieldName << coll.getNss().toString()),
                           update.getQ());
-        ASSERT_BSONOBJ_EQ(coll.toBSON(), update.getU().getUpdateClassic());
+        const auto& updateBSON =
+            update.getU().type() == write_ops::UpdateModification::Type::kReplacement
+            ? update.getU().getUpdateReplacement()
+            : update.getU().getUpdateModifier();
+        ASSERT_BSONOBJ_EQ(coll.toBSON(), updateBSON);
 
         BatchedCommandResponse response;
         response.setStatus(Status::OK());
diff --git a/src/mongo/shell/bench.cpp b/src/mongo/shell/bench.cpp
index c2614960c11..3624bbb69bb 100644
--- a/src/mongo/shell/bench.cpp
+++ b/src/mongo/shell/bench.cpp
@@ -1173,9 +1173,15 @@ void BenchRunOp::executeOnce(DBClientBase* conn,
                     BSONObjBuilder singleUpdate;
                     singleUpdate.append("q", query);
                     switch (this->update.type()) {
-                        case write_ops::UpdateModification::Type::kClassic: {
+                        case write_ops::UpdateModification::Type::kReplacement: {
                             singleUpdate.append("u",
-                                                fixQuery(this->update.getUpdateClassic(),
+                                                fixQuery(this->update.getUpdateReplacement(),
+                                                         *state->bsonTemplateEvaluator));
+                            break;
+                        }
+                        case write_ops::UpdateModification::Type::kModifier: {
+                            singleUpdate.append("u",
+                                                fixQuery(this->update.getUpdateModifier(),
                                                          *state->bsonTemplateEvaluator));
                             break;
                         }