Skip to content

Do not use .pluck on .select #1435

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
blnoonan opened this issue Feb 6, 2025 · 0 comments
Open

Do not use .pluck on .select #1435

blnoonan opened this issue Feb 6, 2025 · 0 comments

Comments

@blnoonan
Copy link

blnoonan commented Feb 6, 2025

Is your feature request related to a problem? Please describe.

Do not use .pluck on .select.

  • .select returns an ActiveRecord relation with only the selected column(s) marked for retrieval
  • .pluck returns an array of column values

Using them together is at best redundant and at worst confusing and inefficient.
When chained with .select, .pluck is unaware of any directive passed to .select
(e.g. column aliases or a DISTINCT clause). This can lead to unexpected behavior.

Example: simple select + pluck

# before
User.select(:id).pluck(:id)

# after
User.pluck(:id)

The .select is redundant. Use either .select or .pluck on its own.

Example: select with alias

# before
User.select('id, email AS user_email').pluck('id', 'user_email')

# after
User.pluck(:id, :email)

# after
User.select(:id, 'email AS user_email')

.pluck is unaware of the alias created by .select and will raise an "Unknown column" error.
If you need the alias, use .select on its own. Otherwise, consider using .pluck on its own.

Example: select with DISTINCT string

# before
User.select('DISTINCT email').pluck(:email)

# after
User.group(:email).pluck(:email)

# after
User.distinct.pluck(:email)

# after
User.distinct.select(:email)

.pluck is unaware of .select's DISTINCT directive and will load all User emails from the
database - including duplicates. Use either .select or .pluck on its own with .distinct,
or use .group (which can be more efficient).

Example: select with .distinct

# before
User.select(:company_id).distinct.pluck(:company_id)

# after
User.group(:company_id).pluck(:company_id)

# after
User.distinct.pluck(:company_id)

# after
User.distinct.select(:company_id)

The .select is redundant. Use either .select or .pluck on its own with .distinct,
or use .group (which can be more efficient).

Example: select with block

# before
User.select(&:active?).pluck(:id)

# after
User.where(active: true).pluck(:id)

# after (caution - potentially memory-intensive)
User.select(&:active?).map(&:id)

# after (caution - potentially memory-intensive)
User.filter(&:active?).map(&:id)

.select and .pluck make this statement look like an ActiveRecord operation, but under the hood
.select is loading all Users into memory before filtering them using the active? method in Ruby.
Use .where to avoid loading all Users into memory, or use .filter or .map to make it
clear to readers this is not an ActiveRecord operation.

Note: This one may be the most controversial case and could therefore optionally be covered.

Describe the solution you'd like

Add a rule to disallow using pluck on select. Optionally apply the rule to the block case.

Describe alternatives you've considered

  • Autocorrect .select("RAW").pluck to .select("RAW").to_a.pluck - technically works, but not always desired
  • Disallow this behavior in rails (appears to be a no-go)
@blnoonan blnoonan changed the title Disallow chaining manual .selects + .pluck Do not use .pluck on .select Feb 24, 2025
@blnoonan blnoonan changed the title Do not use .pluck on .select Do not use .pluck on manual .select Feb 24, 2025
@blnoonan blnoonan changed the title Do not use .pluck on manual .select Do not use .pluck on .select Mar 12, 2025
blnoonan added a commit to blnoonan/rubocop-rails that referenced this issue Mar 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant