Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Use $ positional operator for updating embedded documents #2545

Closed
dblock opened this Issue · 4 comments

2 participants

@dblock

Mongoid supports a certain level of concurrency for top level documents, but not the same level of concurrency for embedded documents. Consider this:

require "spec_helper"

class Sandwich
  include Mongoid::Document
  embeds_many :slices
end

class Slice
  include Mongoid::Document
  field :thickness
  embedded_in :sandwich
end

describe "Concurrent updates" do

  it "work" do
    sandwich = Sandwich.create!
    slice1 = Slice.create!({ sandwich: sandwich, thickness: 1 })
    slice2 = Slice.create!({ sandwich: sandwich, thickness: 2 })

    sandwich.slices.count.should == 2
    sandwich.slices.first.thickness.should == 1
    sandwich.slices.last.thickness.should == 2

    Mongoid.default_session["sandwiches"].where({ _id: sandwich.id }).update({
      "$pull" => { "slices" => { _id: slice1.id }}
    })
    Mongoid.default_session["sandwiches"].find.first.should == {
      "_id" => sandwich.id,
      "slices" => [{ "_id" => slice2.id, "thickness" => slice2.thickness }]
    }

    # => we don't expect mongoid to notice the concurrent removal
    sandwich.slices.count.should == 2

    slice2.update_attributes!({ thickness: 3 })

    # => we do expect mongoid to update the correct record
    Mongoid.default_session["sandwiches"].find.first.should == {
      "_id" => sandwich.id,
      "slices" => [{ "_id" => slice2.id, "thickness" => slice2.thickness }]
    }

  end

end

Another process removed an embedded document, and a subsequent update inserts a new document without an _id.

  1) Concurrent updates works
     Failure/Error: }
       expected: {"_id"=>"509fd8d13b5552dde5000001", "slices"=>[{"_id"=>"509fd8d13b5552dde5000003", "thickness"=>3}]}
            got: {"_id"=>"509fd8d13b5552dde5000001", "slices"=>[{"_id"=>"509fd8d13b5552dde5000003", "thickness"=>2}, {"thickness"=>3}]} (using ==)
       Diff:
       @@ -1,3 +1,3 @@
        "_id" => "509fd8d13b5552dde5000001",
       -"slices" => [{"_id"=>"509fd8d13b5552dde5000003", "thickness"=>3}]
       +"slices" => [{"_id"=>"509fd8d13b5552dde5000003", "thickness"=>2}, {"thickness"=>3}]

Given that all embedded documents have an ID, this can be solved using a positional $ operator for updates (update({ slices._id : slice2.id }, { $set : { "slices.$.thickness" : 3 }}, false, true)) and a $pull with an ID (like above) for the removal.

@durran
Owner

I would love to do this, and the selectors are there but commented out at the moment when it can be implemented. However, the positional operator in MongoDB is not a full feature and won't work if you embed more than one level deep. Thus Mongoid uses indexes to update embedded documents instead of the positional operator, but I really want to remove all this code since it's overly complex and limits updating embedded documents with accuracy like you pointed out. But unfortunately I need this issue fixed before I can do it: https://jira.mongodb.org/browse/SERVER-831

Note that it's been open for coming up on 3 years in March, so I have very low confidence of it getting fixed anytime soon.

Mongoid's selectors are ready to go and have been since version 1.0: https://github.com/mongoid/mongoid/blob/master/lib/mongoid/atomic/paths/embedded.rb#L37

And I make a note of it on the website: http://mongoid.org/en/mongoid/docs/tips.html#reorder_embedded

So unfortunately it has to wait.

@durran durran closed this
@dblock

Of course, everybody's schema is different. I looked at ours and we don't have any embedded collections inside embedded documents. I would imagine that a large % of projects have the same situation, when you're doing multiple embedded levels of collections you're probably already doing something pretty complicated :)

Given my situation, I would want mongoid to change the behavior for top-level embedded documents and issue a warning when the model has collections of embedded documents below. Another possibility could be to turn this on with an option.

I got mountains of embedded collections to refactor now into top-level documents with, likely, significant performance implications. Before I do that, what do you think of any of the above?

@durran
Owner

Since we are guaranteed at least one level, then I think we can then use the positional operator for the first level then indexes for the remaining... I'm just hoping the code doesn't get too complex but should be able to do it, although that's what I was trying to avoid... :) I'll reopen.

@durran durran reopened this
@dblock

Thanks Durran! LMK if I can be of help, happy to point us to a HEAD of mongoid, including in production.

@durran durran was assigned
@durran durran referenced this issue from a commit
@durran durran Bring back the detailed update selectors.
This is part 1 of #2545.
95884bd
@durran durran closed this issue from a commit
@durran durran User $ operator where appropriate.
In the cases where the selector permits it, we now use the positional
operator on atomic updates for embedded documents nested at the first
level deep.

Documents lower than one level will continue to use indexes.

[ close #2545 ]
0bc6a69
@durran durran closed this in 0bc6a69
@arthurnn arthurnn referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
@arthurnn arthurnn referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
@arthurnn arthurnn referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
@arthurnn arthurnn referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.