SERVER-16063 Rewrite the findAndModify command.

Changed UpdateStage to return the prior or newly-updated version of
a document if request. also changed DeleteStage to return the deleted
document if requested.

Added explain support to the findAndModify command.

9 files changed, 134 insertions, 27 deletions

diff --git a/src/mongo/db/ops/delete.h b/src/mongo/db/ops/delete.h
index 70957d988d0..563734428ab 100644
--- a/src/mongo/db/ops/delete.h
+++ b/src/mongo/db/ops/delete.h
@@ -39,7 +39,6 @@ namespace mongo {
     class Database;
     class OperationContext;
 
-    // If justOne is true, deletedId is set to the id of the deleted object.
     long long deleteObjects(OperationContext* txn,
                             Database* db,
                             StringData ns,
diff --git a/src/mongo/db/ops/delete_request.h b/src/mongo/db/ops/delete_request.h
index 2d2a3f46a0b..27ea4f2bbb9 100644
--- a/src/mongo/db/ops/delete_request.h
+++ b/src/mongo/db/ops/delete_request.h
@@ -45,21 +45,28 @@ namespace mongo {
             _god(false),
             _fromMigrate(false),
             _isExplain(false),
+            _returnDeleted(false),
             _yieldPolicy(PlanExecutor::YIELD_MANUAL) {}
 
         void setQuery(const BSONObj& query) { _query = query; }
+        void setProj(const BSONObj& proj) { _proj = proj; }
+        void setSort(const BSONObj& sort) { _sort = sort; }
         void setMulti(bool multi = true) { _multi = multi; }
         void setGod(bool god = true) { _god = god; }
         void setFromMigrate(bool fromMigrate = true) { _fromMigrate = fromMigrate; }
         void setExplain(bool isExplain = true) { _isExplain = isExplain; }
+        void setReturnDeleted(bool returnDeleted = true) { _returnDeleted = returnDeleted; }
         void setYieldPolicy(PlanExecutor::YieldPolicy yieldPolicy) { _yieldPolicy = yieldPolicy; }
 
         const NamespaceString& getNamespaceString() const { return _nsString; }
         const BSONObj& getQuery() const { return _query; }
+        const BSONObj& getProj() const { return _proj; }
+        const BSONObj& getSort() const { return _sort; }
         bool isMulti() const { return _multi; }
         bool isGod() const { return _god; }
         bool isFromMigrate() const { return _fromMigrate; }
         bool isExplain() const { return _isExplain; }
+        bool shouldReturnDeleted() const { return _returnDeleted; }
         PlanExecutor::YieldPolicy getYieldPolicy() const { return _yieldPolicy; }
 
         std::string toString() const;
@@ -67,10 +74,13 @@ namespace mongo {
     private:
         const NamespaceString& _nsString;
         BSONObj _query;
+        BSONObj _proj;
+        BSONObj _sort;
         bool _multi;
         bool _god;
         bool _fromMigrate;
         bool _isExplain;
+        bool _returnDeleted;
         PlanExecutor::YieldPolicy _yieldPolicy;
     };
 
diff --git a/src/mongo/db/ops/parsed_delete.cpp b/src/mongo/db/ops/parsed_delete.cpp
index 64a67c50f80..62243d0818d 100644
--- a/src/mongo/db/ops/parsed_delete.cpp
+++ b/src/mongo/db/ops/parsed_delete.cpp
@@ -52,6 +52,13 @@ namespace mongo {
 
     Status ParsedDelete::parseRequest() {
         dassert(!_canonicalQuery.get());
+        // It is invalid to request that the DeleteStage return the deleted document during a
+        // multi-remove.
+        invariant(!(_request->shouldReturnDeleted() && _request->isMulti()));
+
+        // It is invalid to request that a ProjectionStage be applied to the DeleteStage if the
+        // DeleteStage would not return the deleted document.
+        invariant(_request->getProj().isEmpty() || _request->shouldReturnDeleted());
 
         if (CanonicalQuery::isSimpleIdQuery(_request->getQuery())) {
             return Status::OK();
@@ -66,8 +73,27 @@ namespace mongo {
         CanonicalQuery* cqRaw;
         const WhereCallbackReal whereCallback(_txn, _request->getNamespaceString().db());
 
+        // Limit should only used for the findAndModify command when a sort is specified. If a sort
+        // is requested, we want to use a top-k sort for efficiency reasons, so should pass the
+        // limit through. Generally, a delete stage expects to be able to skip documents that were
+        // deleted out from under it, but a limit could inhibit that and give an EOF when the delete
+        // has not actually deleted a document. This behavior is fine for findAndModify, but should
+        // not apply to deletes in general.
+        long long limit = (!_request->isMulti() && !_request->getSort().isEmpty()) ? -1 : 0;
+
+        // The projection needs to be applied after the delete operation, so we specify an empty
+        // BSONObj as the projection during canonicalization.
+        const BSONObj emptyObj;
         Status status = CanonicalQuery::canonicalize(_request->getNamespaceString().ns(),
                                                      _request->getQuery(),
+                                                     _request->getSort(),
+                                                     emptyObj, // projection
+                                                     0, // skip
+                                                     limit,
+                                                     emptyObj, // hint
+                                                     emptyObj, // min
+                                                     emptyObj, // max
+                                                     false, // snapshot
                                                      _request->isExplain(),
                                                      &cqRaw,
                                                      whereCallback);
diff --git a/src/mongo/db/ops/parsed_delete.h b/src/mongo/db/ops/parsed_delete.h
index cb189bfccc8..ad473d35e91 100644
--- a/src/mongo/db/ops/parsed_delete.h
+++ b/src/mongo/db/ops/parsed_delete.h
@@ -44,6 +44,10 @@ namespace mongo {
      * via the parseRequest() method. A ParsedDelete can then be used to retrieve a PlanExecutor
      * capable of executing the delete.
      *
+     * It is invalid to request that the DeleteStage return the deleted document during a
+     * multi-remove. It is also invalid to request that a ProjectionStage be applied to the
+     * DeleteStage if the DeleteStage would not return the deleted document.
+     *
      * A delete request is parsed to a CanonicalQuery, so this class is a thin, delete-specific
      * wrapper around canonicalization.
      *
diff --git a/src/mongo/db/ops/parsed_update.cpp b/src/mongo/db/ops/parsed_update.cpp
index 9e084b2dfe5..d4651371e3f 100644
--- a/src/mongo/db/ops/parsed_update.cpp
+++ b/src/mongo/db/ops/parsed_update.cpp
@@ -42,9 +42,13 @@ namespace mongo {
         _canonicalQuery() { }
 
     Status ParsedUpdate::parseRequest() {
-        // It is invalid to request that the update plan stores a copy of the resulting document
-        // if it is a multi-update.
-        invariant(!(_request->shouldStoreResultDoc() && _request->isMulti()));
+        // It is invalid to request that the UpdateStage return the prior or newly-updated version
+        // of a document during a multi-update.
+        invariant(!(_request->shouldReturnAnyDocs() && _request->isMulti()));
+
+        // It is invalid to request that a ProjectionStage be applied to the UpdateStage if the
+        // UpdateStage would not return any document.
+        invariant(_request->getProj().isEmpty() || _request->shouldReturnAnyDocs());
 
         // We parse the update portion before the query portion because the dispostion of the update
         // may determine whether or not we need to produce a CanonicalQuery at all.  For example, if
@@ -75,8 +79,27 @@ namespace mongo {
         CanonicalQuery* cqRaw;
         const WhereCallbackReal whereCallback(_txn, _request->getNamespaceString().db());
 
+        // Limit should only used for the findAndModify command when a sort is specified. If a sort
+        // is requested, we want to use a top-k sort for efficiency reasons, so should pass the
+        // limit through. Generally, a update stage expects to be able to skip documents that were
+        // deleted/modified under it, but a limit could inhibit that and give an EOF when the update
+        // has not actually updated a document. This behavior is fine for findAndModify, but should
+        // not apply to update in general.
+        long long limit = (!_request->isMulti() && !_request->getSort().isEmpty()) ? -1 : 0;
+
+        // The projection needs to be applied after the update operation, so we specify an empty
+        // BSONObj as the projection during canonicalization.
+        const BSONObj emptyObj;
         Status status = CanonicalQuery::canonicalize(_request->getNamespaceString().ns(),
                                                      _request->getQuery(),
+                                                     _request->getSort(),
+                                                     emptyObj, // projection
+                                                     0, // skip
+                                                     limit,
+                                                     emptyObj, // hint
+                                                     emptyObj, // min
+                                                     emptyObj, // max
+                                                     false, // snapshot
                                                      _request->isExplain(),
                                                      &cqRaw,
                                                      whereCallback);
diff --git a/src/mongo/db/ops/parsed_update.h b/src/mongo/db/ops/parsed_update.h
index 5a7854f93fa..98472eba377 100644
--- a/src/mongo/db/ops/parsed_update.h
+++ b/src/mongo/db/ops/parsed_update.h
@@ -43,6 +43,10 @@ namespace mongo {
      * via the parseRequest() method. A ParsedUpdate can then be used to retrieve a PlanExecutor
      * capable of executing the update.
      *
+     * It is invalid to request that the UpdateStage return the prior or newly-updated version of a
+     * document during a multi-update. It is also invalid to request that a ProjectionStage be
+     * applied to the UpdateStage if the UpdateStage would not return any document.
+     *
      * No locks need to be held during parsing.
      *
      * The query part of the update is parsed to a CanonicalQuery, and the update part is parsed
diff --git a/src/mongo/db/ops/update_request.h b/src/mongo/db/ops/update_request.h
index 9f69fdc3142..d300a8fc4f2 100644
--- a/src/mongo/db/ops/update_request.h
+++ b/src/mongo/db/ops/update_request.h
@@ -43,6 +43,17 @@ namespace mongo {
 
     class UpdateRequest {
     public:
+        enum ReturnDocOption {
+            // Return no document.
+            RETURN_NONE,
+
+            // Return the document as it was before the update. If the update results in an insert,
+            // no document will be returned.
+            RETURN_OLD,
+
+            // Return the document as it is after the update.
+            RETURN_NEW
+        };
         inline UpdateRequest(const NamespaceString& nsString)
             : _nsString(nsString)
             , _god(false)
@@ -51,7 +62,7 @@ namespace mongo {
             , _fromMigration(false)
             , _lifecycle(NULL)
             , _isExplain(false)
-            , _storeResultDoc(false)
+            , _returnDocs(ReturnDocOption::RETURN_NONE)
             , _yieldPolicy(PlanExecutor::YIELD_MANUAL) {}
 
         const NamespaceString& getNamespaceString() const {
@@ -66,6 +77,22 @@ namespace mongo {
             return _query;
         }
 
+        inline void setProj(const BSONObj& proj) {
+            _proj = proj;
+        }
+
+        inline const BSONObj& getProj() const {
+            return _proj;
+        }
+
+        inline void setSort(const BSONObj& sort) {
+            _sort = sort;
+        }
+
+        inline const BSONObj& getSort() const {
+            return _sort;
+        }
+
         inline void setUpdates(const BSONObj& updates) {
             _updates = updates;
         }
@@ -125,12 +152,20 @@ namespace mongo {
             return _isExplain;
         }
 
-        inline void setStoreResultDoc(bool value = true) {
-            _storeResultDoc = value;
+        inline void setReturnDocs(ReturnDocOption value) {
+            _returnDocs = value;
+        }
+
+        inline bool shouldReturnOldDocs() const {
+            return _returnDocs == ReturnDocOption::RETURN_OLD;
+        }
+
+        inline bool shouldReturnNewDocs() const {
+            return _returnDocs == ReturnDocOption::RETURN_NEW;
         }
 
-        inline bool shouldStoreResultDoc() const {
-            return _storeResultDoc;
+        inline bool shouldReturnAnyDocs() const {
+            return shouldReturnOldDocs() || shouldReturnNewDocs();
         }
 
         inline void setYieldPolicy(PlanExecutor::YieldPolicy yieldPolicy) {
@@ -144,6 +179,8 @@ namespace mongo {
         const std::string toString() const {
             return str::stream()
                         << " query: " << _query
+                        << " projection: " << _proj
+                        << " sort: " << _sort
                         << " updated: " << _updates
                         << " god: " << _god
                         << " upsert: " << _upsert
@@ -158,6 +195,12 @@ namespace mongo {
         // Contains the query that selects documents to update.
         BSONObj _query;
 
+        // Contains the projection information.
+        BSONObj _proj;
+
+        // Contains the sort order information.
+        BSONObj _sort;
+
         // Contains the modifiers to apply to matched objects, or a replacement document.
         BSONObj _updates;
 
@@ -182,13 +225,19 @@ namespace mongo {
         // Whether or not we are requesting an explained update. Explained updates are read-only.
         bool _isExplain;
 
-        // Whether or not we keep an owned copy of the resulting document for a non-multi update.
-        // This allows someone executing an update to retrieve the resulting document without
-        // another query once the update is complete.
+        // Specifies which version of the documents to return, if any.
+        //
+        //   RETURN_NONE (default): Never return any documents, old or new.
+        //   RETURN_OLD: Return ADVANCED when a matching document is encountered, and the value of
+        //               the document before it was updated. If there were no matches, return
+        //               IS_EOF instead (even in case of an upsert).
+        //   RETURN_NEW: Return ADVANCED when a matching document is encountered, and the value of
+        //               the document after being updated. If an upsert was specified and it
+        //               resulted in an insert, return the inserted document.
         //
-        // It is illegal to use this flag in combination with the '_multi' flag, and doing so will
-        // trigger an invariant check.
-        bool _storeResultDoc;
+        // This allows findAndModify to execute an update and retrieve the resulting document
+        // without another query before or after the update.
+        ReturnDocOption _returnDocs;
 
         // Whether or not the update should yield. Defaults to YIELD_MANUAL.
         PlanExecutor::YieldPolicy _yieldPolicy;
diff --git a/src/mongo/db/ops/update_result.cpp b/src/mongo/db/ops/update_result.cpp
index d2664d34e38..f5e31c1a3d8 100644
--- a/src/mongo/db/ops/update_result.cpp
+++ b/src/mongo/db/ops/update_result.cpp
@@ -42,13 +42,11 @@ namespace mongo {
                                bool modifiers_,
                                unsigned long long numDocsModified_,
                                unsigned long long numMatched_,
-                               const BSONObj& upsertedObject_,
-                               const BSONObj& newObj_)
+                               const BSONObj& upsertedObject_)
         : existing(existing_),
           modifiers(modifiers_),
           numDocsModified(numDocsModified_),
-          numMatched(numMatched_),
-          newObj(newObj_) {
+          numMatched(numMatched_) {
 
         BSONElement id = upsertedObject_["_id"];
         if ( ! existing && numMatched == 1 && !id.eoo() ) {
diff --git a/src/mongo/db/ops/update_result.h b/src/mongo/db/ops/update_result.h
index 250b9d1fe23..568da353554 100644
--- a/src/mongo/db/ops/update_result.h
+++ b/src/mongo/db/ops/update_result.h
@@ -43,8 +43,7 @@ namespace mongo {
                       bool modifiers_,
                       unsigned long long numDocsModified_,
                       unsigned long long numMatched_,
-                      const BSONObj& upsertedObject_,
-                      const BSONObj& newObj_ );
+                      const BSONObj& upsertedObject_);
 
 
         // if existing objects were modified
@@ -62,18 +61,13 @@ namespace mongo {
         // if something was upserted, the new _id of the object
         BSONObj upserted;
 
-        // For a non-multi update, the new version of the document. If we did an insert, this
-        // is the full document that got inserted (whereas 'upserted' is just the _id field).
-        BSONObj newObj;
-
         const std::string toString() const {
             return str::stream()
                         << " upserted: " << upserted
                         << " modifiers: " << modifiers
                         << " existing: " << existing
                         << " numDocsModified: " << numDocsModified
-                        << " numMatched: " << numMatched
-                        << " newObj: " << newObj;
+                        << " numMatched: " << numMatched;
         }
     };