Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(bulk): allow upsert with empty update when using bulkWrite() #2490

Closed
wants to merge 1 commit into from
Closed

fix(bulk): allow upsert with empty update when using bulkWrite() #2490

wants to merge 1 commit into from

Conversation

vkarpov15
Copy link
Contributor

Description

In v3.5 bulkWrite() could do an updateOne with an empty update, so long as upsert was set. For example, the below script succeeds in v3.5.8 but throws a "Update document requires atomic operators" error in v3.6.

const { MongoClient, ObjectId } = require('mongodb');

void async function() {
  const client = await MongoClient.connect('mongodb://localhost:27017/test', {
    useNewUrlParser: true,
    useUnifiedTopology: true
  });

  const db = client.db();
  
  await db.collection('Test').bulkWrite([{
    updateOne: { filter: { name: 'test' }, update: {}, upsert: true }
  }]);

  console.log('Done');
  process.exit(0);
}();

What changed?

Added a check to allow the above script to pass

Are there any files to ignore?

Nope

@mbroadst
Copy link
Member

mbroadst commented Aug 4, 2020

Hey @vkarpov15 , I think this behavior change is desired. Can't you refactor the code to use an atomic operator:

  await db.collection('Test').bulkWrite([{
    updateOne: { filter: { name: 'test' }, update: { $set: {} }, upsert: true }
  }]);

?

@vkarpov15
Copy link
Contributor Author

I could do that, the only problem is that the server throws errors when you do an empty $set. The below script:

const { MongoClient, ObjectId } = require('mongodb');
  
void async function() {
  const client = await MongoClient.connect('mongodb://localhost:27017/test', {
    useNewUrlParser: true,
    useUnifiedTopology: true
  });

  const db = client.db();

  await db.collection('Test').bulkWrite([{
    updateOne: { filter: { name: 'test' }, update: { $set: {} }, upsert: true }
  }]);

  console.log('Done');
  process.exit(0);
}();

Throws the below error:

$ node ./test.js 
(node:10277) UnhandledPromiseRejectionWarning: BulkWriteError: '$set' is empty. You must specify a field like so: {$set: {<field>: ...}}
    at OrderedBulkOperation.handleWriteError (/troubleshoot-mongoose/node_modules/mongodb/lib/bulk/common.js:1213:9)
    at resultHandler (/troubleshoot-mongoose/node_modules/mongodb/lib/bulk/common.js:519:23)

Is this behavior defined in a spec somewhere? If not, I'd like to ask that we not go down the route of throwing errors on empty updates. It's already tricky to work around the empty $set issue in the server: https://jira.mongodb.org/browse/SERVER-12266, and this change breaks our workaround.

@mbroadst
Copy link
Member

mbroadst commented Aug 4, 2020

@vkarpov15 oops, good catch! Let me get back to you shortly on this one

vkarpov15 added a commit to Automattic/mongoose that referenced this pull request Aug 14, 2020
@mbroadst
Copy link
Member

@vkarpov15 I think we can close this now based on the discussion in DRIVERS-1359?

@vkarpov15
Copy link
Contributor Author

@mbroadst yep! Thanks for your patience in explaining this change

@vkarpov15 vkarpov15 closed this Aug 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants