From b3caf0c99aea48f6738777f722863159058ec3be Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Tue, 4 May 2021 16:34:42 +0200 Subject: [PATCH] Improve performance of #one? and #many? #one? and #many? generate a query like `SELECT COUNT(*) FROM posts` under the hood, and then compare if the result is equal (or greater) to 1. That count operation can be really slow for large tables or complex conditions, but there's no need to count all the records in these cases. It's much faster just by adding a limit, like `SELECT COUNT(*) FROM posts LIMIT 2` --- activerecord/CHANGELOG.md | 4 ++++ activerecord/lib/active_record/relation.rb | 12 ++++++++++-- .../associations/has_many_associations_test.rb | 16 ++++++++++++++++ 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 34c19b924a..34357dca35 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,7 @@ +* Improve performance of `one?` and `many?` by limiting the generated count query to 2 results. + + *Gonzalo Riestra* + * Don't check type when using `if_not_exists` on `add_column`. Previously, if a migration called `add_column` with the `if_not_exists` option set to true diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index cd11cd71d5..ae7112fc10 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -33,6 +33,7 @@ module ActiveRecord @delegate_to_klass = false @future_result = nil @records = nil + @limited_count = nil end def initialize_copy(other) @@ -294,13 +295,15 @@ module ActiveRecord # Returns true if there is exactly one record. def one? return super if block_given? - limit_value ? records.one? : size == 1 + return records.one? if limit_value || loaded? + limited_count == 1 end # Returns true if there is more than one record. def many? return super if block_given? - limit_value ? records.many? : size > 1 + return records.many? if limit_value || loaded? + limited_count > 1 end # Returns a stable cache key that can be used to identify this query. @@ -690,6 +693,7 @@ module ActiveRecord @offsets = @take = nil @cache_keys = nil @records = nil + @limited_count = nil self end @@ -964,5 +968,9 @@ module ActiveRecord # ignore raw_sql_ that is used by Oracle adapter as alias for limit/offset subqueries string.scan(/[a-zA-Z_][.\w]+(?=.?\.)/).map!(&:downcase) - ["raw_sql_"] end + + def limited_count + @limited_count ||= limit(2).count + end end end diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 963976c733..b0c9f3e6d4 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -2278,6 +2278,14 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_no_queries { assert firm.clients.many? } end + def test_subsequent_calls_to_many_should_not_use_query + firm = companies(:first_firm) + assert_queries(1) do + firm.clients.many? + firm.clients.many? + end + end + def test_calling_many_should_defer_to_collection_if_using_a_block firm = companies(:first_firm) assert_queries(1) do @@ -2354,6 +2362,14 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_no_queries { assert_not firm.clients.one? } end + def test_subsequent_calls_to_one_should_not_use_query + firm = companies(:first_firm) + assert_queries(1) do + firm.clients.one? + firm.clients.one? + end + end + def test_calling_one_should_defer_to_collection_if_using_a_block firm = companies(:first_firm) assert_queries(1) do