From 46a64388d283cce2e86d38f23fde8999080ebc19 Mon Sep 17 00:00:00 2001 From: miloops Date: Sat, 30 Aug 2008 19:17:29 -0300 Subject: [PATCH] Performance: Better query for ASSOCIATION_ids. Select only ids if the association hasn't been loaded. --- activerecord/lib/active_record/associations.rb | 6 +++- .../has_and_belongs_to_many_associations_test.rb | 34 ++++++++++++++----- .../associations/has_many_associations_test.rb | 30 +++++++++++++---- .../has_many_through_associations_test.rb | 20 +++++++++++ 4 files changed, 73 insertions(+), 17 deletions(-) diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 3ca93db..62cc414 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1304,7 +1304,11 @@ module ActiveRecord end define_method("#{reflection.name.to_s.singularize}_ids") do - send(reflection.name).map { |record| record.id } + if send(reflection.name).loaded? + send(reflection.name).map(&:id) + else + send(reflection.name).all(:select => "#{reflection.quoted_table_name}.{reflection.primary_key}").map(&:id) + end end end 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 1b31d28..edca3c6 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 17fd88c..feac4b0 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 353262c..18ac992 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 -- 1.5.5.1