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

Prevent three queries #2

Merged
merged 3 commits into from
May 14, 2021
Merged

Prevent three queries #2

merged 3 commits into from
May 14, 2021

Conversation

mightymercado
Copy link

@mightymercado mightymercado commented May 14, 2021

Logic:

  • Prevent chewy from doing a count + fetch + fetch query. Instead, do a fetch only (and just use array length. Find and count have same performance
  • If you passed a list of Mongoid::Document, we should not do a mongo query to fetch them again. There are two important conditions.
    • doc.__selected_fields has to be nil. It means that it wasn't queried with .only(*fields)
    • default_scope / indexable_criteria has to be empty. There is no builtin way to check if a Mongoid::Document satisfies a Mongoid::Criteria other than querying mongo. So I will leave it to future optimization to manually define a corresponding proc on each of our indexable_criteria functions.

Screen Shot 2021-05-14 at 12 09 04 AM

https://github.com/apolloio/leadgenie/pull/5952

@mightymercado mightymercado changed the title Prevent double query Prevent three queries May 14, 2021
@@ -30,6 +30,41 @@ def import_scope(scope, options)
end.all?
end

def import_objects(collection, options)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just a modified version of https://github.com/apolloio/chewy/blob/master/lib/chewy/type/adapter/orm.rb #import_objects

@mightymercado mightymercado merged commit 3f8ff3c into master May 14, 2021
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