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(document): allow calling push() with different $position arguments #14254

Merged
merged 2 commits into from
Jan 18, 2024

Conversation

vkarpov15
Copy link
Collaborator

@vkarpov15 vkarpov15 commented Jan 12, 2024

Fix #14244
Re: #4322

Summary

When we originally implemented push() with $position in #4322, we made it so that calling push() with different $position arguments would throw a sync error. This decision is not completely unreasonable because the MongoDB server doesn't support $push with different $position arguments. However, this decision is inconsistent with the rest of the array API: as a general rule, Mongoose arrays revert to just making the operation a $set on the entire array when Mongoose can't represent the user's desired update using atomic operators. With this PR, we will convert from atomic update to $set on the whole array if the user calls push() with multiple different $position arguments.

With this change, a modified version of the repro script from #14244 prints the following:

Mongoose: events.insertOne({ name: 'event object', arr: [ 'x', 'y', 'z' ], _id: new ObjectId("65a1512d0debc605ade502af"), __v: 0}, {})
true
[ 'x', 'y', 'z', '8' ]
[ 'x', 'y', 'z', '8' ]
[ 'x', 'y', 'b', 'z', '8' ]
[ 'x', 'c', 'y', 'b', 'z', '8' ]
Mongoose: events.updateOne({ _id: new ObjectId("65a1512d0debc605ade502af"), __v: 0 }, { '$set': { arr: [ 'x', 'c', 'y', 'b', 'z', '8' ] }, '$inc': { __v: 1 }}, {})

Modified version as follows:

const mongoose = require('mongoose');

mongoose.set('debug', true);
const util = require('util')
console.log(mongoose.version)
let Schema = mongoose.Schema
var eventSchema = new Schema({
  name: { type: String },
  arr: { type: Array, patch: true },
})

var Event = mongoose.model('Event', eventSchema)
async function run() {
  await mongoose.connect('mongodb://localhost:27017');
  let eventObj = new Event({ name: 'event object', arr: ['x', 'y', 'z'] })
  eventObj.save(function (err, eventObj) {
    console.log(util.types.isProxy(eventObj['arr']))
    eventObj['arr'].push('8')
    console.log(eventObj['arr'])
    eventObj['arr'].push({
      $each: ['a'],
      $position: 3
    })
    console.log(eventObj['arr'])
    eventObj['arr'].push({
      $each: ['b'],
      $position: 2
    })
    console.log(eventObj['arr'])
    eventObj['arr'].push({
      $each: ['c'],
      $position: 1
    })
    console.log(eventObj['arr']);

    eventObj.save();
  })
}
run();

Examples

@vkarpov15 vkarpov15 merged commit 4b6d208 into 6.x Jan 18, 2024
34 checks passed
@hasezoey hasezoey deleted the vkarpov15/gh-14244 branch January 19, 2024 11:54
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.

1 participant