From 8875e28a50b117aa862c8563c49f7e3a6ee7deff Mon Sep 17 00:00:00 2001 From: Lann Martin Date: Wed, 4 Sep 2013 16:47:28 -0700 Subject: Make CollectionAssociation first/last with integer fetch with query When first or last is called with an integer on an unloaded association, the entire collection is loaded. This differs surprisingly from the behavior of Relation#first/last, which translate the call into a limit query. For large collections this can make a big difference in performance. Change CollectionAssociation#fetch_first_or_last_using_find? to make this kind of call delegate to Relation. --- activerecord/CHANGELOG.md | 5 +++++ .../lib/active_record/associations/collection_association.rb | 4 +--- .../test/cases/associations/has_many_associations_test.rb | 8 +++++--- 3 files changed, 11 insertions(+), 6 deletions(-) (limited to 'activerecord') diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index b79b9a87c6..18a37d4bdf 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,8 @@ +* `CollectionAssociation#first`/`#last` (e.g. `has_many`) use a `LIMIT`ed + query to fetch results rather than loading the entire collection. + + *Lann Martin* + * Re-use `order` argument pre-processing for `reorder`. *Paul Nikitochkin* diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index 228c500f0a..0c326ed747 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -528,15 +528,13 @@ module ActiveRecord # * target already loaded # * owner is new record # * target contains new or changed record(s) - # * the first arg is an integer (which indicates the number of records to be returned) def fetch_first_or_last_using_find?(args) if args.first.is_a?(Hash) true else !(loaded? || owner.new_record? || - target.any? { |record| record.new_record? || record.changed? } || - args.first.kind_of?(Integer)) + target.any? { |record| record.new_record? || record.changed? }) end end diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 4c0fa88917..ebeead0dc2 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -1414,15 +1414,17 @@ class HasManyAssociationsTest < ActiveRecord::TestCase end end - def test_calling_first_or_last_with_integer_on_association_should_load_association + def test_calling_first_or_last_with_integer_on_association_should_not_load_association firm = companies(:first_firm) + firm.clients.create(:name => 'Foo') + assert !firm.clients.loaded? - assert_queries 1 do + assert_queries 2 do firm.clients.first(2) firm.clients.last(2) end - assert firm.clients.loaded? + assert !firm.clients.loaded? end def test_calling_many_should_count_instead_of_loading_association -- cgit v1.2.3