Skip to content

Commit

Permalink
Count returns 0 without querying if parent is not saved
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Francesco Rodriguez committed Oct 3, 2012
1 parent e4e84fe commit aa202ad
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 0 deletions.
17 changes: 17 additions & 0 deletions activerecord/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,8 @@ def sum(*args)
# 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]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -799,6 +799,12 @@ def test_count_with_counter_sql
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,12 @@ def test_counting_with_association_limit
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -767,6 +767,12 @@ def test_count_has_many_through_with_named_scope
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)

Expand Down

0 comments on commit aa202ad

Please sign in to comment.