Skip to content

Adds a new cop to flag the usage of pluck without a limit

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

  1. Select a file that is not present in the .rubocop_todo/cop/avoid_using_pluck_without_limit.yml, like app/models/packages/package_file.rb
  2. Add a method that uses pluck:
def all
  pluck(:id)
end
  1. run bundle exec rubocop app/models/packages/package_file.rb
  2. 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
  1. Update the all method and add a limit:
def all
  limit(100).pluck(:id)
end
  1. run bundle exec rubocop app/models/packages/package_file.rb again.
  2. 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.

Related to #432828 (closed)

Edited by Leonardo da Rosa

Merge request reports

Loading