From aa202adf6c78468c8e03efd11d84b71478de7b03 Mon Sep 17 00:00:00 2001 From: Francesco Rodriguez Date: Wed, 3 Oct 2012 18:02:14 -0500 Subject: Count returns 0 without querying if parent is not saved Patches `CollectionAssociation#count` to return 0 without querying if the parent record is new. Consider the following code: class Account has_many :dossiers end class Dossier belongs_to :account end a = Account.new a.dossiers.build # before patch a.dossiers.count # SELECT COUNT(*) FROM "dossiers" WHERE "dossiers"."account_id" IS NULL # => 0 # after a.dosiers.count # fires without sql query # => 0 Fixes #1856. --- activerecord/CHANGELOG.md | 17 +++++++++++++++++ .../associations/collection_association.rb | 2 ++ .../has_and_belongs_to_many_associations_test.rb | 6 ++++++ .../cases/associations/has_many_associations_test.rb | 6 ++++++ .../associations/has_many_through_associations_test.rb | 6 ++++++ 5 files changed, 37 insertions(+) (limited to 'activerecord') diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 8ff4c4706c..2a0f1bf92a 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,5 +1,22 @@ ## Rails 4.0.0 (unreleased) ## +* `CollectionAssociation#count` returns 0 without querying if the + parent record is new. + + Before: + + person.pets + # SELECT COUNT(*) FROM "pets" WHERE "pets"."person_id" IS NULL + # => 0 + + After: + + person.pets + # fires without sql query + # => 0 + + *Francesco Rodriguez* + * Fix `reset_counters` crashing on `has_many :through` associations. Fix #7822. diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index fe3e5b00f7..96270ec0e9 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -174,6 +174,8 @@ module ActiveRecord # association, it will be used for the query. Otherwise, construct options and pass them with # scope to the target class's +count+. def count(column_name = nil, count_options = {}) + return 0 if owner.new_record? + column_name, count_options = nil, column_name if column_name.is_a?(Hash) if options[:counter_sql] || options[:finder_sql] 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 f3520d43e0..42f5b69d4e 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 @@ -799,6 +799,12 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase assert_equal 1, developer.projects.count end + def test_counting_should_not_fire_sql_if_parent_is_unsaved + assert_no_queries do + assert_equal 0, Developer.new.projects.count + end + end + unless current_adapter?(:PostgreSQLAdapter) def test_count_with_finder_sql assert_equal 3, projects(:active_record).developers_with_finder_sql.count diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 4b56037a08..50c23c863f 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -262,6 +262,12 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal firm.limited_clients.length, firm.limited_clients.count end + def test_counting_should_not_fire_sql_if_parent_is_unsaved + assert_no_queries do + assert_equal 0, Person.new.readers.count + end + end + def test_finding assert_equal 2, Firm.all.merge!(:order => "id").first.clients.length end 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 d4ceae6f80..f0582a3090 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -767,6 +767,12 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase assert_equal 1, authors(:mary).categories.general.count end + def test_counting_should_not_fire_sql_if_parent_is_unsaved + assert_no_queries do + assert_equal 0, Person.new.posts.count + end + end + def test_has_many_through_belongs_to_should_update_when_the_through_foreign_key_changes post = posts(:eager_other) -- cgit v1.2.3