summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSanika Phanse <sanika.phanse@mongodb.com>2023-03-30 12:47:36 +0000
committerEvergreen Agent <no-reply@evergreen.mongodb.com>2023-03-30 13:48:17 +0000
commitcf860fd6504eb80ea47905dab3b8b4c407a4b9f8 (patch)
tree542fca6b31754c5fe839ece796e66f5091186f34
parent222dde5f2593af7c0e4175696d2fdd6ca54e24cd (diff)
downloadmongo-cf860fd6504eb80ea47905dab3b8b4c407a4b9f8.tar.gz
SERVER-74952 Insert upsert document for single writes without shard keys
-rw-r--r--jstests/sharding/extract_shard_key_values.js47
-rw-r--r--jstests/sharding/mongos_validate_writes.js27
-rw-r--r--jstests/sharding/query/collation_targeting.js6
-rw-r--r--jstests/sharding/query/collation_targeting_inherited.js5
-rw-r--r--jstests/sharding/query/explain_cmd.js27
-rw-r--r--jstests/sharding/regex_targeting.js53
-rw-r--r--jstests/sharding/shard6.js14
-rw-r--r--jstests/sharding/updateOne_without_shard_key/find_and_modify_without_shard_key.js74
-rw-r--r--jstests/sharding/updateOne_without_shard_key/libs/write_without_shard_key_test_util.js5
-rw-r--r--jstests/sharding/updateOne_without_shard_key/updateOne_without_shard_key_basic.js32
-rw-r--r--jstests/sharding/update_compound_shard_key.js31
-rw-r--r--jstests/sharding/update_replace_id.js14
-rw-r--r--jstests/sharding/update_sharded.js24
-rw-r--r--jstests/sharding/upsert_sharded.js149
-rw-r--r--src/mongo/s/collection_routing_info_targeter.cpp11
-rw-r--r--src/mongo/s/commands/cluster_query_without_shard_key_cmd.cpp2
-rw-r--r--src/mongo/s/write_ops/batch_write_exec.cpp32
-rw-r--r--src/mongo/s/write_ops/write_without_shard_key_util.cpp100
18 files changed, 456 insertions, 197 deletions
diff --git a/jstests/sharding/extract_shard_key_values.js b/jstests/sharding/extract_shard_key_values.js
index 15f9c936658..d0523b3b300 100644
--- a/jstests/sharding/extract_shard_key_values.js
+++ b/jstests/sharding/extract_shard_key_values.js
@@ -159,41 +159,44 @@ assert.commandWorked(mongos.getCollection(kNsName).insert({a: -100, b: 1, c: 1,
assert.commandWorked(mongos.getCollection(kNsName).insert({a: 0, b: 1, c: 2, d: 1}));
assert.commandWorked(mongos.getCollection(kNsName).insert({a: 100, b: 1, c: 3, d: 1}));
-// Verify we cannot update a shard key value via a query that's missing the shard key, despite being
-// able to target using the replacement document.
-
// Need to start a session to change the shard key.
const session = st.s.startSession({retryWrites: true});
const sessionDB = session.getDatabase(kDbName);
const sessionColl = sessionDB[kCollName];
-// Sharded updateOnes that do not directly target a shard can now use the two phase write
-// protocol to execute.
if (WriteWithoutShardKeyTestUtil.isWriteWithoutShardKeyFeatureEnabled(st.s)) {
assert.commandWorked(sessionColl.update({d: 1}, {b: 1, c: 4, d: 1}));
-} else {
- assert.commandFailedWithCode(sessionColl.update({d: 1}, {b: 1, c: 4, d: 1}), 31025);
-}
-// Verify that an upsert targets shards without treating missing shard key fields as null values.
-// This implies that upsert still requires the entire shard key to be specified in the query.
-assert.writeErrorWithCode(
- mongos.getCollection(kNsName).update({b: 1}, {$set: {c: 2}}, {upsert: true}),
- ErrorCodes.ShardKeyNotFound);
+ let res = assert.commandWorked(
+ mongos.getCollection(kNsName).update({b: 2}, {$set: {c: 2}}, {upsert: true}));
+ assert.eq(0, res.nMatched);
+ assert.eq(1, res.nUpserted);
+ docsArr = mongos.getCollection(kNsName).find({b: 2}).toArray();
+ assert.eq(1, docsArr.length);
-// Sharded findAndModify that do not directly target a shard can now use the two phase write
-// protocol to execute.
-if (WriteWithoutShardKeyTestUtil.isWriteWithoutShardKeyFeatureEnabled(st.s)) {
assert.commandWorked(sessionColl.insert({_id: "findAndModify", a: 1}));
- let res = assert.commandWorked(sessionDB.runCommand(
- {findAndModify: kCollName, query: {a: 1}, update: {$set: {updated: true}}}));
+ res = assert.commandWorked(sessionDB.runCommand(
+ {findAndModify: kCollName, query: {a: 2}, update: {$set: {updated: true}}, upsert: true}));
assert.eq(1, res.lastErrorObject.n);
+ assert.eq(0, res.lastErrorObject.updatedExisting);
+ assert(res.lastErrorObject.upserted);
+ docsArr = mongos.getCollection(kNsName).find({a: 2}).toArray();
+ assert.eq(1, docsArr.length);
} else {
- assert.commandWorked(sessionColl.insert({_id: "findAndModify", a: 1}));
- assert.commandFailedWithCode(
- sessionDB.runCommand(
- {findAndModify: kCollName, query: {a: 1}, update: {$set: {updated: true}}}),
+ // When the updateOneWithouShardKey feature flag is not enabled, upsert operations require the
+ // entire shard key to be specified in the query.
+ assert.commandFailedWithCode(sessionColl.update({d: 1}, {b: 1, c: 4, d: 1}), 31025);
+ assert.writeErrorWithCode(
+ mongos.getCollection(kNsName).update({b: 2}, {$set: {c: 2}}, {upsert: true}),
ErrorCodes.ShardKeyNotFound);
+
+ assert.commandWorked(sessionColl.insert({_id: "findAndModify", a: 1}));
+ assert.commandFailedWithCode(sessionDB.runCommand({
+ findAndModify: kCollName,
+ query: {a: 2},
+ update: {$set: {updated: true}, upsert: true}
+ }),
+ ErrorCodes.ShardKeyNotFound);
}
st.stop();
diff --git a/jstests/sharding/mongos_validate_writes.js b/jstests/sharding/mongos_validate_writes.js
index d52fe988aa9..9eb2aeb2562 100644
--- a/jstests/sharding/mongos_validate_writes.js
+++ b/jstests/sharding/mongos_validate_writes.js
@@ -48,9 +48,12 @@ st.shardColl(coll, {c: 1}, {c: 0}, {c: 1}, coll.getDB(), true);
// Make sure we can successfully upsert, even though we have stale state
assert.commandWorked(staleCollA.update({c: "c"}, {c: "c"}, true));
-// Make sure we unsuccessfully upsert with old info
-assert.commandFailedWithCode(staleCollB.update({b: "b"}, {b: "b"}, true),
- ErrorCodes.ShardKeyNotFound);
+if (!WriteWithoutShardKeyTestUtil.isWriteWithoutShardKeyFeatureEnabled(coll.getDB())) {
+ // When updateOneWithoutShardKey feature flag is disabled, make sure we unsuccessfully upsert
+ // with old info.
+ assert.commandFailedWithCode(staleCollB.update({b: "b"}, {b: "b"}, true),
+ ErrorCodes.ShardKeyNotFound);
+}
// Change the collection sharding state
coll.drop();
@@ -63,12 +66,9 @@ assert.commandWorked(coll.insert({d: "d"}));
assert.commandWorked(staleCollA.update({d: "d"}, {$set: {x: "x"}}, false, false));
assert.eq(staleCollA.findOne().x, "x");
-// Sharded updateOnes that do not directly target a shard can now use the two phase write protocol
-// to execute.
-if (WriteWithoutShardKeyTestUtil.isWriteWithoutShardKeyFeatureEnabled(admin)) {
- assert.commandWorked(staleCollB.update({c: "c"}, {$set: {x: "y"}}, false, false));
-} else {
- // Make sure we unsuccessfully update with old info
+if (!WriteWithoutShardKeyTestUtil.isWriteWithoutShardKeyFeatureEnabled(coll.getDB())) {
+ // When updateOneWithoutShardKey feature flag is disabled, make sure we unsuccessfully update
+ // with old info.
assert.commandFailedWithCode(staleCollB.update({c: "c"}, {$set: {x: "y"}}, false, false),
ErrorCodes.InvalidOptions);
}
@@ -88,12 +88,9 @@ assert.commandWorked(coll.insert({e: "e"}));
assert.commandWorked(staleCollA.remove({e: "e"}, true));
assert.eq(null, staleCollA.findOne());
-// Sharded deleteOnes that do not directly target a shard can now use the two phase write protocol
-// to execute.
-if (WriteWithoutShardKeyTestUtil.isWriteWithoutShardKeyFeatureEnabled(admin)) {
- assert.commandWorked(staleCollB.remove({d: "d"}, true));
-} else {
- // Make sure we unsuccessfully remove with old info
+if (!WriteWithoutShardKeyTestUtil.isWriteWithoutShardKeyFeatureEnabled(coll.getDB())) {
+ // When updateOneWithoutShardKey feature flag is disabled, make sure we unsuccessfully delete
+ // with old info.
assert.commandFailedWithCode(staleCollB.remove({d: "d"}, true), ErrorCodes.ShardKeyNotFound);
}
diff --git a/jstests/sharding/query/collation_targeting.js b/jstests/sharding/query/collation_targeting.js
index c5795ac2328..602c63c990a 100644
--- a/jstests/sharding/query/collation_targeting.js
+++ b/jstests/sharding/query/collation_targeting.js
@@ -491,7 +491,11 @@ assert.eq(1, writeRes.nMatched);
// Sharded upsert that does not target a single shard can now be executed with a two phase
// write protocol that will target at most 1 matching document.
if (WriteWithoutShardKeyTestUtil.isWriteWithoutShardKeyFeatureEnabled(testDB)) {
- // TODO: SERVER-73057 Implement upsert behavior for _clusterQueryWithoutShardKey
+ writeRes = coll.update(
+ {a: "filter"}, {$set: {b: 1}}, {multi: false, upsert: true, collation: caseInsensitive});
+ assert.commandWorked(writeRes);
+ assert.eq(1, writeRes.nUpserted);
+ assert.commandWorked(coll.remove({a: "filter"}, {justOne: true}));
} else {
// Upsert must always be single-shard.
diff --git a/jstests/sharding/query/collation_targeting_inherited.js b/jstests/sharding/query/collation_targeting_inherited.js
index c2e894940f9..7662baeee0f 100644
--- a/jstests/sharding/query/collation_targeting_inherited.js
+++ b/jstests/sharding/query/collation_targeting_inherited.js
@@ -520,7 +520,10 @@ assert.eq(1, writeRes.nMatched);
// Sharded upsert that does not target a single shard can now be executed with a two phase
// write protocol that will target at most 1 matching document.
if (WriteWithoutShardKeyTestUtil.isWriteWithoutShardKeyFeatureEnabled(testDB)) {
- // TODO: SERVER-73057 Implement upsert behavior for _clusterQueryWithoutShardKey
+ writeRes =
+ collCaseInsensitive.update({a: "filter"}, {$set: {b: 1}}, {multi: false, upsert: true});
+ assert.commandWorked(writeRes);
+ assert.eq(1, writeRes.nUpserted);
} else {
// Upsert must always be single-shard.
diff --git a/jstests/sharding/query/explain_cmd.js b/jstests/sharding/query/explain_cmd.js
index 00cce44d184..544c426f546 100644
--- a/jstests/sharding/query/explain_cmd.js
+++ b/jstests/sharding/query/explain_cmd.js
@@ -2,6 +2,8 @@
(function() {
'use strict';
+load("jstests/sharding/updateOne_without_shard_key/libs/write_without_shard_key_test_util.js");
+
// Create a cluster with 3 shards.
var st = new ShardingTest({shards: 2});
@@ -133,12 +135,25 @@ assert.eq(explain.queryPlanner.winningPlan.shards.length, 1);
// Check that the upsert didn't actually happen.
assert.eq(0, collSharded.count({a: 10}));
-// Explain an upsert operation which cannot be targeted, ensure an error is thrown
-explain = db.runCommand({
- explain: {update: collSharded.getName(), updates: [{q: {b: 10}, u: {b: 10}, upsert: true}]},
- verbosity: "allPlansExecution"
-});
-assert.commandFailed(explain, tojson(explain));
+if (WriteWithoutShardKeyTestUtil.isWriteWithoutShardKeyFeatureEnabled(collSharded.getDB())) {
+ // Explain an upsert operation which cannot be targeted and verify that it is successful.
+ // TODO SERVER-69922: Verify expected response.
+ explain = db.runCommand({
+ explain: {update: collSharded.getName(), updates: [{q: {b: 10}, u: {b: 10}, upsert: true}]},
+ verbosity: "allPlansExecution"
+ });
+ assert.commandWorked(explain, tojson(explain));
+ assert.eq(explain.queryPlanner.winningPlan.shards.length, 2);
+ // Check that the upsert didn't actually happen.
+ assert.eq(0, collSharded.count({b: 10}));
+} else {
+ // Explain an upsert operation which cannot be targeted, ensure an error is thrown
+ explain = db.runCommand({
+ explain: {update: collSharded.getName(), updates: [{q: {b: 10}, u: {b: 10}, upsert: true}]},
+ verbosity: "allPlansExecution"
+ });
+ assert.commandFailed(explain, tojson(explain));
+}
// Explain a changeStream, ensure an error is thrown under snapshot read concern.
const session = db.getMongo().startSession();
diff --git a/jstests/sharding/regex_targeting.js b/jstests/sharding/regex_targeting.js
index f07eaf95bfa..e0bbf452208 100644
--- a/jstests/sharding/regex_targeting.js
+++ b/jstests/sharding/regex_targeting.js
@@ -1,4 +1,6 @@
-// This checks to make sure that sharded regex queries behave the same as unsharded regex queries
+// This checks to make sure that sharded regex queries behave the same as unsharded regex queries.
+// Note, when the updateOneWithoutShardKey feature flag is enabled, upsert operations with queries
+// that do not match on the entire shard key are successful.
(function() {
'use strict';
@@ -156,14 +158,32 @@ collSharded.remove({});
collCompound.remove({});
collNested.remove({});
-// Sharded updateOnes that do not directly target a shard can now use the two phase write
-// protocol to execute.
if (WriteWithoutShardKeyTestUtil.isWriteWithoutShardKeyFeatureEnabled(st.s)) {
+ // Op-style updates with regex succeed on sharded collections.
assert.commandWorked(collSharded.update({a: /abcde-1/}, {"$set": {b: 1}}, {upsert: false}));
assert.commandWorked(collSharded.update({a: /abcde-[1-2]/}, {"$set": {b: 1}}, {upsert: false}));
assert.commandWorked(collNested.update(
{a: {b: /abcde-1/}}, {"$set": {"a.b": /abcde-1/, b: 1}}, {upsert: false}));
assert.commandWorked(collNested.update({"a.b": /abcde.*/}, {"$set": {b: 1}}, {upsert: false}));
+
+ assert.commandWorked(
+ collSharded.update({a: /abcde.*/}, {$set: {a: /abcde.*/}}, {upsert: true}));
+ assert.commandWorked(
+ collCompound.update({a: /abcde-1/}, {$set: {a: /abcde.*/, b: 1}}, {upsert: true}));
+ assert.commandWorked(
+ collNested.update({'a.b': /abcde.*/}, {$set: {'a.b': /abcde.*/}}, {upsert: true}));
+ assert.commandWorked(
+ collNested.update({a: {b: /abcde.*/}}, {$set: {'a.b': /abcde.*/}}, {upsert: true}));
+ assert.commandWorked(collNested.update({c: 1}, {$set: {'a.b': /abcde.*/}}, {upsert: true}));
+
+ // Upsert by replacement-style regex succeed on sharded collections.
+ assert.commandWorked(collSharded.update({a: /abcde.*/}, {a: /abcde.*/}, {upsert: true}));
+ assert.commandWorked(collCompound.update({a: /abcde.*/}, {a: /abcde.*/, b: 1}, {upsert: true}));
+ assert.commandWorked(
+ collNested.update({'a.b': /abcde-1/}, {a: {b: /abcde.*/}}, {upsert: true}));
+ assert.commandWorked(
+ collNested.update({a: {b: /abcde.*/}}, {a: {b: /abcde.*/}}, {upsert: true}));
+ assert.commandWorked(collNested.update({c: 1}, {a: {b: /abcde.*/}}, {upsert: true}));
} else {
//
//
@@ -181,26 +201,6 @@ if (WriteWithoutShardKeyTestUtil.isWriteWithoutShardKeyFeatureEnabled(st.s)) {
assert.commandFailedWithCode(
collNested.update({"a.b": /abcde.*/}, {"$set": {b: 1}}, {upsert: false}),
ErrorCodes.InvalidOptions);
-}
-
-//
-//
-// Replacement style updates with regex should work on sharded collections.
-// If query clause is ambiguous, we fallback to using update clause for targeting.
-assert.commandWorked(collSharded.update({a: /abcde.*/}, {a: /abcde.*/, b: 1}, {upsert: false}));
-assert.commandWorked(collSharded.update({a: /abcde-1/}, {a: /abcde-1/, b: 1}, {upsert: false}));
-assert.commandWorked(collNested.update({a: {b: /abcde.*/}}, {a: {b: /abcde.*/}}, {upsert: false}));
-assert.commandWorked(collNested.update({'a.b': /abcde-1/}, {a: {b: /abcde.*/}}, {upsert: false}));
-
-// Sharded updateOnes that do not directly target a shard can now use the two phase write
-// protocol to execute.
-if (WriteWithoutShardKeyTestUtil.isWriteWithoutShardKeyFeatureEnabled(st.s)) {
- // TODO: SERVER-73057 Implement upsert behavior for _clusterQueryWithoutShardKey
-} else {
- //
- //
- // Upsert with op-style regex should fail on sharded collections
- // Query clause is targeted, and regex in query clause is ambiguous
// The queries will also be interpreted as regex based prefix search and cannot target a single
// shard.
@@ -234,6 +234,13 @@ if (WriteWithoutShardKeyTestUtil.isWriteWithoutShardKeyFeatureEnabled(st.s)) {
ErrorCodes.ShardKeyNotFound);
}
+// Replacement style updates with regex should work on sharded collections.
+// If query clause is ambiguous, we fallback to using update clause for targeting.
+assert.commandWorked(collSharded.update({a: /abcde.*/}, {a: /abcde.*/, b: 1}, {upsert: false}));
+assert.commandWorked(collSharded.update({a: /abcde-1/}, {a: /abcde-1/, b: 1}, {upsert: false}));
+assert.commandWorked(collNested.update({a: {b: /abcde.*/}}, {a: {b: /abcde.*/}}, {upsert: false}));
+assert.commandWorked(collNested.update({'a.b': /abcde-1/}, {a: {b: /abcde.*/}}, {upsert: false}));
+
//
//
// Remove by regex should hit all matching keys, across all shards if applicable
diff --git a/jstests/sharding/shard6.js b/jstests/sharding/shard6.js
index a2b60b77b98..491eb9f5a16 100644
--- a/jstests/sharding/shard6.js
+++ b/jstests/sharding/shard6.js
@@ -1,6 +1,9 @@
// shard6.js
(function() {
"use strict";
+
+load("jstests/sharding/updateOne_without_shard_key/libs/write_without_shard_key_test_util.js");
+
var summary = "";
var s = new ShardingTest({name: "shard6", shards: 2});
@@ -103,11 +106,14 @@ checkItCount(2);
poolStats("after checking itcount");
-// --- Verify that modify & save style updates doesn't work on sharded clusters ---
+// When the updateOneWithoutShardKey feature flag is disabled, verify that modify & save style
+// updates doesn't work on sharded clusters.
+if (!WriteWithoutShardKeyTestUtil.isWriteWithoutShardKeyFeatureEnabled(db)) {
+ var o = db.data.findOne();
+ o.x = 16;
+ assert.commandFailedWithCode(db.data.save(o), ErrorCodes.ShardKeyNotFound);
+}
-var o = db.data.findOne();
-o.x = 16;
-assert.commandFailedWithCode(db.data.save(o), ErrorCodes.ShardKeyNotFound);
poolStats("at end");
print(summary);
diff --git a/jstests/sharding/updateOne_without_shard_key/find_and_modify_without_shard_key.js b/jstests/sharding/updateOne_without_shard_key/find_and_modify_without_shard_key.js
index 09ad5a8e797..4a3bb99dbec 100644
--- a/jstests/sharding/updateOne_without_shard_key/find_and_modify_without_shard_key.js
+++ b/jstests/sharding/updateOne_without_shard_key/find_and_modify_without_shard_key.js
@@ -37,27 +37,42 @@ function verifyResult(testCase, res) {
assert.commandFailedWithCode(res, testCase.errorCode);
} else {
assert.commandWorked(res);
- assert.eq(testCase.resultDoc, testColl.findOne(testCase.resultDoc));
- // No document matched the query, no modification was made.
- if (testCase.insertDoc.y != testCase.cmdObj.query.y) {
- assert.eq(0, res.lastErrorObject.n, res);
- assert.eq(false, res.lastErrorObject.updatedExisting);
- } else {
+ let noMod = testCase.insertDoc ? (testCase.insertDoc.y != testCase.cmdObj.query.y) : false;
+ if (testCase.cmdObj.upsert) {
assert.eq(1, res.lastErrorObject.n, res);
+ assert.eq(false, res.lastErrorObject.updatedExisting);
+ assert.eq(testCase.resultDoc._id, res.lastErrorObject.upserted);
+ assert.eq(testCase.resultDoc, testColl.findOne(testCase.resultDoc));
- // Check for pre/post image in command response.
- if (testCase.cmdObj.new) {
- assert.eq(testCase.resultDoc, res.value, res.value);
+ // Clean up, remove upserted document from db.
+ assert.commandWorked(testColl.deleteOne({_id: testCase.resultDoc._id}));
+ assert.eq(null, testColl.findOne({_id: testCase.resultDoc._id}));
+ } else {
+ if (noMod) {
+ // No modification expected.
+ assert.eq(0, res.lastErrorObject.n, res);
+ assert.eq(false, res.lastErrorObject.updatedExisting);
+ assert.eq(testCase.insertDoc, testColl.findOne(testCase.insertDoc));
} else {
- assert.eq(testCase.insertDoc, res.value, res.value);
+ assert.eq(1, res.lastErrorObject.n, res);
+ assert.eq(testCase.resultDoc, testColl.findOne(testCase.resultDoc));
}
+
+ // Clean up, remove inserted document from db.
+ assert.commandWorked(testColl.deleteOne({_id: testCase.insertDoc._id}));
+ assert.eq(null, testColl.findOne({_id: testCase.insertDoc._id}));
}
- }
- // Clean up, remove document from db.
- assert.commandWorked(testColl.deleteOne({_id: testCase.insertDoc._id}));
- assert.eq(null, testColl.findOne({_id: testCase.insertDoc._id}));
+ // Check for pre/post image in command response.
+ if (noMod) {
+ assert.eq(null, res.value);
+ } else if (testCase.cmdObj.new) {
+ assert.eq(testCase.resultDoc, res.value, res.value);
+ } else {
+ assert.eq(testCase.insertDoc, res.value, res.value);
+ }
+ }
}
// When more than one document matches the command query, ensure that a single document is modified
@@ -110,7 +125,9 @@ function verifySingleModification(testCase, res) {
function runCommandAndVerify(testCase, additionalCmdFields = {}) {
const cmdObjWithAdditionalFields = Object.assign({}, testCase.cmdObj, additionalCmdFields);
- assert.commandWorked(testColl.insert(testCase.insertDoc));
+ if (testCase.insertDoc) {
+ assert.commandWorked(testColl.insert(testCase.insertDoc));
+ }
const res = st.getDB(dbName).runCommand(cmdObjWithAdditionalFields);
if (cmdObjWithAdditionalFields.hasOwnProperty("autocommit") && !testCase.errorCode) {
@@ -122,7 +139,7 @@ function runCommandAndVerify(testCase, additionalCmdFields = {}) {
}));
}
- if (testCase.insertDoc.length > 1) {
+ if (testCase.insertDoc && testCase.insertDoc.length > 1) {
return verifySingleModification(testCase, res);
} else {
verifyResult(testCase, res);
@@ -142,6 +159,29 @@ const testCases = [
}
},
{
+ logMessage: "Upsert document.",
+ insertDoc: null,
+ resultDoc: {_id: 5, a: 0},
+ cmdObj: {
+ findAndModify: collectionName,
+ query: {_id: 5, a: -1},
+ update: {$inc: {a: 1}},
+ upsert: true,
+ },
+ },
+ {
+ logMessage: "Upsert document with post image.",
+ insertDoc: null,
+ resultDoc: {_id: 6, a: 3},
+ cmdObj: {
+ findAndModify: collectionName,
+ query: {_id: 6, a: -1},
+ update: {$inc: {a: 4}},
+ upsert: true,
+ new: true,
+ },
+ },
+ {
logMessage: "Aggregation update style, no sort filter, preimage.",
insertDoc: {_id: 1, x: 1, y: 4},
resultDoc: {_id: 1, x: 1, y: 1},
@@ -164,7 +204,7 @@ const testCases = [
{
logMessage: "Query does not match, no update.",
insertDoc: {_id: 2, x: -2, y: 6},
- resultDoc: {_id: 2, x: -2, y: 6},
+ resultDoc: null,
cmdObj: {
findAndModify: collectionName,
query: {y: 5},
diff --git a/jstests/sharding/updateOne_without_shard_key/libs/write_without_shard_key_test_util.js b/jstests/sharding/updateOne_without_shard_key/libs/write_without_shard_key_test_util.js
index cc171b32065..f39bde71698 100644
--- a/jstests/sharding/updateOne_without_shard_key/libs/write_without_shard_key_test_util.js
+++ b/jstests/sharding/updateOne_without_shard_key/libs/write_without_shard_key_test_util.js
@@ -98,6 +98,11 @@ var WriteWithoutShardKeyTestUtil = (function() {
if (operationType === OperationType.updateOne) {
assert.eq(expectedResponse.n, res.n);
assert.eq(expectedResponse.nModified, res.nModified);
+ cmdObj.updates.forEach(update => {
+ if (update.upsert) {
+ assert.eq(expectedResponse.upserted, res.upserted);
+ }
+ });
} else if (operationType === OperationType.deleteOne) {
assert.eq(expectedResponse.n, res.n);
} else {
diff --git a/jstests/sharding/updateOne_without_shard_key/updateOne_without_shard_key_basic.js b/jstests/sharding/updateOne_without_shard_key/updateOne_without_shard_key_basic.js
index 6838465c238..aba60f182ac 100644
--- a/jstests/sharding/updateOne_without_shard_key/updateOne_without_shard_key_basic.js
+++ b/jstests/sharding/updateOne_without_shard_key/updateOne_without_shard_key_basic.js
@@ -378,6 +378,38 @@ const testCases = [
dbName: dbName,
collName: collName
},
+ {
+ logMessage:
+ "Running a single update where no document matches on the query and {upsert: true}",
+ docsToInsert: [],
+ cmdObj: {
+ update: collName,
+ updates: [{q: {y: 5}, u: {_id: 5, x: -1}, upsert: true}],
+ },
+ options: [{ordered: true}, {ordered: false}],
+ expectedMods: [{_id: 5, x: -1, y: 5}],
+ expectedResponse: {n: 1, nModified: 0, upserted: [{"index": 0, _id: 5}]},
+ dbName: dbName,
+ collName: collName
+ },
+ {
+ logMessage: "Running a batch update without shard key with an upsert: true update.",
+ docsToInsert: [
+ {_id: 0, x: xFieldValShard0_1, y: yFieldVal},
+ ],
+ cmdObj: {
+ update: collName,
+ updates: [
+ {q: {y: yFieldVal}, u: {y: yFieldVal + 1}},
+ {q: {y: 6}, u: {x: -1, _id: 6}, upsert: true}
+ ],
+ },
+ options: [{ordered: true}, {ordered: false}],
+ expectedMods: [{_id: 0, x: xFieldValShard0_1, y: yFieldVal + 1}, {_id: 6, y: 6, x: -1}],
+ expectedResponse: {n: 2, nModified: 1, upserted: [{"index": 1, _id: 6}]},
+ dbName: dbName,
+ collName: collName
+ },
];
const configurations = [
diff --git a/jstests/sharding/update_compound_shard_key.js b/jstests/sharding/update_compound_shard_key.js
index e651f1d8cb2..1924aef7ce1 100644
--- a/jstests/sharding/update_compound_shard_key.js
+++ b/jstests/sharding/update_compound_shard_key.js
@@ -240,7 +240,15 @@ assert.eq(1, sessionDB.coll.find(updateDocTxn).itcount());
// Shard key field modifications do not have to specify full shard key.
if (WriteWithoutShardKeyTestUtil.isWriteWithoutShardKeyFeatureEnabled(st.s)) {
- // TODO: SERVER-73057 Implement upsert behavior for _clusterQueryWithoutShardKey
+ // Query on partial shard key.
+ assertUpdateWorkedWithNoMatchingDoc({x: 101, y: 50, a: 5},
+ {x: 100, y: 55, z: 3, a: 1},
+ true /* isUpsert */,
+ false /* inTransaction */);
+ assertUpdateWorkedWithNoMatchingDoc({x: 102, y: 50, nonExistingField: true},
+ {x: 102, y: 55, z: 3, a: 1},
+ true /* isUpsert */,
+ false /* inTransaction */);
} else {
// Full shard key not specified in query.
@@ -363,11 +371,19 @@ assert.eq(1, sessionDB.coll.find(upsertDocTxn["$set"]).itcount());
// Shard key field modifications do not have to specify full shard key.
if (WriteWithoutShardKeyTestUtil.isWriteWithoutShardKeyFeatureEnabled(st.s)) {
- // TODO: SERVER-73057 Implement upsert behavior for _clusterQueryWithoutShardKey
+ // Partial shard key updates are successful.
+ assertUpdateWorkedWithNoMatchingDoc(
+ {x: 14}, {"$set": {opStyle: 5}}, true /* isUpsert */, false /* inTransaction */);
+ assertUpdateWorkedWithNoMatchingDoc({x: 100, y: 51, nonExistingField: true},
+ {"$set": {x: 110, y: 55, z: 3, a: 8}},
+ true /* isUpsert */,
+ false /* inTransaction */);
+ assertUpdateWorkedWithNoMatchingDoc(
+ {y: 4}, {"$set": {z: 3, x: 4, y: 3, a: 2}}, true /* isUpsert */, false /* inTransaction */);
} else {
- // Full shard key not specified in query.
-
- // Query on _id doesn't work for upserts.
+ // TODO SERVER-75194: Move test case outside if-else. Should error with ShardKeyNotFound
+ // regardless of updateOneWithoutShardKey feature flag enablement. Query on _id doesn't work for
+ // upserts.
assert.commandFailedWithCode(
st.s.getDB(kDbName).coll.update(
{_id: 0}, {"$set": {x: 2, y: 11, z: 10, opStyle: 7}}, {upsert: true}),
@@ -516,7 +532,10 @@ assert.commandWorked(session.commitTransaction_forTesting());
assert.eq(1, sessionDB.coll.find(upsertProjectTxnDoc).itcount());
if (WriteWithoutShardKeyTestUtil.isWriteWithoutShardKeyFeatureEnabled(st.s)) {
- // TODO: SERVER-73057 Implement upsert behavior for _clusterQueryWithoutShardKey
+ assertUpdateWorkedWithNoMatchingDoc({_id: 18, z: 4, x: 3},
+ [{$addFields: {foo: 4}}],
+ true /* isUpsert */,
+ false /* inTransaction */);
} else {
// Full shard key not specified in query.
assert.commandFailedWithCode(
diff --git a/jstests/sharding/update_replace_id.js b/jstests/sharding/update_replace_id.js
index 346ff90424b..04215a1c602 100644
--- a/jstests/sharding/update_replace_id.js
+++ b/jstests/sharding/update_replace_id.js
@@ -18,6 +18,7 @@
*/
(function() {
load("jstests/libs/profiler.js"); // For profilerHas*OrThrow helper functions.
+load("jstests/sharding/updateOne_without_shard_key/libs/write_without_shard_key_test_util.js");
// Test deliberately inserts orphans outside of migrations.
TestData.skipCheckOrphans = true;
@@ -162,11 +163,14 @@ function runReplacementUpdateTestsForCompoundShardKey() {
filter: {op: "update", "command.u.msg": "update_extracted_id_from_query"}
});
- // An upsert whose query doesn't have full shard key will fail.
- assert.commandFailedWithCode(
- mongosColl.update(
- {_id: 101}, {a: 101, msg: "upsert_extracted_id_from_query"}, {upsert: true}),
- ErrorCodes.ShardKeyNotFound);
+ // An upsert whose query doesn't have full shard key will fail if updateOneWithoutShardKey
+ // feature flag is not enabled.
+ if (!WriteWithoutShardKeyTestUtil.isWriteWithoutShardKeyFeatureEnabled(mongosColl.getDB())) {
+ assert.commandFailedWithCode(
+ mongosColl.update(
+ {_id: 101}, {a: 101, msg: "upsert_extracted_id_from_query"}, {upsert: true}),
+ ErrorCodes.ShardKeyNotFound);
+ }
// Verify that the document did not perform any writes.
assert.eq(0, mongosColl.find({_id: 101}).itcount());
diff --git a/jstests/sharding/update_sharded.js b/jstests/sharding/update_sharded.js
index af9176a91a4..8dfbb591d4a 100644
--- a/jstests/sharding/update_sharded.js
+++ b/jstests/sharding/update_sharded.js
@@ -58,14 +58,11 @@ for (let i = 0; i < 2; i++) {
assert.commandWorked(coll.update({_id: 2}, {key: 2, other: 2}));
assert.commandWorked(coll.update({_id: 3}, {key: 3, other: 3}));
- // TODO: SERVER-73057 Implement upsert behavior for _clusterQueryWithoutShardKey
- if (!WriteWithoutShardKeyTestUtil.isWriteWithoutShardKeyFeatureEnabled(sessionDb)) {
- // do a replacement-style update which queries the shard key and keeps it constant
- assert.commandWorked(coll.update({key: 4}, {_id: 4, key: 4}, {upsert: true}));
- assert.commandWorked(coll.update({key: 4}, {key: 4, other: 4}));
- assert.eq(coll.find({key: 4, other: 4}).count(), 1, 'replacement update error');
- coll.remove({_id: 4});
- }
+ // do a replacement-style update which queries the shard key and keeps it constant
+ assert.commandWorked(coll.update({key: 4}, {_id: 4, key: 4}, {upsert: true}));
+ assert.commandWorked(coll.update({key: 4}, {key: 4, other: 4}));
+ assert.eq(coll.find({key: 4, other: 4}).count(), 1, 'replacement update error');
+ coll.remove({_id: 4});
assert.eq(coll.count(), 3, "count B");
coll.find().forEach(function(x) {
@@ -80,14 +77,11 @@ for (let i = 0; i < 2; i++) {
assert.commandWorked(coll.update({_id: 1, key: 1}, {$set: {foo: 2}}));
- // TODO: SERVER-73057 Implement upsert behavior for _clusterQueryWithoutShardKey
- if (!WriteWithoutShardKeyTestUtil.isWriteWithoutShardKeyFeatureEnabled(sessionDb)) {
- coll.update({key: 17}, {$inc: {x: 5}}, true);
- assert.eq(5, coll.findOne({key: 17}).x, "up1");
+ coll.update({key: 17}, {$inc: {x: 5}}, true);
+ assert.eq(5, coll.findOne({key: 17}).x, "up1");
- coll.update({key: 18}, {$inc: {x: 5}}, true, true);
- assert.eq(5, coll.findOne({key: 18}).x, "up2");
- }
+ coll.update({key: 18}, {$inc: {x: 5}}, true, true);
+ assert.eq(5, coll.findOne({key: 18}).x, "up2");
// Make sure we can extract exact _id from certain queries
assert.commandWorked(coll.update({_id: ObjectId()}, {$set: {x: 1}}, {multi: false}));
diff --git a/jstests/sharding/upsert_sharded.js b/jstests/sharding/upsert_sharded.js
index 10d705bc532..84065b120d3 100644
--- a/jstests/sharding/upsert_sharded.js
+++ b/jstests/sharding/upsert_sharded.js
@@ -1,10 +1,13 @@
//
-// Upsert behavior tests for sharding
-// NOTE: Generic upsert behavior tests belong in the core suite
+// Upsert behavior tests for sharding. If updateOneWithoutShardKey feature flag is enabled, upsert
+// operations with queries that do not match on the entire shard key are successful. NOTE: Generic
+// upsert behavior tests belong in the core suite
//
(function() {
'use strict';
+load("jstests/sharding/updateOne_without_shard_key/libs/write_without_shard_key_test_util.js");
+
const st = new ShardingTest({shards: 2, mongos: 1});
const mongos = st.s0;
@@ -77,35 +80,48 @@ assert.eq(1, upsertedXVal(coll, {x: {$in: [1]}}, {$set: {a: 1}}));
assert.eq(1, upsertedXVal(coll, {$and: [{x: {$eq: 1}}]}, {$set: {a: 1}}));
assert.eq(1, upsertedXVal(coll, {$or: [{x: {$eq: 1}}]}, {$set: {a: 1}}));
-// Missing shard key in query.
-assert.commandFailedWithCode(upsertedResult(coll, {}, {$set: {a: 1, x: 1}}),
- ErrorCodes.ShardKeyNotFound);
-
-// Missing equality match on shard key in query.
-assert.commandFailedWithCode(upsertedResult(coll, {x: {$gt: 10}}, {$set: {a: 1, x: 5}}),
- ErrorCodes.ShardKeyNotFound);
-
-// Regex shard key value in query is ambigious and cannot be extracted for an equality match.
-assert.commandFailedWithCode(
- upsertedResult(coll, {x: {$eq: /abc*/}}, {$set: {a: 1, x: "regexValue"}}),
- ErrorCodes.ShardKeyNotFound);
-assert.commandFailedWithCode(upsertedResult(coll, {x: {$eq: /abc/}}, {$set: {a: 1, x: /abc/}}),
- ErrorCodes.ShardKeyNotFound);
+if (WriteWithoutShardKeyTestUtil.isWriteWithoutShardKeyFeatureEnabled(coll.getDB())) {
+ assert.eq(1, upsertedXVal(coll, {}, {$set: {a: 1, x: 1}}));
+ assert.eq(5, upsertedXVal(coll, {x: {$gt: 10}}, {$set: {a: 1, x: 5}}));
+ assert.eq("regexValue",
+ upsertedXVal(coll, {x: {$eq: /abc*/}}, {$set: {a: 1, x: "regexValue"}}));
+ assert.eq(/abc/, upsertedXVal(coll, {x: {$eq: /abc/}}, {$set: {a: 1, x: /abc/}}));
+ assert.eq({$gt: 5}, upsertedXVal(coll, {x: {$eq: {$gt: 5}}}, {$set: {a: 1}}));
+ assert.eq({x: 1}, upsertedXVal(coll, {"x.x": 1}, {$set: {a: 1}}));
+ assert.eq({x: 1}, upsertedXVal(coll, {"x.x": {$eq: 1}}, {$set: {a: 1}}));
+} else {
+ // Missing shard key in query.
+ assert.commandFailedWithCode(upsertedResult(coll, {}, {$set: {a: 1, x: 1}}),
+ ErrorCodes.ShardKeyNotFound);
+
+ // Missing equality match on shard key in query.
+ assert.commandFailedWithCode(upsertedResult(coll, {x: {$gt: 10}}, {$set: {a: 1, x: 5}}),
+ ErrorCodes.ShardKeyNotFound);
+
+ // Regex shard key value in query is ambigious and cannot be extracted for an equality match.
+ assert.commandFailedWithCode(
+ upsertedResult(coll, {x: {$eq: /abc*/}}, {$set: {a: 1, x: "regexValue"}}),
+ ErrorCodes.ShardKeyNotFound);
+ assert.commandFailedWithCode(upsertedResult(coll, {x: {$eq: /abc/}}, {$set: {a: 1, x: /abc/}}),
+ ErrorCodes.ShardKeyNotFound);
+
+ // Shard key in query is not extractable.
+ assert.commandFailedWithCode(upsertedResult(coll, {x: {$eq: {$gt: 5}}}, {$set: {a: 1}}),
+ ErrorCodes.ShardKeyNotFound);
+
+ // Nested field extraction always fails with non-nested key - like _id, we require setting the
+ // elements directly
+ assert.commandFailedWithCode(upsertedResult(coll, {"x.x": 1}, {$set: {a: 1}}),
+ ErrorCodes.ShardKeyNotFound);
+ assert.commandFailedWithCode(upsertedResult(coll, {"x.x": {$eq: 1}}, {$set: {a: 1}}),
+ ErrorCodes.ShardKeyNotFound);
+}
// Shard key in query not extractable.
assert.commandFailedWithCode(upsertedResult(coll, {x: undefined}, {$set: {a: 1}}),
ErrorCodes.BadValue);
assert.commandFailedWithCode(upsertedResult(coll, {x: [1, 2]}, {$set: {a: 1}}),
ErrorCodes.ShardKeyNotFound);
-assert.commandFailedWithCode(upsertedResult(coll, {x: {$eq: {$gt: 5}}}, {$set: {a: 1}}),
- ErrorCodes.ShardKeyNotFound);
-
-// Nested field extraction always fails with non-nested key - like _id, we require setting the
-// elements directly
-assert.commandFailedWithCode(upsertedResult(coll, {"x.x": 1}, {$set: {a: 1}}),
- ErrorCodes.ShardKeyNotFound);
-assert.commandFailedWithCode(upsertedResult(coll, {"x.x": {$eq: 1}}, {$set: {a: 1}}),
- ErrorCodes.ShardKeyNotFound);
coll.drop();
@@ -174,9 +190,13 @@ assert.commandFailedWithCode(upsertedResult(coll, {"x.x": -1}, {x: {x: []}}),
assert.commandFailedWithCode(upsertedResult(coll, {"x.x": -1}, {x: [{x: 1}]}),
ErrorCodes.NotSingleValueField);
-// Can't set sub-fields of nested key
-assert.commandFailedWithCode(upsertedResult(coll, {"x.x.x": {$eq: 1}}, {$set: {a: 1}}),
- ErrorCodes.ShardKeyNotFound);
+if (WriteWithoutShardKeyTestUtil.isWriteWithoutShardKeyFeatureEnabled(coll.getDB())) {
+ assert.eq({x: {x: 1}}, upsertedXVal(coll, {"x.x.x": {$eq: 1}}, {$set: {a: 1}}));
+} else {
+ // Can't set sub-fields of nested key
+ assert.commandFailedWithCode(upsertedResult(coll, {"x.x.x": {$eq: 1}}, {$set: {a: 1}}),
+ ErrorCodes.ShardKeyNotFound);
+}
coll.drop();
@@ -249,26 +269,59 @@ assert.commandFailedWithCode(upsertSuppliedResult(coll, {_id: {x: -1, y: -1}}, {
assert.commandFailedWithCode(upsertedResult(coll, {_id: {x: -1, y: -1}}, {$unset: {"_id.y": 1}}),
ErrorCodes.ImmutableField);
-// No upsert type can set array element for nested _id key.
-assert.commandFailedWithCode(upsertedResult(coll, {_id: {x: [1]}}, {}),
- ErrorCodes.ShardKeyNotFound);
-assert.commandFailedWithCode(upsertedResult(coll, {"_id.x": [1]}, {}), ErrorCodes.ShardKeyNotFound);
-assert.commandFailedWithCode(upsertedResult(coll, {_id: [{x: 1}]}, {}),
- ErrorCodes.ShardKeyNotFound);
-
-assert.commandFailedWithCode(upsertSuppliedResult(coll, {_id: {x: [1]}}, {}),
- ErrorCodes.ShardKeyNotFound);
-assert.commandFailedWithCode(upsertSuppliedResult(coll, {"_id.x": [1]}, {}),
- ErrorCodes.ShardKeyNotFound);
-assert.commandFailedWithCode(upsertSuppliedResult(coll, {_id: [{x: 1}]}, {}),
- ErrorCodes.ShardKeyNotFound);
-
-assert.commandFailedWithCode(upsertedResult(coll, {_id: {x: [1]}}, {$set: {y: 1}}),
- ErrorCodes.ShardKeyNotFound);
-assert.commandFailedWithCode(upsertedResult(coll, {"_id.x": [1]}, {$set: {y: 1}}),
- ErrorCodes.ShardKeyNotFound);
-assert.commandFailedWithCode(upsertedResult(coll, {_id: [{x: 1}]}, {$set: {y: 1}}),
- ErrorCodes.ShardKeyNotFound);
+if (WriteWithoutShardKeyTestUtil.isWriteWithoutShardKeyFeatureEnabled(coll.getDB())) {
+ // Incorrect format of _id or shard key errors.
+ assert.commandFailedWithCode(
+ upsertedResult(coll, {_id: {x: [1]}}, {}),
+ ErrorCodes
+ .ShardKeyNotFound); // Shard key cannot contain array values or array descendants.
+ assert.commandFailedWithCode(upsertedResult(coll, {"_id.x": [1]}, {}),
+ ErrorCodes.NotExactValueField);
+ assert.commandFailedWithCode(upsertedResult(coll, {_id: [{x: 1}]}, {}),
+ ErrorCodes.NotSingleValueField);
+
+ assert.commandFailedWithCode(upsertSuppliedResult(coll, {_id: {x: [1]}}, {}),
+ ErrorCodes.NotSingleValueField);
+ assert.commandFailedWithCode(
+ upsertSuppliedResult(coll, {"_id.x": [1]}, {}),
+ ErrorCodes
+ .ShardKeyNotFound); // Shard key cannot contain array values or array descendants.
+ assert.commandFailedWithCode(upsertSuppliedResult(coll, {_id: [{x: 1}]}, {}),
+ ErrorCodes.NotSingleValueField);
+
+ assert.commandFailedWithCode(upsertedResult(coll, {_id: {x: [1]}}, {$set: {y: 1}}),
+ ErrorCodes.NotSingleValueField);
+ assert.commandFailedWithCode(
+ upsertedResult(coll, {"_id.x": [1]}, {$set: {y: 1}}),
+ ErrorCodes
+ .ShardKeyNotFound); // Shard key cannot contain array values or array descendants.
+ assert.commandFailedWithCode(
+ upsertedResult(coll, {_id: [{x: 1}]}, {$set: {y: 1}}),
+ ErrorCodes
+ .ShardKeyNotFound); // Shard key cannot contain array values or array descendants.
+} else {
+ // No upsert type can set array element for nested _id key.
+ assert.commandFailedWithCode(upsertedResult(coll, {_id: {x: [1]}}, {}),
+ ErrorCodes.ShardKeyNotFound);
+ assert.commandFailedWithCode(upsertedResult(coll, {"_id.x": [1]}, {}),
+ ErrorCodes.ShardKeyNotFound);
+ assert.commandFailedWithCode(upsertedResult(coll, {_id: [{x: 1}]}, {}),
+ ErrorCodes.ShardKeyNotFound);
+
+ assert.commandFailedWithCode(upsertSuppliedResult(coll, {_id: {x: [1]}}, {}),
+ ErrorCodes.ShardKeyNotFound);
+ assert.commandFailedWithCode(upsertSuppliedResult(coll, {"_id.x": [1]}, {}),
+ ErrorCodes.ShardKeyNotFound);
+ assert.commandFailedWithCode(upsertSuppliedResult(coll, {_id: [{x: 1}]}, {}),
+ ErrorCodes.ShardKeyNotFound);
+
+ assert.commandFailedWithCode(upsertedResult(coll, {_id: {x: [1]}}, {$set: {y: 1}}),
+ ErrorCodes.ShardKeyNotFound);
+ assert.commandFailedWithCode(upsertedResult(coll, {"_id.x": [1]}, {$set: {y: 1}}),
+ ErrorCodes.ShardKeyNotFound);
+ assert.commandFailedWithCode(upsertedResult(coll, {_id: [{x: 1}]}, {$set: {y: 1}}),
+ ErrorCodes.ShardKeyNotFound);
+}
// Replacement and op-style {$set _id} fail when using dotted-path query on nested _id key.
// TODO SERVER-44615: these tests should succeed when SERVER-44615 is complete.
diff --git a/src/mongo/s/collection_routing_info_targeter.cpp b/src/mongo/s/collection_routing_info_targeter.cpp
index 915567835e7..3bf81d1816b 100644
--- a/src/mongo/s/collection_routing_info_targeter.cpp
+++ b/src/mongo/s/collection_routing_info_targeter.cpp
@@ -456,10 +456,13 @@ std::vector<ShardEndpoint> CollectionRoutingInfoTargeter::targetUpdate(
uassertStatusOKWithContext(_targetShardKey(shardKey, collation, chunkRanges), msg)};
};
- // If this is an upsert, then the query must contain an exact match on the shard key. If we were
- // to target based on the replacement doc, it could result in an insertion even if a document
- // matching the query exists on another shard.
- if (isUpsert) {
+ // With the introduction of PM-1632, we can use the two phase write protocol to successfully
+ // target an upsert without the full shard key. Else, the the query must contain an exact match
+ // on the shard key. If we were to target based on the replacement doc, it could result in an
+ // insertion even if a document matching the query exists on another shard.
+ if (!feature_flags::gFeatureFlagUpdateOneWithoutShardKey.isEnabled(
+ serverGlobalParams.featureCompatibility) &&
+ isUpsert) {
return targetByShardKey(
extractShardKeyFromBasicQueryWithContext(expCtx, shardKeyPattern, query),
"Failed to target upsert by query");
diff --git a/src/mongo/s/commands/cluster_query_without_shard_key_cmd.cpp b/src/mongo/s/commands/cluster_query_without_shard_key_cmd.cpp
index b53add70911..e6b1ae47bd4 100644
--- a/src/mongo/s/commands/cluster_query_without_shard_key_cmd.cpp
+++ b/src/mongo/s/commands/cluster_query_without_shard_key_cmd.cpp
@@ -184,7 +184,7 @@ ParsedCommandInfo parseWriteCommand(OperationContext* opCtx,
parsedInfo.collation = *parsedCollation;
}
} else {
- uasserted(ErrorCodes::InvalidOptions, "Not a supported batch write command");
+ uasserted(ErrorCodes::InvalidOptions, "Not a supported write command");
}
return parsedInfo;
}
diff --git a/src/mongo/s/write_ops/batch_write_exec.cpp b/src/mongo/s/write_ops/batch_write_exec.cpp
index 665fb26a166..fa459e4f37d 100644
--- a/src/mongo/s/write_ops/batch_write_exec.cpp
+++ b/src/mongo/s/write_ops/batch_write_exec.cpp
@@ -408,9 +408,14 @@ void executeTwoPhaseWrite(OperationContext* opCtx,
Status responseStatus = swRes.getStatus();
BatchedCommandResponse batchedCommandResponse;
if (swRes.isOK()) {
- std::string errMsg;
- if (!batchedCommandResponse.parseBSON(swRes.getValue().getResponse(), &errMsg)) {
- responseStatus = {ErrorCodes::FailedToParse, errMsg};
+ // Explicitly set the status of a no-op if there is no response.
+ if (swRes.getValue().getResponse().isEmpty()) {
+ batchedCommandResponse.setStatus(Status::OK());
+ } else {
+ std::string errMsg;
+ if (!batchedCommandResponse.parseBSON(swRes.getValue().getResponse(), &errMsg)) {
+ responseStatus = {ErrorCodes::FailedToParse, errMsg};
+ }
}
}
@@ -433,17 +438,16 @@ void executeTwoPhaseWrite(OperationContext* opCtx,
invariant(nextBatch->getWrites().size() == 1);
if (responseStatus.isOK()) {
- // If no document matches were made, then the response shardId would be the empty string
- // and all of the child batches would record no-ops.
- if (!hasRecordedWriteResponseForFirstBatch && !swRes.getValue().getShardId().empty()) {
- if ((abortBatch = processResponseFromRemote(
- opCtx,
- targeter,
- ShardId(swRes.getValue().getShardId().toString()),
- batchedCommandResponse,
- batchOp,
- nextBatch.get(),
- stats))) {
+ if (!hasRecordedWriteResponseForFirstBatch) {
+ // Resolve the first child batch with the response of the write or a no-op response
+ // if there was no matching document.
+ if ((abortBatch = processResponseFromRemote(opCtx,
+ targeter,
+ nextBatch.get()->getShardId(),
+ batchedCommandResponse,
+ batchOp,
+ nextBatch.get(),
+ stats))) {
break;
}
hasRecordedWriteResponseForFirstBatch = true;
diff --git a/src/mongo/s/write_ops/write_without_shard_key_util.cpp b/src/mongo/s/write_ops/write_without_shard_key_util.cpp
index f098b60bb6f..a303ed20ef5 100644
--- a/src/mongo/s/write_ops/write_without_shard_key_util.cpp
+++ b/src/mongo/s/write_ops/write_without_shard_key_util.cpp
@@ -125,6 +125,51 @@ BSONObj generateUpsertDocument(OperationContext* opCtx, const UpdateRequest& upd
return parsedUpdate.getDriver()->getDocument().getObject();
}
+BSONObj constructUpsertResponse(BatchedCommandResponse& writeRes,
+ const BSONObj& targetDoc,
+ StringData commandName,
+ bool appendPostImage) {
+ BSONObj reply;
+ auto upsertedId = IDLAnyTypeOwned::parseFromBSON(targetDoc.getField(kIdFieldName));
+
+ if (commandName == write_ops::FindAndModifyCommandRequest::kCommandName ||
+ commandName == write_ops::FindAndModifyCommandRequest::kCommandAlias) {
+ write_ops::FindAndModifyLastError lastError;
+ lastError.setNumDocs(writeRes.getN());
+ lastError.setUpdatedExisting(false);
+ lastError.setUpserted(upsertedId);
+
+ write_ops::FindAndModifyCommandReply result;
+ result.setLastErrorObject(std::move(lastError));
+ if (appendPostImage) {
+ result.setValue(targetDoc);
+ }
+
+ reply = result.toBSON();
+ } else {
+ write_ops::UpdateCommandReply updateReply = write_ops::UpdateCommandReply::parse(
+ IDLParserContext("upsertWithoutShardKeyResult"), writeRes.toBSON());
+ write_ops::Upserted upsertedType;
+
+ // It is guaranteed that the index of this update is 0 since shards evaluate one
+ // targetedWrite per batch in a singleWriteWithoutShardKey.
+ upsertedType.setIndex(0);
+ upsertedType.set_id(upsertedId);
+ updateReply.setUpserted(std::vector<mongo::write_ops::Upserted>{upsertedType});
+
+ reply = updateReply.toBSON();
+ }
+
+ BSONObjBuilder responseBob(reply);
+ responseBob.append("ok", 1);
+
+ BSONObjBuilder bob;
+ bob.append("response", responseBob.obj());
+ bob.append("shardId", "");
+
+ return bob.obj();
+}
+
bool useTwoPhaseProtocol(OperationContext* opCtx,
NamespaceString nss,
bool isUpdateOrDelete,
@@ -218,26 +263,51 @@ StatusWith<ClusterWriteWithoutShardKeyResponse> runTwoPhaseWriteProtocol(Operati
ClusterQueryWithoutShardKeyResponse::parseOwned(
IDLParserContext("_clusterQueryWithoutShardKeyResponse"), std::move(queryRes));
- // If there's no matching document and upsert:false, then no modification needs to be
- // made.
+ // The target document can contain the target document's _id or a generated upsert
+ // document. If there's no targetDocument, then no modification needs to be made.
if (!queryResponse.getTargetDoc()) {
return SemiFuture<void>::makeReady();
}
- BSONObjBuilder bob(sharedBlock->cmdObj);
- ClusterWriteWithoutShardKey clusterWriteWithoutShardKeyCommand(
- bob.obj(),
- std::string(*queryResponse.getShardId()) /* shardId */,
- *queryResponse.getTargetDoc() /* targetDocId */);
-
- auto writeRes = txnClient
- .runCommand(sharedBlock->nss.dbName(),
- clusterWriteWithoutShardKeyCommand.toBSON(BSONObj()))
- .get();
- uassertStatusOK(getStatusFromCommandResult(writeRes));
+ // If upsertRequired, insert target document directly into the database.
+ if (queryResponse.getUpsertRequired()) {
+ std::vector<BSONObj> docs;
+ docs.push_back(queryResponse.getTargetDoc().get());
+ write_ops::InsertCommandRequest insertRequest(sharedBlock->nss, docs);
+
+ auto writeRes =
+ txnClient.runCRUDOp(insertRequest, std::vector<StmtId>{kUninitializedStmtId})
+ .get();
+
+ auto upsertResponse =
+ constructUpsertResponse(writeRes,
+ queryResponse.getTargetDoc().get(),
+ sharedBlock->cmdObj.firstElementFieldNameStringData(),
+ sharedBlock->cmdObj.getBoolField("new"));
+
+ sharedBlock->clusterWriteResponse = ClusterWriteWithoutShardKeyResponse::parseOwned(
+ IDLParserContext("_clusterWriteWithoutShardKeyResponse"),
+ std::move(upsertResponse));
+ } else {
+ BSONObjBuilder bob(sharedBlock->cmdObj);
+ ClusterWriteWithoutShardKey clusterWriteWithoutShardKeyCommand(
+ bob.obj(),
+ std::string(*queryResponse.getShardId()) /* shardId */,
+ *queryResponse.getTargetDoc() /* targetDocId */);
+
+ auto writeRes =
+ txnClient
+ .runCommand(sharedBlock->nss.dbName(),
+ clusterWriteWithoutShardKeyCommand.toBSON(BSONObj()))
+ .get();
+ uassertStatusOK(getStatusFromCommandResult(writeRes));
+
+ sharedBlock->clusterWriteResponse = ClusterWriteWithoutShardKeyResponse::parseOwned(
+ IDLParserContext("_clusterWriteWithoutShardKeyResponse"), std::move(writeRes));
+ }
+ uassertStatusOK(
+ getStatusFromWriteCommandReply(sharedBlock->clusterWriteResponse.getResponse()));
- sharedBlock->clusterWriteResponse = ClusterWriteWithoutShardKeyResponse::parseOwned(
- IDLParserContext("_clusterWriteWithoutShardKeyResponse"), std::move(writeRes));
return SemiFuture<void>::makeReady();
});