Resolve N+1 queries in pub status report and index #864
Conversation
app/controllers/thesis_controller.rb
Outdated
| @@ -43,7 +43,7 @@ def publication_statuses | |||
| @terms = defined_terms Thesis.all | |||
| @publication_statuses = Thesis.all.pluck(:publication_status).uniq.sort | |||
| # Filter relevant theses by selected term from querystring | |||
| term_filtered = filter_theses_by_term Thesis.all | |||
| term_filtered = filter_theses_by_term Thesis.all.includes(:degrees).includes(:departments).includes(:users) | |||
There was a problem hiding this comment.
Is there a difference between this and .includes(:degrees, departments, users)?
There was a problem hiding this comment.
No idea! I can try it that way if it reads cleaner to you
There was a problem hiding this comment.
That definitely results in the same thing, so I'll switch to your suggested syntax change as it's likely easier to read the intent when we come back to this in the future. 👍🏻
| @@ -223,7 +223,7 @@ def data_student_contributions | |||
| row_data = {} | |||
| terms = Thesis.all.pluck(:grad_date).uniq.sort | |||
| terms.each do |term| | |||
| row_data[term] = Thesis.where('grad_date = ?', term).map(&:student_contributed?).count(true) | |||
| row_data[term] = Thesis.where('grad_date = ?', term).includes(:versions).map(&:student_contributed?).count(true) | |||
There was a problem hiding this comment.
Is the goal here to reduce the number of queries by loading only theses with papertrail versions?
There was a problem hiding this comment.
Nope. This eager loads the versions so the views don't get into an N+1 situation. Theses without versions should still load I believe (although I'm not sure if that's even a thing)
Why are these changes being introduced: * There were multiple N+1 queries that when combined with production data led to Heroku timeouts Relevant ticket(s): * https://mitlibraries.atlassian.net/browse/ETD-550 * https://mitlibraries.atlassian.net/browse/ETD-551 How does this address that need: * eager load the relevant data to eliminate the N+1 problems
JPrevost
force-pushed
the
publication_status_report
branch
from
2483f3b to
a3cb95d
Compare
Why are these changes being introduced:
data led to Heroku timeouts
Relevant ticket(s):
How does this address that need:
Developer
our guide and
all issues introduced by these changes have been resolved or opened as new
issues (link to those issues in the Pull Request details above)
Code Reviewer
(not just this pull request message)
Requires database migrations?
NO
Includes new or updated dependencies?
YES