SERVER-63201 Allow user deletes on capped collections

5 files changed, 26 insertions, 19 deletions

diff --git a/jstests/noPassthroughWithMongod/capped4.js b/jstests/noPassthroughWithMongod/capped4.js
index c659ff65935..f948614f24e 100644
--- a/jstests/noPassthroughWithMongod/capped4.js
+++ b/jstests/noPassthroughWithMongod/capped4.js
@@ -23,7 +23,6 @@ assert(!d.hasNext(), "C");
 assert(t.find().sort({i: 1}).hint({i: 1}).toArray().length > 10, "D");
 
 assert(t.findOne({i: i - 1}), "E");
-var res = assert.writeError(t.remove({i: i - 1}));
-assert(res.getWriteError().errmsg.indexOf("capped") >= 0, "F");
+assert(t.remove({i: i - 1}));
 
 assert(t.validate().valid, "G");
diff --git a/jstests/replsets/apply_ops_capped_collection.js b/jstests/replsets/apply_ops_capped_collection.js
index 2bd5d00f604..cfdd7ec6299 100644
--- a/jstests/replsets/apply_ops_capped_collection.js
+++ b/jstests/replsets/apply_ops_capped_collection.js
@@ -1,7 +1,12 @@
 /**
  * Tests the applyOps command on a capped collection that needs capped deletes.
+ * Tests that a delete applyOps command against a capped collection works.
  *
- * @tags: [requires_capped, requires_replication]
+ * @tags: [
+ *     multiversion_incompatible,
+ *     requires_capped,
+ *     requires_replication,
+ * ]
  */
 (function() {
 "use strict";
@@ -29,5 +34,9 @@ for (let i = 0; i < 20; i++) {
 
 assert.commandWorked(db.runCommand({applyOps: ops}));
 
+// Test that capped deletes via applyOps are permitted.
+assert.commandWorked(db.runCommand(
+    {applyOps: [{op: "d", ns: nss(dbName, collName), ts: Timestamp(20, 0), o: {_id: 19}}]}));
+
 rst.stopSet();
 }());
diff --git a/src/mongo/db/catalog/collection_impl.cpp b/src/mongo/db/catalog/collection_impl.cpp
index a96c82aa357..faad2b4ee34 100644
--- a/src/mongo/db/catalog/collection_impl.cpp
+++ b/src/mongo/db/catalog/collection_impl.cpp
@@ -1138,11 +1138,9 @@ void CollectionImpl::deleteDocument(OperationContext* opCtx,
                                     bool fromMigrate,
                                     bool noWarn,
                                     Collection::StoreDeletedDoc storeDeletedDoc) const {
-    if (isCapped() && opCtx->isEnforcingConstraints()) {
-        // System operations such as tenant migration or secondary batch application can delete from
-        // capped collections.
-        LOGV2(20291, "failing remove on a capped ns", "namespace"_attr = _ns);
-        uasserted(10089, "cannot remove from a capped collection");
+    if (isCapped() && opCtx->inMultiDocumentTransaction()) {
+        uasserted(ErrorCodes::IllegalOperation,
+                  "Cannot remove from a capped collection in a multi-document transaction");
     }
 
     std::vector<OplogSlot> oplogSlots;
diff --git a/src/mongo/db/query/get_executor.cpp b/src/mongo/db/query/get_executor.cpp
index 6021008c7d5..500722734d7 100644
--- a/src/mongo/db/query/get_executor.cpp
+++ b/src/mongo/db/query/get_executor.cpp
@@ -1327,11 +1327,16 @@ StatusWith<std::unique_ptr<PlanExecutor, PlanExecutor::Deleter>> getExecutorDele
         }
     }
 
-    if (collection && collection->isCapped() && opCtx->isEnforcingConstraints()) {
-        // System operations such as tenant migration or secondary batch application can delete
-        // from capped collections.
-        return Status(ErrorCodes::IllegalOperation,
-                      str::stream() << "cannot remove from a capped collection: " << nss.ns());
+    if (collection && collection->isCapped() && opCtx->inMultiDocumentTransaction()) {
+        // This check is duplicated from CollectionImpl::deleteDocument() for two reasons:
+        // - Performing a remove on an empty capped collection would not call
+        //   CollectionImpl::deleteDocument().
+        // - We can avoid doing lookups on documents and erroring later when trying to delete them.
+        return Status(
+            ErrorCodes::IllegalOperation,
+            str::stream()
+                << "Cannot remove from a capped collection in a multi-document transaction: "
+                << nss.ns());
     }
 
     bool userInitiatedWritesAndNotPrimary = opCtx->writesAreReplicated() &&
diff --git a/src/mongo/db/repl/storage_interface_impl_test.cpp b/src/mongo/db/repl/storage_interface_impl_test.cpp
index e65dd9dc484..e14a645fc70 100644
--- a/src/mongo/db/repl/storage_interface_impl_test.cpp
+++ b/src/mongo/db/repl/storage_interface_impl_test.cpp
@@ -2268,8 +2268,7 @@ TEST_F(StorageInterfaceImplTest, DeleteByFilterReturnsBadValueWhenFilterContains
     ASSERT_STRING_CONTAINS(status.reason(), "unknown operator: $unknownFilterOp");
 }
 
-TEST_F(StorageInterfaceImplTest, DeleteByFilterReturnsIllegalOperationOnCappedCollection) {
-    // User operations are not allowed to delete from capped collections.
+TEST_F(StorageInterfaceImplTest, DeleteByFilterOnCappedCollection) {
     transport::TransportLayerMock transportLayerMock;
     auto userClient = getOperationContext()->getServiceContext()->makeClient(
         "user", transportLayerMock.createSession());
@@ -2285,10 +2284,7 @@ TEST_F(StorageInterfaceImplTest, DeleteByFilterReturnsIllegalOperationOnCappedCo
     ASSERT_OK(storage.createCollection(opCtx.get(), nss, options));
 
     auto filter = BSON("x" << 1);
-    auto status = storage.deleteByFilter(opCtx.get(), nss, filter);
-    ASSERT_EQUALS(ErrorCodes::IllegalOperation, status);
-    ASSERT_STRING_CONTAINS(status.reason(),
-                           str::stream() << "cannot remove from a capped collection: " << nss.ns());
+    ASSERT_OK(storage.deleteByFilter(opCtx.get(), nss, filter));
 }
 
 TEST_F(