Created
July 23, 2008 12:03
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
From 7f49582ac214d20258dccd222e360350591e1cce Mon Sep 17 00:00:00 2001 | |
From: =?utf-8?q?Tarmo=20T=C3=A4nav?= <tarmo@itech.ee> | |
Date: Wed, 23 Jul 2008 14:57:42 +0300 | |
Subject: [PATCH] has_many association and named_scope empty?() uses exists?({}) instead of counting. | |
This way the database only has to find one match using an index | |
instead of counting all the matches. If the association or scope | |
is already loaded or the association has a custom counter_sql | |
size check is used instead. | |
--- | |
.../associations/association_collection.rb | 6 +++++- | |
activerecord/lib/active_record/named_scope.rb | 2 +- | |
.../associations/has_many_associations_test.rb | 16 ++++++++++++++++ | |
activerecord/test/cases/named_scope_test.rb | 11 +++++++++++ | |
4 files changed, 33 insertions(+), 2 deletions(-) | |
diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb | |
index a28be9e..9c44eb9 100644 | |
--- a/activerecord/lib/active_record/associations/association_collection.rb | |
+++ b/activerecord/lib/active_record/associations/association_collection.rb | |
@@ -206,7 +206,11 @@ module ActiveRecord | |
end | |
def empty? | |
- size.zero? | |
+ if loaded? || @reflection.options[:counter_sql] | |
+ size.zero? | |
+ else | |
+ !exists?({}) | |
+ end | |
end | |
def any? | |
diff --git a/activerecord/lib/active_record/named_scope.rb b/activerecord/lib/active_record/named_scope.rb | |
index d5a1c5f..ab842b4 100644 | |
--- a/activerecord/lib/active_record/named_scope.rb | |
+++ b/activerecord/lib/active_record/named_scope.rb | |
@@ -137,7 +137,7 @@ module ActiveRecord | |
end | |
def empty? | |
- @found ? @found.empty? : count.zero? | |
+ @found ? @found.empty? : !exists?({}) | |
end | |
def any? | |
diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb | |
index f8b8b1f..899e20d 100644 | |
--- a/activerecord/test/cases/associations/has_many_associations_test.rb | |
+++ b/activerecord/test/cases/associations/has_many_associations_test.rb | |
@@ -1017,4 +1017,20 @@ class HasManyAssociationsTest < ActiveRecord::TestCase | |
ActiveRecord::Base.store_full_sti_class = old | |
end | |
+ def test_empty_with_counter_sql | |
+ company = Firm.find(:first) | |
+ assert_sql /COUNT/i do | |
+ assert_equal false, company.clients.empty? | |
+ end | |
+ end | |
+ | |
+ def test_empty? | |
+ company = Firm.find(:first) | |
+ assert_queries(1) do | |
+ assert_equal false, company.plain_clients.empty? | |
+ end | |
+ | |
+ # should have used LIMIT, but because of different SQL adapters we can't check for that here. | |
+ assert_no_match /COUNT/i, $queries_executed.first | |
+ end | |
end | |
diff --git a/activerecord/test/cases/named_scope_test.rb b/activerecord/test/cases/named_scope_test.rb | |
index e21ffbb..fd0b09d 100644 | |
--- a/activerecord/test/cases/named_scope_test.rb | |
+++ b/activerecord/test/cases/named_scope_test.rb | |
@@ -231,4 +231,15 @@ class NamedScopeTest < ActiveRecord::TestCase | |
assert topic.approved | |
assert_equal 'lifo', topic.author_name | |
end | |
+ | |
+ def test_empty_should_use_exists_when_not_loaded | |
+ ActiveRecord::NamedScope::Scope.any_instance.expects(:exists?).with({}).returns(true) | |
+ assert_equal false, Topic.approved.empty? | |
+ end | |
+ | |
+ def test_empty_should_not_use_exists_when_loaded | |
+ Topic.approved.to_a # load the association | |
+ Topic.approved.expects(:exists?).never | |
+ assert_equal false, Topic.approved.empty? | |
+ end | |
end | |
-- | |
1.5.6.3 | |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment