diff options
author | Sanika Phanse <sanika.phanse@mongodb.com> | 2023-03-30 12:47:36 +0000 |
---|---|---|
committer | Evergreen Agent <no-reply@evergreen.mongodb.com> | 2023-03-30 13:48:17 +0000 |
commit | cf860fd6504eb80ea47905dab3b8b4c407a4b9f8 (patch) | |
tree | 542fca6b31754c5fe839ece796e66f5091186f34 | |
parent | 222dde5f2593af7c0e4175696d2fdd6ca54e24cd (diff) | |
download | mongo-cf860fd6504eb80ea47905dab3b8b4c407a4b9f8.tar.gz |
SERVER-74952 Insert upsert document for single writes without shard keys
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(); }); |