From d7a7a050e103def759d1e5694e5aaa35b1d03cd1 Mon Sep 17 00:00:00 2001 From: Eugene Gilburg Date: Sun, 27 Oct 2013 23:58:23 -0700 Subject: Optimize none? and one? relation query methods to use LIMIT and COUNT. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use SQL COUNT and LIMIT 1 queries for none? and one? methods if no block or limit is given, instead of loading the entire collection to memory. The any? and many? methods already follow this behavior. [Eugene Gilburg & Rafael Mendonça França] --- activerecord/CHANGELOG.md | 22 +++++++ .../associations/collection_association.rb | 6 +- activerecord/lib/active_record/null_relation.rb | 8 +++ activerecord/lib/active_record/relation.rb | 18 +++++ .../associations/has_many_associations_test.rb | 76 ++++++++++++++++++++++ activerecord/test/cases/relations_test.rb | 34 ++++++++++ 6 files changed, 162 insertions(+), 2 deletions(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 16f3f47d29..e1cace7d88 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,25 @@ +* Use SQL COUNT and LIMIT 1 queries for `none?` and `one?` methods if no block or limit is given, + instead of loading the entire collection to memory. + This applies to relations (e.g. `User.all`) as well as associations (e.g. `account.users`) + + # Before: + + users.none? + # SELECT "users".* FROM "users" + + users.one? + # SELECT "users".* FROM "users" + + # After: + + users.none? + # SELECT 1 AS one FROM "users" LIMIT 1 + + users.one? + # SELECT COUNT(*) FROM "users" + + *Eugene Gilburg* + * Allow `:precision` option for time type columns. *Ryuta Kamizono* diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index 4b7591e15c..33e4516bd1 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -319,7 +319,8 @@ module ActiveRecord end # Returns true if the collections is not empty. - # Equivalent to +!collection.empty?+. + # If block given, loads all records and checks for one or more matches. + # Otherwise, equivalent to +!collection.empty?+. def any? if block_given? load_target.any? { |*block_args| yield(*block_args) } @@ -329,7 +330,8 @@ module ActiveRecord end # Returns true if the collection has more than 1 record. - # Equivalent to +collection.size > 1+. + # If block given, loads all records and checks for two or more matches. + # Otherwise, equivalent to +collection.size > 1+. def many? if block_given? load_target.many? { |*block_args| yield(*block_args) } diff --git a/activerecord/lib/active_record/null_relation.rb b/activerecord/lib/active_record/null_relation.rb index 63ca29305d..fa6eaa7e64 100644 --- a/activerecord/lib/active_record/null_relation.rb +++ b/activerecord/lib/active_record/null_relation.rb @@ -30,10 +30,18 @@ module ActiveRecord true end + def none? + true + end + def any? false end + def one? + false + end + def many? false end diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 8073eb99dd..6bbec7c0c0 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -271,6 +271,15 @@ module ActiveRecord end end + # Returns true if there are no records. + def none? + if block_given? + to_a.none? { |*block_args| yield(*block_args) } + else + empty? + end + end + # Returns true if there are any records. def any? if block_given? @@ -280,6 +289,15 @@ module ActiveRecord end end + # Returns true if there is exactly one record. + def one? + if block_given? + to_a.one? { |*block_args| yield(*block_args) } + else + limit_value ? to_a.one? : size == 1 + end + end + # Returns true if there is more than one record. def many? if block_given? diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 19f4384e83..fde7b0ea3a 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -1678,6 +1678,82 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal 3, firm.clients.size end + def test_calling_none_should_count_instead_of_loading_association + firm = companies(:first_firm) + assert_queries(1) do + firm.clients.none? # use count query + end + assert !firm.clients.loaded? + end + + def test_calling_none_on_loaded_association_should_not_use_query + firm = companies(:first_firm) + firm.clients.collect # force load + assert_no_queries { assert ! firm.clients.none? } + end + + def test_calling_none_should_defer_to_collection_if_using_a_block + firm = companies(:first_firm) + assert_queries(1) do + firm.clients.expects(:size).never + firm.clients.none? { true } + end + assert firm.clients.loaded? + end + + def test_calling_none_should_return_true_if_none + firm = companies(:another_firm) + assert firm.clients_like_ms.none? + assert_equal 0, firm.clients_like_ms.size + end + + def test_calling_none_should_return_false_if_any + firm = companies(:first_firm) + assert !firm.limited_clients.none? + assert_equal 1, firm.limited_clients.size + end + + def test_calling_one_should_count_instead_of_loading_association + firm = companies(:first_firm) + assert_queries(1) do + firm.clients.one? # use count query + end + assert !firm.clients.loaded? + end + + def test_calling_one_on_loaded_association_should_not_use_query + firm = companies(:first_firm) + firm.clients.collect # force load + assert_no_queries { assert ! firm.clients.one? } + end + + def test_calling_one_should_defer_to_collection_if_using_a_block + firm = companies(:first_firm) + assert_queries(1) do + firm.clients.expects(:size).never + firm.clients.one? { true } + end + assert firm.clients.loaded? + end + + def test_calling_one_should_return_false_if_zero + firm = companies(:another_firm) + assert ! firm.clients_like_ms.one? + assert_equal 0, firm.clients_like_ms.size + end + + def test_calling_one_should_return_true_if_one + firm = companies(:first_firm) + assert firm.limited_clients.one? + assert_equal 1, firm.limited_clients.size + end + + def test_calling_one_should_return_false_if_more_than_one + firm = companies(:first_firm) + assert ! firm.clients.one? + assert_equal 3, firm.clients.size + end + def test_joins_with_namespaced_model_should_use_correct_type old = ActiveRecord::Base.store_full_sti_class ActiveRecord::Base.store_full_sti_class = true diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 5c5ab499a0..dec3a37f42 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -342,7 +342,9 @@ class RelationTest < ActiveRecord::TestCase assert_equal 0, Developer.none.size assert_equal 0, Developer.none.count assert_equal true, Developer.none.empty? + assert_equal true, Developer.none.none? assert_equal false, Developer.none.any? + assert_equal false, Developer.none.one? assert_equal false, Developer.none.many? end end @@ -1102,6 +1104,38 @@ class RelationTest < ActiveRecord::TestCase assert ! posts.limit(1).many? end + def test_none? + posts = Post.all + assert_queries(1) do + assert ! posts.none? # Uses COUNT() + end + + assert ! posts.loaded? + + assert_queries(1) do + assert posts.none? {|p| p.id < 0 } + assert ! posts.none? {|p| p.id == 1 } + end + + assert posts.loaded? + end + + def test_one + posts = Post.all + assert_queries(1) do + assert ! posts.one? # Uses COUNT() + end + + assert ! posts.loaded? + + assert_queries(1) do + assert ! posts.one? {|p| p.id < 3 } + assert posts.one? {|p| p.id == 1 } + end + + assert posts.loaded? + end + def test_build posts = Post.all -- cgit v1.2.3