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

Find a better way to rewrite how the Mongoid::Identity mixin yields conditions #47

Closed
dblock opened this issue Jul 11, 2013 · 1 comment

Comments

@dblock
Copy link
Contributor

dblock commented Jul 11, 2013

We set Garner.config.mongoid_identity_fields = [:_id, :_slugs]. In our case, these are two separate ways to find an instance of a Model that are mutually exclusive and follow distinct formats.

Garner's Garner::Mixins::Mongoid::Identity will combine these into an $or. Here're two possible queries:

{"$or"=>[{"_id"=>"jason-bryant-hope-slash-hate"}, {"_slugs"=>"jason-bryant-hope-slash-hate"}]}

{"$or"=>[{"_id"=>"51ca3bec8b3b8132c60002ea"}, {"_slugs"=>"51ca3bec8b3b8132c60002ea"}]}

This is not very efficient and we can make the decision about whether the query is done by slug or by id upfront, so we monkey patch this as follows.

module Garner
  module Mixins
    module Mongoid
      class Identity
        private

        def self.conditions_for(klass, handle)
          # optimize away the query from an $or to a find where appropriate
          # in the case of Gravity _id and _slugs can be used independently
          # and we can never have a non BSON _id and valid BSON IDs are redundant with _id inside _slugs
          conditions = Moped::BSON::ObjectId.legal?(handle) ? { _id: handle } : { _slugs: handle }

          # _type conditions
          selector = klass.where({})
          conditions.merge!(selector.send(:type_selection)) if selector.send(:type_selectable?)

          conditions
        end
      end
    end
  end
end

This is very much custom. But maybe there's a better way to describe this in Garner without writing a completely different mixin?

@dblock
Copy link
Contributor Author

dblock commented Jul 11, 2013

This is actually a bigger deal. If you throw a sort in there, you no longer hit an index.

The $sort throws it off.

Without Sort

> db.artworks.find({"$or":[{_id:ObjectId("51c9e8ba275b249958000164")},{_slugs:"carlo-bugatti-a-rare-and-important-cartonnier"}]}).explain()
{
    "clauses" : [
        {
            "cursor" : "BtreeCursor _id_",
            "isMultiKey" : false,
            "n" : 1,
            "nscannedObjects" : 1,
            "nscanned" : 1,
            "nscannedObjectsAllPlans" : 1,
            "nscannedAllPlans" : 1,
            "scanAndOrder" : false,
            "indexOnly" : false,
            "nYields" : 0,
            "nChunkSkips" : 0,
            "millis" : 0,
            "indexBounds" : {
                "_id" : [
                    [
                        ObjectId("51c9e8ba275b249958000164"),
                        ObjectId("51c9e8ba275b249958000164")
                    ]
                ]
            }
        },
        {
            "cursor" : "BtreeCursor _slugs_1",
            "isMultiKey" : true,
            "n" : 0,
            "nscannedObjects" : 1,
            "nscanned" : 1,
            "nscannedObjectsAllPlans" : 1,
            "nscannedAllPlans" : 1,
            "scanAndOrder" : false,
            "indexOnly" : false,
            "nYields" : 0,
            "nChunkSkips" : 0,
            "millis" : 0,
            "indexBounds" : {
                "_slugs" : [
                    [
                        "carlo-bugatti-a-rare-and-important-cartonnier",
                        "carlo-bugatti-a-rare-and-important-cartonnier"
                    ]
                ]
            }
        }
    ],
    "n" : 1,
    "nscannedObjects" : 2,
    "nscanned" : 2,
    "nscannedObjectsAllPlans" : 2,
    "nscannedAllPlans" : 2,
    "millis" : 0,
    "server" : "dblock-mac.home:27017"
}

With Sort

> db.artworks.find({"$or":[{_id:ObjectId("51c9e8ba275b249958000164")},{_slugs:"carlo-bugatti-a-rare-and-important-cartonnier"}]}).sort({_id:1}).explain()
{
    "cursor" : "BtreeCursor _id_",
    "isMultiKey" : false,
    "n" : 1,
    "nscannedObjects" : 90078,
    "nscanned" : 90078,
    "nscannedObjectsAllPlans" : 90078,
    "nscannedAllPlans" : 90078,
    "scanAndOrder" : false,
    "indexOnly" : false,
    "nYields" : 0,
    "nChunkSkips" : 0,
    "millis" : 143,
    "indexBounds" : {
        "_id" : [
            [
                {
                    "$minElement" : 1
                },
                {
                    "$maxElement" : 1
                }
            ]
        ]
    },
    "server" : "dblock-mac.home:27017"
}

Ouch.

dblock added a commit to dblock/garner that referenced this issue Jul 11, 2013
@dblock dblock closed this as completed in e5ad9a2 Jul 11, 2013
fancyremarker pushed a commit that referenced this issue Jul 11, 2013
Fixed #47: use a database index when generating proxy binding in Garner::Mixins::Mongoid::Identity with multiple Garner.config.mongoid_identity_fields.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant