Performance: Better query for ASSOCIATION_ids. Select only ids if the association hasn't been loaded.

Signed-off-by: Jeremy Kemper <jeremy@bitsweat.net>

3 files changed, 68 insertions, 16 deletions

diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
index 1b31d28679..edca3c622b 100644
--- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
@@ -223,10 +223,10 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase
     devel = Developer.find(1)
     proj = assert_no_queries { devel.projects.build("name" => "Projekt") }
     assert !devel.projects.loaded?
-    
+
     assert_equal devel.projects.last, proj
     assert devel.projects.loaded?
-    
+
     assert proj.new_record?
     devel.save
     assert !proj.new_record?
@@ -251,10 +251,10 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase
     devel = Developer.find(1)
     proj = devel.projects.create("name" => "Projekt")
     assert !devel.projects.loaded?
-    
+
     assert_equal devel.projects.last, proj
     assert devel.projects.loaded?
-    
+
     assert !proj.new_record?
     assert_equal Developer.find(1).projects.sort_by(&:id).last, proj  # prove join table is updated
   end
@@ -274,10 +274,10 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase
 
   def test_creation_respects_hash_condition
     post = categories(:general).post_with_conditions.build(:body => '')
-    
+
     assert        post.save
     assert_equal  'Yet Another Testing Title', post.title
-    
+
     another_post = categories(:general).post_with_conditions.create(:body => '')
 
     assert        !another_post.new_record?
@@ -288,7 +288,7 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase
     dev = developers(:jamis)
     dev.projects << projects(:active_record)
     dev.projects << projects(:active_record)
-    
+
     assert_equal 3, dev.projects.size
     assert_equal 1, dev.projects.uniq.size
   end
@@ -415,13 +415,13 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase
     project.developers.class # force load target
 
     developer = project.developers.first
-    
+
     assert_no_queries do
       assert project.developers.loaded?
       assert project.developers.include?(developer)
     end
   end
-  
+
   def test_include_checks_if_record_exists_if_target_not_loaded
     project = projects(:active_record)
     developer = project.developers.first
@@ -641,6 +641,22 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase
     assert_equal [projects(:active_record).id], developers(:jamis).project_ids
   end
 
+  def test_get_ids_for_loaded_associations
+    developer = developers(:david)
+    developer.projects(true)
+    assert_queries(0) do
+      developer.project_ids
+      developer.project_ids
+    end
+  end
+
+  def test_get_ids_for_unloaded_associations_does_not_load_them
+    developer = developers(:david)
+    assert !developer.projects.loaded?
+    assert_equal projects(:active_record, :action_controller).map(&:id).sort, developer.project_ids.sort
+    assert !developer.projects.loaded?
+  end
+
   def test_assign_ids
     developer = Developer.new("name" => "Joe")
     developer.project_ids = projects(:active_record, :action_controller).map(&:id)
diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb
index 17fd88ce65..feac4b002b 100644
--- a/activerecord/test/cases/associations/has_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_associations_test.rb
@@ -384,7 +384,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
     company = companies(:first_firm)
     new_client = assert_no_queries { company.clients_of_firm.build("name" => "Another Client") }
     assert !company.clients_of_firm.loaded?
-    
+
     assert_equal "Another Client", new_client.name
     assert new_client.new_record?
     assert_equal new_client, company.clients_of_firm.last
@@ -416,7 +416,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
   def test_build_many
     company = companies(:first_firm)
     new_clients = assert_no_queries { company.clients_of_firm.build([{"name" => "Another Client"}, {"name" => "Another Client II"}]) }
-    
+
     assert_equal 2, new_clients.size
     company.name += '-changed'
     assert_queries(3) { assert company.save }
@@ -655,10 +655,10 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
 
   def test_creation_respects_hash_condition
     ms_client = companies(:first_firm).clients_like_ms_with_hash_conditions.build
-    
+
     assert        ms_client.save
     assert_equal  'Microsoft', ms_client.name
-    
+
     another_ms_client = companies(:first_firm).clients_like_ms_with_hash_conditions.create
 
     assert        !another_ms_client.new_record?
@@ -830,6 +830,22 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
     assert_equal [companies(:first_client).id, companies(:second_client).id], companies(:first_firm).client_ids
   end
 
+  def test_get_ids_for_loaded_associations
+    company = companies(:first_firm)
+    company.clients(true)
+    assert_queries(0) do
+      company.client_ids
+      company.client_ids
+    end
+  end
+
+  def test_get_ids_for_unloaded_associations_does_not_load_them
+    company = companies(:first_firm)
+    assert !company.clients.loaded?
+    assert_equal [companies(:first_client).id, companies(:second_client).id], company.client_ids
+    assert !company.clients.loaded?
+  end
+
   def test_assign_ids
     firm = Firm.new("name" => "Apple")
     firm.client_ids = [companies(:first_client).id, companies(:second_client).id]
@@ -900,7 +916,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
     assert_equal 4, authors(:david).limited_comments.find(:all, :conditions => "comments.type = 'SpecialComment'", :limit => 9_000).length
     assert_equal 4, authors(:david).limited_comments.find_all_by_type('SpecialComment', :limit => 9_000).length
   end
-  
+
   def test_find_all_include_over_the_same_table_for_through
     assert_equal 2, people(:michael).posts.find(:all, :include => :people).length
   end
@@ -937,13 +953,13 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
   def test_include_loads_collection_if_target_uses_finder_sql
     firm = companies(:first_firm)
     client = firm.clients_using_sql.first
-    
+
     firm.reload
     assert ! firm.clients_using_sql.loaded?
     assert firm.clients_using_sql.include?(client)
     assert firm.clients_using_sql.loaded?
   end
-  
+
 
   def test_include_returns_false_for_non_matching_record_to_verify_scoping
     firm = companies(:first_firm)
diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb
index 353262c81b..3e3c7a95b7 100644
--- a/activerecord/test/cases/associations/has_many_through_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb
@@ -200,4 +200,24 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase
   def test_count_with_include_should_alias_join_table
     assert_equal 2, people(:michael).posts.count(:include => :readers)
   end
+
+  def test_get_ids
+    assert_equal [posts(:welcome).id, posts(:authorless).id], people(:michael).post_ids
+  end
+
+  def test_get_ids_for_loaded_associations
+    person = people(:michael)
+    person.posts(true)
+    assert_queries(0) do
+      person.post_ids
+      person.post_ids
+    end
+  end
+
+  def test_get_ids_for_unloaded_associations_does_not_load_them
+    person = people(:michael)
+    assert !person.posts.loaded?
+    assert_equal [posts(:welcome).id, posts(:authorless).id], person.post_ids
+    assert !person.posts.loaded?
+  end
 end