feat: allow hinting the delete command

This change adds a new hint parameter to each statement of the delete command, which works just like hinting in update. Errors will now also be thrown if a hint is passed on older server versions which don't support them. NODE-2477
mongodb · Apr 15, 2020 · 95fedf4 · 95fedf4
1 parent e5aced7
commit 95fedf4
Show file tree

Hide file tree

Showing 46 changed files with 4,245 additions and 113 deletions.
diff --git a/lib/bulk/common.js b/lib/bulk/common.js
@@ -984,6 +984,9 @@ class BulkOperationBase {
     if (op.deleteOne || op.deleteMany) {
       const limit = op.deleteOne ? 1 : 0;
       const operation = { q: op[key].filter, limit: limit };
+      if (op[key].hint) {
+        operation.hint = op[key].hint;
+      }
       if (this.isOrdered) {
         if (op.collation) operation.collation = op.collation;
       }

diff --git a/lib/collection.js b/lib/collection.js
@@ -932,6 +932,7 @@ Collection.prototype.update = deprecate(function(selector, update, options, call
  * @param {boolean} [options.serializeFunctions=false] Serialize functions on any object.
  * @param {boolean} [options.ignoreUndefined=false] Specify if the BSON serializer should ignore undefined fields.
  * @param {ClientSession} [options.session] optional session to use for this operation
+ * @param {string|object} [options.hint] optional index hint for optimizing the filter query
  * @param {Collection~deleteWriteOpCallback} [callback] The command result callback
  * @returns {Promise} returns Promise if no callback passed
  */
@@ -966,6 +967,7 @@ Collection.prototype.removeOne = Collection.prototype.deleteOne;
  * @param {boolean} [options.serializeFunctions=false] Serialize functions on any object.
  * @param {boolean} [options.ignoreUndefined=false] Specify if the BSON serializer should ignore undefined fields.
  * @param {ClientSession} [options.session] optional session to use for this operation
+ * @param {string|object} [options.hint] optional index hint for optimizing the filter query
  * @param {Collection~deleteWriteOpCallback} [callback] The command result callback
  * @returns {Promise} returns Promise if no callback passed
  */

diff --git a/lib/operations/common_functions.js b/lib/operations/common_functions.js
@@ -299,6 +299,9 @@ function removeDocuments(coll, selector, options, callback) {
   } else if (finalOptions.retryWrites) {
     finalOptions.retryWrites = false;
   }
+  if (options.hint) {
+    op.hint = options.hint;
+  }
 
   // Have we specified collation
   try {

diff --git a/lib/sdam/server.js b/lib/sdam/server.js
@@ -448,6 +448,12 @@ function executeWriteOperation(args, options, callback) {
     callback(new MongoError(`server ${server.name} does not support collation`));
     return;
   }
+  if (maxWireVersion(server) < 5) {
+    if ((op === 'update' || op === 'remove') && ops.find(o => o.hint)) {
+      callback(new MongoError(`servers < 3.4 do not support hint on ${op}`));
+      return;
+    }
+  }
 
   server.s.pool.withConnection((err, conn, cb) => {
     if (err) {

diff --git a/test/functional/crud_spec.test.js b/test/functional/crud_spec.test.js
@@ -12,6 +12,19 @@ const TestRunnerContext = require('./spec-runner').TestRunnerContext;
 const gatherTestSuites = require('./spec-runner').gatherTestSuites;
 const generateTopologyTests = require('./spec-runner').generateTopologyTests;
 
+function enforceServerVersionLimits(requires, scenario) {
+  const versionLimits = [];
+  if (scenario.minServerVersion) {
+    versionLimits.push(`>=${scenario.minServerVersion}`);
+  }
+  if (scenario.maxServerVersion) {
+    versionLimits.push(`<=${scenario.maxServerVersion}`);
+  }
+  if (versionLimits.length) {
+    requires.mongodb = versionLimits.join(' ');
+  }
+}
+
 function findScenarios() {
   const route = [__dirname, '..', 'spec', 'crud'].concat(Array.from(arguments));
   return fs
@@ -53,9 +66,7 @@ describe('CRUD spec', function() {
         }
       };
 
-      if (scenario.minServerVersion) {
-        metadata.requires.mongodb = `>=${scenario.minServerVersion}`;
-      }
+      enforceServerVersionLimits(metadata.requires, scenario);
 
       describe(scenarioName, function() {
         scenario.tests.forEach(scenarioTest => {
@@ -83,9 +94,7 @@ describe('CRUD spec', function() {
         }
       };
 
-      if (scenario.minServerVersion) {
-        metadata.requires.mongodb = `>=${scenario.minServerVersion}`;
-      }
+      enforceServerVersionLimits(metadata.requires, scenario);
 
       describe(scenarioName, function() {
         beforeEach(() => testContext.db.dropDatabase());

diff --git a/test/functional/spec-runner/index.js b/test/functional/spec-runner/index.js
@@ -99,8 +99,10 @@ function generateTopologyTests(testSuites, testContext, filter) {
     let runOn = testSuite.runOn;
     if (!testSuite.runOn) {
       runOn = [{ minServerVersion: testSuite.minServerVersion }];
+      if (testSuite.maxServerVersion) {
+        runOn.push({ maxServerVersion: testSuite.maxServerVersion });
+      }
     }
-
     const environmentRequirementList = parseRunOn(runOn);
 
     environmentRequirementList.forEach(requires => {

diff --git a/test/spec/crud/v2/bulkWrite-arrayFilters.json b/test/spec/crud/v2/bulkWrite-arrayFilters.json
@@ -32,7 +32,7 @@
   "database_name": "crud-tests",
   "tests": [
     {
-      "description": "BulkWrite with arrayFilters",
+      "description": "BulkWrite updateOne with arrayFilters",
       "operations": [
         {
           "name": "bulkWrite",
@@ -53,7 +53,86 @@
                     }
                   ]
                 }
-              },
+              }
+            ],
+            "options": {
+              "ordered": true
+            }
+          },
+          "result": {
+            "deletedCount": 0,
+            "insertedCount": 0,
+            "insertedIds": {},
+            "matchedCount": 1,
+            "modifiedCount": 1,
+            "upsertedCount": 0,
+            "upsertedIds": {}
+          }
+        }
+      ],
+      "expectations": [
+        {
+          "command_started_event": {
+            "command": {
+              "update": "test",
+              "updates": [
+                {
+                  "q": {},
+                  "u": {
+                    "$set": {
+                      "y.$[i].b": 2
+                    }
+                  },
+                  "arrayFilters": [
+                    {
+                      "i.b": 3
+                    }
+                  ]
+                }
+              ],
+              "ordered": true
+            },
+            "command_name": "update",
+            "database_name": "crud-tests"
+          }
+        }
+      ],
+      "outcome": {
+        "collection": {
+          "data": [
+            {
+              "_id": 1,
+              "y": [
+                {
+                  "b": 2
+                },
+                {
+                  "b": 1
+                }
+              ]
+            },
+            {
+              "_id": 2,
+              "y": [
+                {
+                  "b": 0
+                },
+                {
+                  "b": 1
+                }
+              ]
+            }
+          ]
+        }
+      }
+    },
+    {
+      "description": "BulkWrite updateMany with arrayFilters",
+      "operations": [
+        {
+          "name": "bulkWrite",
+          "arguments": {
+            "requests": [
               {
                 "name": "updateMany",
                 "arguments": {
@@ -79,8 +158,8 @@
             "deletedCount": 0,
             "insertedCount": 0,
             "insertedIds": {},
-            "matchedCount": 3,
-            "modifiedCount": 3,
+            "matchedCount": 2,
+            "modifiedCount": 2,
             "upsertedCount": 0,
             "upsertedIds": {}
           }
@@ -92,19 +171,6 @@
             "command": {
               "update": "test",
               "updates": [
-                {
-                  "q": {},
-                  "u": {
-                    "$set": {
-                      "y.$[i].b": 2
-                    }
-                  },
-                  "arrayFilters": [
-                    {
-                      "i.b": 3
-                    }
-                  ]
-                },
                 {
                   "q": {},
                   "u": {
@@ -134,7 +200,7 @@
               "_id": 1,
               "y": [
                 {
-                  "b": 2
+                  "b": 3
                 },
                 {
                   "b": 2

diff --git a/test/spec/crud/v2/bulkWrite-arrayFilters.yml b/test/spec/crud/v2/bulkWrite-arrayFilters.yml
@@ -11,7 +11,7 @@ database_name: &database_name "crud-tests"
 
 tests:
   -
-    description: "BulkWrite with arrayFilters"
+    description: "BulkWrite updateOne with arrayFilters"
     operations:
       -
         name: "bulkWrite"
@@ -24,6 +24,44 @@ tests:
                 filter: {}
                 update: { $set: { "y.$[i].b": 2 } }
                 arrayFilters: [ { "i.b": 3 } ]
+          options: { ordered: true }
+        result:
+          deletedCount: 0
+          insertedCount: 0
+          insertedIds: {}
+          matchedCount: 1
+          modifiedCount: 1
+          upsertedCount: 0
+          upsertedIds: {}
+    expectations:
+      -
+        command_started_event:
+          command:
+            update: *collection_name
+            updates:
+              -
+                q: {}
+                u: { $set: { "y.$[i].b": 2 } }
+                arrayFilters: [ { "i.b": 3 } ]
+            ordered: true
+            # TODO: check that these fields do not exist once
+            # https://jira.mongodb.org/browse/SPEC-1215 has been resolved.
+            # writeConcern: null
+            # bypassDocumentValidation: null
+          command_name: update
+          database_name: *database_name
+    outcome:
+      collection:
+        data:
+          - {_id: 1, y: [{b: 2}, {b: 1}]}
+          - {_id: 2, y: [{b: 0}, {b: 1}]}
+  -
+    description: "BulkWrite updateMany with arrayFilters"
+    operations:
+      -
+        name: "bulkWrite"
+        arguments:
+          requests:
             -
               # UpdateMany when multiple documents match arrayFilters
               name: "updateMany"
@@ -36,8 +74,8 @@ tests:
           deletedCount: 0
           insertedCount: 0
           insertedIds: {}
-          matchedCount: 3
-          modifiedCount: 3
+          matchedCount: 2
+          modifiedCount: 2
           upsertedCount: 0
           upsertedIds: {}
     expectations:
@@ -46,10 +84,6 @@ tests:
           command:
             update: *collection_name
             updates:
-              -
-                q: {}
-                u: { $set: { "y.$[i].b": 2 } }
-                arrayFilters: [ { "i.b": 3 } ]
               -
                 q: {}
                 u: { $set: { "y.$[i].b": 2 } }
@@ -65,5 +99,5 @@ tests:
     outcome:
       collection:
         data:
-          - {_id: 1, y: [{b: 2}, {b: 2}]}
+          - {_id: 1, y: [{b: 3}, {b: 2}]}
           - {_id: 2, y: [{b: 0}, {b: 2}]}