rubocop / rubocop-rails

A RuboCop extension focused on enforcing Rails best practices and coding conventions.
https://docs.rubocop.org/rubocop-rails
MIT License
821 stars 263 forks source link

Modify `Rails/Pluck` to ignore `map/collect` when used inside blocks to prevent potential N+1 queries #1388

Closed masato-bkn closed 1 day ago

masato-bkn commented 1 week ago

This PR modifies the Rails/Pluck cop to ignore map/collect when used inside blocks to prevent potential N+1 queries.

When the receiver's relation is not loaded and pluck is used inside an iteration, it can result in N+1 queries. This happens because pluck executes a database query on every iteration when the relation is not loaded.

Example

users = User.all
5.times do
  users.map { |user| user[:id] } # Only one query is executed
end
users = User.all
5.times do
  users.pluck(:id) # A query is executed on every iteration
end
users = User.all
users.load

5.times do
  users.pluck(:id) # Since the records are loaded, only one query is executed
end

As shown above, replacing map/collect with pluck in such cases can degrade performance due to the N+1 queries. To prevent this, it is better to ignore map/collect when used inside an iteration.

Challenge

Since Ruby has numerous methods for performing iterations, it is difficult to list all of them. Therefore, this PR assumes that map/collect used inside blocks are likely part of an iteration and ignores offenses.


Before submitting the PR make sure the following are checked:

koic commented 1 day ago

Targeting all blocks might be too broad, but let's proceed with this for now.