Adds a new cop to flag the usage of pluck without a limit
requested to merge 432828-limit-the-number-of-values-allowable-an-in-clause-in-database-queries into master
What does this MR do and why?
Adds a new cop for to check the usage of .pluck
.
Matches:
-
pluck(:attribute)
is used without setting a limit.
Bad:
module Gitlab
module BackgroundMigration
class MyClass < ::Gitlab::BackgroundMigration::BatchedMigrationJob
def all
Project.where(user_id: User.pluck(:id))
end
end
end
end
Good:
module Gitlab
module BackgroundMigration
class MyClass < ::Gitlab::BackgroundMigration::BatchedMigrationJob
def all(limit)
Project.where(user_id: User.limit(limit).pluck(:id))
end
end
end
end
How to set up and validate locally
- Select a file that is not present in the
.rubocop_todo/cop/avoid_using_pluck_without_limit.yml
, likeapp/models/packages/package_file.rb
- Add a method that uses
pluck
:
def all
pluck(:id)
end
- run
bundle exec rubocop app/models/packages/package_file.rb
- The file must be in the offenses report:
Offenses:
app/models/packages/package_file.rb:140:5: C: Cop/AvoidUsingPluckWithoutLimit: Avoid using pluck without defining a limit. Plucking without a limit can cause database performance degradation.
pluck(:id)
^^^^^
1 file inspected, 1 offense detected
- Update the
all
method and add alimit
:
def all
limit(100).pluck(:id)
end
- run
bundle exec rubocop app/models/packages/package_file.rb
again. - No offenses should be raised:
Inspecting 1 file
.
1 file inspected, no offenses detected
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.
Related to #432828 (closed)
Edited by Leonardo da Rosa