From a8cf910bc756f17f7e2fbe02217e2ea4ad6ddfd6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christopher=20M=C3=BChl?= Date: Fri, 5 Jul 2024 08:37:30 +0200 Subject: [PATCH 1/2] Clarify the wording of the `Rails/PluckInWhere` cop Clarifies the existing wording to be easier to understand and adds an additional paragraph about possible performance implications of this cop. --- lib/rubocop/cop/rails/pluck_in_where.rb | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/lib/rubocop/cop/rails/pluck_in_where.rb b/lib/rubocop/cop/rails/pluck_in_where.rb index 05d0fb9715..f6c5fe29df 100644 --- a/lib/rubocop/cop/rails/pluck_in_where.rb +++ b/lib/rubocop/cop/rails/pluck_in_where.rb @@ -7,17 +7,26 @@ module Rails # and can be replaced with `select`. # # Since `pluck` is an eager method and hits the database immediately, - # using `select` helps to avoid additional database queries. + # using `select` helps to avoid additional database queries by running as + # a subquery. # - # This cop has two different enforcement modes. When the `EnforcedStyle` - # is `conservative` (the default) then only calls to `pluck` on a constant - # (i.e. a model class) in the `where` is used as offenses. + # This cop has two modes of enforcement. When the `EnforcedStyle` is set + # to `conservative` (the default), only calls to `pluck` on a constant + # (e.g. a model class) within `where` are considered offenses. # # @safety - # When the `EnforcedStyle` is `aggressive` then all calls to `pluck` in the - # `where` is used as offenses. This may lead to false positives - # as the cop cannot replace to `select` between calls to `pluck` on an - # `ActiveRecord::Relation` instance vs a call to `pluck` on an `Array` instance. + # When `EnforcedStyle` is set to `aggressive`, all calls to `pluck` + # within `where` are considered offenses. This might lead to false + # positives because the check cannot distinguish between calls to + # `pluck` on an `ActiveRecord::Relation` instance and calls to `pluck` + # on an `Array` instance. + # + # Additionally, when using a subquery with the SQL `IN` operator, + # databases like PostgreSQL and MySQL can't optimize complex queries as + # well. They need to scan all records of the outer table against the + # subquery result sequentially, rather than using an index. This can + # cause significant performance issues compared to writing the query + # differently or using `pluck`. # # @example # # bad From 6d55fb57ca5b719cf3218cd4661a539ac355ce27 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Sat, 6 Jul 2024 22:29:57 +0900 Subject: [PATCH 2/2] Suppress RuboCop offenses This commit suppresses the following new RuboCop offenses: ```console $ bundle exec rubocop (snip) rubocop-rails.gemspec:34:5: C: [Correctable] Gemspec/AddRuntimeDependency: Use add_dependency instead of add_runtime_dependency. s.add_runtime_dependency 'activesupport', '>= 4.2.0' ^^^^^^^^^^^^^^^^^^^^^^ rubocop-rails.gemspec:37:5: C: [Correctable] Gemspec/AddRuntimeDependency: Use add_dependency instead of add_runtime_dependency. s.add_runtime_dependency 'rack', '>= 1.1' ^^^^^^^^^^^^^^^^^^^^^^ rubocop-rails.gemspec:38:5: C: [Correctable] Gemspec/AddRuntimeDependency: Use add_dependency instead of add_runtime_dependency. s.add_runtime_dependency 'rubocop', '>= 1.33.0', '< 2.0' ^^^^^^^^^^^^^^^^^^^^^^ rubocop-rails.gemspec:39:5: C: [Correctable] Gemspec/AddRuntimeDependency: Use add_dependency instead of add_runtime_dependency. s.add_runtime_dependency 'rubocop-ast', '>= 1.31.1', '< 2.0' ^^^^^^^^^^^^^^^^^^^^^^ 294 files inspected, 4 offenses detected, 4 offenses autocorrectable ``` --- rubocop-rails.gemspec | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/rubocop-rails.gemspec b/rubocop-rails.gemspec index 99e503272c..243bd5ff7c 100644 --- a/rubocop-rails.gemspec +++ b/rubocop-rails.gemspec @@ -31,10 +31,10 @@ Gem::Specification.new do |s| 'rubygems_mfa_required' => 'true' } - s.add_runtime_dependency 'activesupport', '>= 4.2.0' + s.add_dependency 'activesupport', '>= 4.2.0' # Rack::Utils::SYMBOL_TO_STATUS_CODE, which is used by HttpStatus cop, was # introduced in rack 1.1 - s.add_runtime_dependency 'rack', '>= 1.1' - s.add_runtime_dependency 'rubocop', '>= 1.33.0', '< 2.0' - s.add_runtime_dependency 'rubocop-ast', '>= 1.31.1', '< 2.0' + s.add_dependency 'rack', '>= 1.1' + s.add_dependency 'rubocop', '>= 1.33.0', '< 2.0' + s.add_dependency 'rubocop-ast', '>= 1.31.1', '< 2.0' end