SERVER-17132 use snapshot id in DeleteStage, and re-check predicate

(cherry picked from commit 4b4c7bcc0880f079868209c58a2da86dad645c1b)

3 files changed, 19 insertions, 4 deletions

diff --git a/src/mongo/db/exec/delete.cpp b/src/mongo/db/exec/delete.cpp
index acfbedc97c0..177895dce50 100644
--- a/src/mongo/db/exec/delete.cpp
+++ b/src/mongo/db/exec/delete.cpp
@@ -36,6 +36,7 @@
 #include "mongo/db/exec/scoped_timer.h"
 #include "mongo/db/exec/working_set_common.h"
 #include "mongo/db/global_environment_experiment.h"
+#include "mongo/db/query/canonical_query.h"
 #include "mongo/db/repl/oplog.h"
 #include "mongo/db/repl/replication_coordinator_global.h"
 #include "mongo/util/log.h"
@@ -98,14 +99,22 @@ namespace mongo {
             }
             RecordId rloc = member->loc;
 
-            // If the working set member is in the owned obj with loc state, then the document may
-            // have already been deleted after-being force-fetched.
-            if (WorkingSetMember::LOC_AND_OWNED_OBJ == member->state) {
+            // If the snapshot changed, then we have to make sure we have the latest copy of the
+            // doc and that it still matches.
+            if (_txn->recoveryUnit()->getSnapshotId() != member->obj.snapshotId()) {
                 if (!_collection->findDoc(_txn, rloc, &member->obj)) {
                     // Doc is already deleted. Nothing more to do.
                     ++_commonStats.needTime;
                     return PlanStage::NEED_TIME;
                 }
+
+                // Make sure the re-fetched doc still matches the predicate.
+                if (_params.canonicalQuery &&
+                    !_params.canonicalQuery->root()->matchesBSON(member->obj.value(), NULL)) {
+                    // Doesn't match.
+                    ++_commonStats.needTime;
+                    return PlanStage::NEED_TIME;
+                }
             }
 
             _ws->free(id);
diff --git a/src/mongo/db/exec/delete.h b/src/mongo/db/exec/delete.h
index 9bdacf19354..6ac8f2adf42 100644
--- a/src/mongo/db/exec/delete.h
+++ b/src/mongo/db/exec/delete.h
@@ -35,6 +35,7 @@
 
 namespace mongo {
 
+    class CanonicalQuery;
     class OperationContext;
     class PlanExecutor;
 
@@ -43,7 +44,8 @@ namespace mongo {
             isMulti(false),
             shouldCallLogOp(false),
             fromMigrate(false),
-            isExplain(false) { }
+            isExplain(false),
+            canonicalQuery(NULL) { }
 
         // Should we delete all documents returned from the child (a "multi delete"), or at most one
         // (a "single delete")?
@@ -58,6 +60,9 @@ namespace mongo {
 
         // Are we explaining a delete command rather than actually executing it?
         bool isExplain;
+
+        // The parsed query predicate for this delete. Not owned here.
+        CanonicalQuery* canonicalQuery;
     };
 
     /**
diff --git a/src/mongo/db/query/get_executor.cpp b/src/mongo/db/query/get_executor.cpp
index 5248d78cfd9..a3cf395ade3 100644
--- a/src/mongo/db/query/get_executor.cpp
+++ b/src/mongo/db/query/get_executor.cpp
@@ -567,6 +567,7 @@ namespace mongo {
             return status;
         }
         invariant(root);
+        deleteStageParams.canonicalQuery = cq.get();
 
         root = new DeleteStage(txn, deleteStageParams, ws.get(), collection, root);
         // We must have a tree of stages in order to have a valid plan executor, but the query