SERVER-47801 Fix $meta:"textScore" for findAndModify

4 files changed, 167 insertions, 36 deletions

diff --git a/buildscripts/resmokeconfig/suites/retryable_writes_jscore_passthrough.yml b/buildscripts/resmokeconfig/suites/retryable_writes_jscore_passthrough.yml
index 40274895e5d..a72cafcf62f 100644
--- a/buildscripts/resmokeconfig/suites/retryable_writes_jscore_passthrough.yml
+++ b/buildscripts/resmokeconfig/suites/retryable_writes_jscore_passthrough.yml
@@ -36,6 +36,7 @@ selector:
   - jstests/core/find_and_modify2.js
   - jstests/core/find_and_modify_pipeline_update.js
   - jstests/core/find_and_modify_server6865.js
+  - jstests/core/fts_find_and_modify.js
 
   # This test makes the assumption that a command is run a certain number of times, but
   # the retryable writes suite overrides the runCommand to repeat commands.
diff --git a/buildscripts/resmokeconfig/suites/retryable_writes_jscore_stepdown_passthrough.yml b/buildscripts/resmokeconfig/suites/retryable_writes_jscore_stepdown_passthrough.yml
index fe733ac1e22..85f9152c252 100644
--- a/buildscripts/resmokeconfig/suites/retryable_writes_jscore_stepdown_passthrough.yml
+++ b/buildscripts/resmokeconfig/suites/retryable_writes_jscore_stepdown_passthrough.yml
@@ -29,6 +29,7 @@ selector:
   - jstests/core/find_and_modify.js
   - jstests/core/find_and_modify2.js
   - jstests/core/find_and_modify_server6865.js
+  - jstests/core/fts_find_and_modify.js
 
   # Stepdown commands during fsync lock will fail.
   - jstests/core/currentop.js
diff --git a/jstests/core/fts_find_and_modify.js b/jstests/core/fts_find_and_modify.js
new file mode 100644
index 00000000000..5fe0183fff7
--- /dev/null
+++ b/jstests/core/fts_find_and_modify.js
@@ -0,0 +1,122 @@
+/**
+ * Test that findAndModify works with $text search predicates.
+ *
+ * Cannot run when collections are implicitly sharded, since findAndModify requires the query
+ * predicate to contain the shard key.
+ * @tags: [assumes_unsharded_collection]
+ */
+(function() {
+"use strict";
+
+const coll = db.fts_find_and_modify;
+coll.drop();
+
+assert.commandWorked(coll.createIndex({numbers: "text"}));
+assert.commandWorked(coll.insert([
+    {_id: 1, numbers: "one"},
+    {_id: 2, numbers: "two"},
+    {_id: 3, numbers: "one two"},
+    {_id: 4, numbers: "four"}
+]));
+
+// Test that findAndModify can delete a document matching a text search predicate.
+assert.eq({_id: 4, numbers: "four"},
+          coll.findAndModify({query: {$text: {$search: "four"}}, remove: true}));
+assert.commandWorked(coll.insert({_id: 4, numbers: "four"}));
+
+// Test that findAndModify can update a document matching a text search predicate, and return the
+// old version of the document.
+assert.eq({_id: 4, numbers: "four"},
+          coll.findAndModify(
+              {query: {$text: {$search: "four"}}, update: [{$set: {addedField: 1}}], new: false}));
+assert.eq({_id: 4, numbers: "four", addedField: 1}, coll.findOne({_id: 4}));
+
+// Test that findAndModify can update a document matching a text search predicate, and return the
+// new version of the document.
+assert.eq({_id: 4, numbers: "four", addedField: 2}, coll.findAndModify({
+    query: {$text: {$search: "four"}},
+    update: [{$set: {addedField: {$add: ["$addedField", 1]}}}],
+    new: true
+}));
+assert.eq({_id: 4, numbers: "four", addedField: 2}, coll.findOne({_id: 4}));
+
+// Test that findAndModify can delete a document and project its text score.
+assert.eq(
+    {_id: 4, numbers: "four", addedField: 2, score: 1.1},
+    coll.findAndModify(
+        {query: {$text: {$search: "four"}}, fields: {score: {$meta: "textScore"}}, remove: true}));
+assert.commandWorked(coll.insert({_id: 4, numbers: "four"}));
+
+// Test that findAndModify can delete a document, where the document is chosen sorting by text
+// score.
+assert.eq(
+    {_id: 3, numbers: "one two"},
+    coll.findAndModify(
+        {query: {$text: {$search: "one two"}}, sort: {score: {$meta: "textScore"}}, remove: true}));
+assert.commandWorked(coll.insert({_id: 3, numbers: "one two"}));
+
+// Test that findAndModify can delete a document and both sort and project the text score.
+assert.eq({_id: 3, numbers: "one two", score: 1.5}, coll.findAndModify({
+    query: {$text: {$search: "one two"}},
+    fields: {score: {$meta: "textScore"}},
+    sort: {score: {$meta: "textScore"}},
+    remove: true
+}));
+assert.commandWorked(coll.insert({_id: 3, numbers: "one two"}));
+
+// Test that findAndModify can update a document, returning the old document with the text score
+// projected.
+assert.eq({_id: 4, numbers: "four", score: 1.1}, coll.findAndModify({
+    query: {$text: {$search: "four"}},
+    update: [{$set: {addedField: 1}}],
+    fields: {score: {$meta: "textScore"}},
+    new: false
+}));
+
+// Test that findAndModify can update a document, returning the new document with the text score
+// projected.
+assert.eq({_id: 4, numbers: "four", addedField: 2, score: 1.1}, coll.findAndModify({
+    query: {$text: {$search: "four"}},
+    update: [{$set: {addedField: {$add: ["$addedField", 1]}}}],
+    fields: {score: {$meta: "textScore"}},
+    new: true
+}));
+
+// Test that findAndModify can update a document chosen by a "textScore" $meta-sort, and return the
+// old version of the document.
+assert.eq({_id: 3, numbers: "one two"}, coll.findAndModify({
+    query: {$text: {$search: "one two"}},
+    update: [{$set: {addedField: 1}}],
+    sort: {score: {$meta: "textScore"}},
+    new: false
+}));
+
+// Test that findAndModify can update a document chosen by a "textScore" $meta-sort, and return the
+// new version of the document.
+assert.eq({_id: 3, numbers: "one two", addedField: 2}, coll.findAndModify({
+    query: {$text: {$search: "one two"}},
+    update: [{$set: {addedField: {$add: ["$addedField", 1]}}}],
+    sort: {score: {$meta: "textScore"}},
+    new: true
+}));
+
+// Test that findAndModify can update a document chosen by a "textScore" $meta-sort, and return the
+// old version of the document with the text score projected.
+assert.eq({_id: 3, numbers: "one two", addedField: 2, score: 1.5}, coll.findAndModify({
+    query: {$text: {$search: "one two"}},
+    update: [{$set: {addedField: {$add: ["$addedField", 1]}}}],
+    fields: {score: {$meta: "textScore"}},
+    sort: {score: {$meta: "textScore"}},
+    new: false
+}));
+
+// Test that findAndModify can update a document chosen by a "textScore" $meta-sort, and return the
+// new version of the document with the text score projected.
+assert.eq({_id: 3, numbers: "one two", addedField: 4, score: 1.5}, coll.findAndModify({
+    query: {$text: {$search: "one two"}},
+    update: [{$set: {addedField: {$add: ["$addedField", 1]}}}],
+    fields: {score: {$meta: "textScore"}},
+    sort: {score: {$meta: "textScore"}},
+    new: true
+}));
+}());
diff --git a/src/mongo/db/query/get_executor.cpp b/src/mongo/db/query/get_executor.cpp
index 4dd0b5d6225..dbbecb419b5 100644
--- a/src/mongo/db/query/get_executor.cpp
+++ b/src/mongo/db/query/get_executor.cpp
@@ -686,19 +686,17 @@ StatusWith<std::unique_ptr<PlanExecutor, PlanExecutor::Deleter>> getExecutorLega
 namespace {
 
 /**
- * Wrap the specified 'root' plan stage in a ProjectionStage. Does not take ownership of any
- * arguments other than root.
+ * Attempts to construct and return the projection AST corresponding to 'projObj'. Illegal to call
+ * if 'projObj' is empty.
  *
- * If the projection was valid, then return Status::OK() with a pointer to the newly created
- * ProjectionStage. Otherwise, return a status indicating the error reason.
+ * If 'allowPositional' is false, and the projection AST involves positional projection, returns a
+ * non-OK status.
+ *
+ * Marks any metadata dependencies required by the projection on the given CanonicalQuery.
  */
-StatusWith<unique_ptr<PlanStage>> applyProjection(OperationContext* opCtx,
-                                                  const NamespaceString& nsString,
-                                                  CanonicalQuery* cq,
-                                                  const BSONObj& projObj,
-                                                  bool allowPositional,
-                                                  WorkingSet* ws,
-                                                  unique_ptr<PlanStage> root) {
+StatusWith<std::unique_ptr<projection_ast::Projection>> makeProjection(const BSONObj& projObj,
+                                                                       bool allowPositional,
+                                                                       CanonicalQuery* cq) {
     invariant(!projObj.isEmpty());
 
     projection_ast::Projection proj =
@@ -724,8 +722,7 @@ StatusWith<unique_ptr<PlanStage>> applyProjection(OperationContext* opCtx,
                 "Cannot use a $meta sortKey projection in findAndModify commands."};
     }
 
-    return {std::make_unique<ProjectionStageDefault>(
-        cq->getExpCtx(), projObj, &proj, ws, std::unique_ptr<PlanStage>(root.release()))};
+    return std::make_unique<projection_ast::Projection>(proj);
 }
 
 }  // namespace
@@ -844,6 +841,18 @@ StatusWith<unique_ptr<PlanExecutor, PlanExecutor::Deleter>> getExecutorDelete(
     // Transfer the explain verbosity level into the expression context.
     cq->getExpCtx()->explain = verbosity;
 
+    std::unique_ptr<projection_ast::Projection> projection;
+    if (!request->getProj().isEmpty()) {
+        invariant(request->getReturnDeleted());
+
+        const bool allowPositional = true;
+        auto projectionWithStatus = makeProjection(request->getProj(), allowPositional, cq.get());
+        if (!projectionWithStatus.isOK()) {
+            return projectionWithStatus.getStatus();
+        }
+        projection = std::move(projectionWithStatus.getValue());
+    }
+
     // The underlying query plan must preserve the record id, since it will be needed in order to
     // identify the record to update.
     const size_t defaultPlannerOptions = QueryPlannerParams::PRESERVE_RECORD_ID;
@@ -863,16 +872,9 @@ StatusWith<unique_ptr<PlanExecutor, PlanExecutor::Deleter>> getExecutorDelete(
     root = std::make_unique<DeleteStage>(
         cq->getExpCtx().get(), std::move(deleteStageParams), ws.get(), collection, root.release());
 
-    if (!request->getProj().isEmpty()) {
-        invariant(request->getReturnDeleted());
-
-        const bool allowPositional = true;
-        StatusWith<unique_ptr<PlanStage>> projStatus = applyProjection(
-            opCtx, nss, cq.get(), request->getProj(), allowPositional, ws.get(), std::move(root));
-        if (!projStatus.isOK()) {
-            return projStatus.getStatus();
-        }
-        root = std::move(projStatus.getValue());
+    if (projection) {
+        root = std::make_unique<ProjectionStageDefault>(
+            cq->getExpCtx(), request->getProj(), projection.get(), ws.get(), std::move(root));
     }
 
     // We must have a tree of stages in order to have a valid plan executor, but the query
@@ -1003,6 +1005,21 @@ StatusWith<unique_ptr<PlanExecutor, PlanExecutor::Deleter>> getExecutorUpdate(
     // Transfer the explain verbosity level into the expression context.
     cq->getExpCtx()->explain = verbosity;
 
+    std::unique_ptr<projection_ast::Projection> projection;
+    if (!request->getProj().isEmpty()) {
+        invariant(request->shouldReturnAnyDocs());
+
+        // If the plan stage is to return the newly-updated version of the documents, then it
+        // is invalid to use a positional projection because the query expression need not
+        // match the array element after the update has been applied.
+        const bool allowPositional = request->shouldReturnOldDocs();
+        auto projectionWithStatus = makeProjection(request->getProj(), allowPositional, cq.get());
+        if (!projectionWithStatus.isOK()) {
+            return projectionWithStatus.getStatus();
+        }
+        projection = std::move(projectionWithStatus.getValue());
+    }
+
     // The underlying query plan must preserve the record id, since it will be needed in order to
     // identify the record to update.
     const size_t defaultPlannerOptions = QueryPlannerParams::PRESERVE_RECORD_ID;
@@ -1027,19 +1044,9 @@ StatusWith<unique_ptr<PlanExecutor, PlanExecutor::Deleter>> getExecutorUpdate(
              : std::make_unique<UpdateStage>(
                    cq->getExpCtx().get(), updateStageParams, ws.get(), collection, root.release()));
 
-    if (!request->getProj().isEmpty()) {
-        invariant(request->shouldReturnAnyDocs());
-
-        // If the plan stage is to return the newly-updated version of the documents, then it
-        // is invalid to use a positional projection because the query expression need not
-        // match the array element after the update has been applied.
-        const bool allowPositional = request->shouldReturnOldDocs();
-        StatusWith<unique_ptr<PlanStage>> projStatus = applyProjection(
-            opCtx, nss, cq.get(), request->getProj(), allowPositional, ws.get(), std::move(root));
-        if (!projStatus.isOK()) {
-            return projStatus.getStatus();
-        }
-        root = std::move(projStatus.getValue());
+    if (projection) {
+        root = std::make_unique<ProjectionStageDefault>(
+            cq->getExpCtx(), request->getProj(), projection.get(), ws.get(), std::move(root));
     }
 
     // We must have a tree of stages in order to have a valid plan executor, but the query