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

Reduce number of queries #170

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Reduce number of queries #170

wants to merge 1 commit into from

Conversation

s-espinosa
Copy link
Owner

@notmarkmiranda Can you take a look at this? Believe it reduces the number of queries run in this method.

Summary

Believe this change reduces the number of queries used to get eligible projects without a vote from a user. Takes advantage of the voted_projects relationship on User.

Details

Previously we had this:

projects = Vote.where(user_id: user_id).pluck(:project_id)
eligible = ["BE Mod 3", "FE Mod 3", "BE Mod 4", "FE Mod 4"]
where.not(id: projects).where(project_type: eligible)

Which results in two queries (one when we set projects, and the other beginning with where on the third line.

Updated to the following:

eligible = ["BE Mod 3", "FE Mod 3", "BE Mod 4", "FE Mod 4"]
where.not(
  id: User.find(user_id)
    .voted_projects
    .pluck(:id)
).where(project_type: eligible)

Finding the user and then finding that user's voted projects (rather than finding a value on the join given the user_id) seems to more directly reflect what this query is trying to do.

Thought this would still run two queries, but when I run #to_sql on that method it results in a single query with a subquery in it to find the voted projects. That feels like a win to me. Can you confirm?

Questions

  1. Believe we sacrifice some legibility here for fewer queries. Is it worth it?

  2. I'm frustrated that the following doesn't work, and can't spot what I'm missing. Tried this with a .find in place of the .where as well:

eligible = ["BE Mod 3", "FE Mod 3", "BE Mod 4", "FE Mod 4"]
where.not(
  User.find(user_id)
    .voted_projects
).where(project_type: eligible)

Might not chase this too much if the updated query in the current PR results in a single query.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant