Document best practice around database queries in backend.md #11205
Conversation
[skip changelog]
|
|
||
| ```ruby | ||
| # Bad: potentially-unnecessary DB query | ||
| if mfa_user.two_factor_enabled? && !in_mfa_selection_flow |
There was a problem hiding this comment.
I think this example relies on specific knowledge of the MfaUser class which itself wraps an ActiveRecord object.
Maybe we could use some example illustrative names?
| if mfa_user.two_factor_enabled? && !in_mfa_selection_flow | |
| if model.association.count > 2 && !local_boolean |
and then it might be clearer?
There was a problem hiding this comment.
I debated this a bit, but you're probably right.
Part of what I wanted to try to convey is that two_factor_enabled? doesn't tell you it's going to do a database query; you need to pay some attention to what the methods are doing. But maybe this is too obtuse an example.
| In this example, the concern is specifically that the `two_factor_enabled?` method | ||
| issues a database query, fetching an association. | ||
|
|
||
| An alternative approach to consider when dealing with associations is to make use | ||
| of [eager loading](https://guides.rubyonrails.org/active_record_querying.html#eager-loading-associations), | ||
| which could allow fetching the associated records with a join in the same query. |
There was a problem hiding this comment.
Again I think it's pretty specific... maybe we could show an example of something we want to eager load:
# BAD
recent_users = User.last(10)
recent_emails = recent_users.flat_map { |u| u.email_addresses.email } # one EmailAddress query per user!
# GOOD
recent_users = User.includes(:email_addresses).last(10)
recent_emails = recent_users.flat_map { |u| u.email_addresses.email } # one EmailAddress query total!There was a problem hiding this comment.
Nice; an example of actually using it is a good idea.
This is another case where I probably would have used #collect
🎫 Ticket
N/A
🛠 Summary of changes
This tries to capture the discussion in #11197 around documenting best practices for avoiding unnecessary database queries.
Relentless critique welcome on this.