From c01c21b31d590f7e8d12e3ae083fcdf0f0c6fd54 Mon Sep 17 00:00:00 2001 From: Emilio Tagua Date: Tue, 1 Sep 2009 15:36:09 -0300 Subject: [PATCH 1/3] Added association preload to relation. --- activerecord/lib/active_record/base.rb | 23 +++++++++++++--- activerecord/lib/active_record/relation.rb | 8 ++++++ activerecord/test/cases/relations_test.rb | 31 +++++++++++++++++++++- 3 files changed, 58 insertions(+), 4 deletions(-) diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 402d68c36e..1c12189e15 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -664,11 +664,28 @@ module ActiveRecord #:nodoc: # This is an alias for find(:all). You can pass in all the same arguments to this method as you can # to find(:all) def all(*args) - if args.empty? && !scoped?(:find) - arel_table + options = args.extract_options! + + + if options.empty? #&& !scoped?(:find) + relation = arel_table else - construct_finder_arel(*args) + include_associations = merge_includes(scope(:find, :include), options[:include]) + + # if include_associations.any? && references_eager_loaded_tables?(options) + # join_dependency = JoinDependency.new(self, include_associations, options[:joins]) + + # relation = construct_finder_arel_with_included_associations(options, join_dependency) + + # relation.preload(include_associations) + # else + relation = construct_finder_arel(options) + if include_associations.any? + relation.preload(include_associations) + # end + end end + relation end # Executes a custom SQL query against your database and returns all the results. The results will diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 4b53857d36..6abb2df8ff 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -6,6 +6,12 @@ module ActiveRecord def initialize(klass, relation) @klass, @relation = klass, relation @readonly = false + @associations_to_preload = [] + end + + def preload(association) + @associations_to_preload << association + @associations_to_preload.flatten! end def readonly @@ -16,6 +22,8 @@ module ActiveRecord def to_a records = @klass.find_by_sql(@relation.to_sql) + @klass.send :preload_associations, records, @associations_to_preload unless @associations_to_preload.empty? + records.each { |record| record.readonly! } if @readonly records diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 655eb3314d..6fb505ca36 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -1,6 +1,7 @@ require "cases/helper" require 'models/post' require 'models/topic' +require 'models/comment' require 'models/reply' require 'models/author' require 'models/entrant' @@ -8,7 +9,7 @@ require 'models/developer' require 'models/company' class RelationTest < ActiveRecord::TestCase - fixtures :authors, :topics, :entrants, :developers, :companies, :developers_projects, :accounts, :categories, :categorizations, :posts + fixtures :authors, :topics, :entrants, :developers, :companies, :developers_projects, :accounts, :categories, :categorizations, :posts, :comments def test_finding_with_conditions assert_equal Author.find(:all, :conditions => "name = 'David'"), Author.all.conditions("name = 'David'").to_a @@ -85,5 +86,33 @@ class RelationTest < ActiveRecord::TestCase Developer.all.readonly.each { |d| assert d.readonly? } Developer.all(:readonly => true).each { |d| assert d.readonly? } end + + def test_eager_association_loading_of_stis_with_multiple_references + authors = Author.all(:include => { :posts => { :special_comments => { :post => [ :special_comments, :very_special_comment ] } } }, :order => 'comments.body, very_special_comments_posts.body', :conditions => 'posts.id = 4').to_a + assert_equal [authors(:david)], authors + assert_no_queries do + authors.first.posts.first.special_comments.first.post.special_comments + authors.first.posts.first.special_comments.first.post.very_special_comment + end + end + + def test_find_with_included_associations + assert_queries(2) do + posts = Post.find(:all, :include => :comments) + posts.first.comments.first + end + assert_queries(2) do + posts = Post.all(:include => :comments).to_a + posts.first.comments.first + end + assert_queries(2) do + posts = Post.find(:all, :include => :author) + posts.first.author + end + assert_queries(2) do + posts = Post.all(:include => :author).to_a + posts.first.author + end + end end From 3747f896a1b727d67e6022001007e5f58b24a267 Mon Sep 17 00:00:00 2001 From: Emilio Tagua Date: Mon, 5 Oct 2009 14:39:20 -0300 Subject: [PATCH 2/3] Moved relation's test to relation_test. --- activerecord/lib/active_record/base.rb | 3 +-- activerecord/test/cases/method_scoping_test.rb | 4 ++-- activerecord/test/cases/relations_test.rb | 11 +++++++++++ arel | 2 +- 4 files changed, 15 insertions(+), 5 deletions(-) diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 1c12189e15..2fd3384877 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -667,7 +667,7 @@ module ActiveRecord #:nodoc: options = args.extract_options! - if options.empty? #&& !scoped?(:find) + if options.empty? && !scoped?(:find) relation = arel_table else include_associations = merge_includes(scope(:find, :include), options[:include]) @@ -1751,7 +1751,6 @@ module ActiveRecord #:nodoc: relation = relation.readonly if options[:readonly] relation - end def construct_finder_sql(options, scope = scope(:find)) diff --git a/activerecord/test/cases/method_scoping_test.rb b/activerecord/test/cases/method_scoping_test.rb index 6dec474f7d..eb4ce0e774 100644 --- a/activerecord/test/cases/method_scoping_test.rb +++ b/activerecord/test/cases/method_scoping_test.rb @@ -593,12 +593,12 @@ class DefaultScopingTest < ActiveRecord::TestCase end def test_default_scope_with_conditions_string - assert_equal Developer.find_all_by_name('David').map(&:id).sort, DeveloperCalledDavid.all.to_a.map(&:id).sort + assert_equal Developer.find_all_by_name('David').map(&:id).sort, DeveloperCalledDavid.find(:all).map(&:id).sort assert_equal nil, DeveloperCalledDavid.create!.name end def test_default_scope_with_conditions_hash - assert_equal Developer.find_all_by_name('Jamis').map(&:id).sort, DeveloperCalledJamis.all.to_a.map(&:id).sort + assert_equal Developer.find_all_by_name('Jamis').map(&:id).sort, DeveloperCalledJamis.find(:all).map(&:id).sort assert_equal 'Jamis', DeveloperCalledJamis.create!.name end diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 6fb505ca36..17c228616b 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -114,5 +114,16 @@ class RelationTest < ActiveRecord::TestCase posts.first.author end end + + def test_default_scope_with_conditions_string + assert_equal Developer.find_all_by_name('David').map(&:id).sort, DeveloperCalledDavid.all.to_a.map(&:id).sort + assert_equal nil, DeveloperCalledDavid.create!.name + end + + def test_default_scope_with_conditions_hash + assert_equal Developer.find_all_by_name('Jamis').map(&:id).sort, DeveloperCalledJamis.all.to_a.map(&:id).sort + assert_equal 'Jamis', DeveloperCalledJamis.create!.name + end + end diff --git a/arel b/arel index 755a7ced2f..8852db7087 160000 --- a/arel +++ b/arel @@ -1 +1 @@ -Subproject commit 755a7ced2f98b0bb246089c80cdfa04cd918fa89 +Subproject commit 8852db7087a8f4f98e5fd26fa33bac14a5400979 From 65f055a3ed790d41aeca8d4ca7f3771b05cf544f Mon Sep 17 00:00:00 2001 From: Emilio Tagua Date: Mon, 5 Oct 2009 15:24:08 -0300 Subject: [PATCH 3/3] Added eager loading support to Relation and ActiveRecord#all. --- .../lib/active_record/associations.rb | 4 +-- activerecord/lib/active_record/base.rb | 17 ++++------ activerecord/lib/active_record/relation.rb | 32 +++++++++++++++---- .../cascaded_eager_loading_test.rb | 2 +- 4 files changed, 34 insertions(+), 21 deletions(-) diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index b363cceccb..016745add2 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1455,9 +1455,9 @@ module ActiveRecord after_destroy(method_name) end - def find_with_associations(options = {}) + def find_with_associations(options = {}, join_dependency = nil) catch :invalid_query do - join_dependency = JoinDependency.new(self, merge_includes(scope(:find, :include), options[:include]), options[:joins]) + join_dependency ||= JoinDependency.new(self, merge_includes(scope(:find, :include), options[:include]), options[:joins]) rows = select_all_rows(options, join_dependency) return join_dependency.instantiate(rows) end diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 2fd3384877..60e69f020c 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -666,23 +666,18 @@ module ActiveRecord #:nodoc: def all(*args) options = args.extract_options! - if options.empty? && !scoped?(:find) relation = arel_table else + relation = construct_finder_arel(options) include_associations = merge_includes(scope(:find, :include), options[:include]) - # if include_associations.any? && references_eager_loaded_tables?(options) - # join_dependency = JoinDependency.new(self, include_associations, options[:joins]) - - # relation = construct_finder_arel_with_included_associations(options, join_dependency) - - # relation.preload(include_associations) - # else - relation = construct_finder_arel(options) - if include_associations.any? + if include_associations.any? + if references_eager_loaded_tables?(options) + relation.eager_load(include_associations) + else relation.preload(include_associations) - # end + end end end relation diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 6abb2df8ff..24fd29c7f3 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -7,11 +7,17 @@ module ActiveRecord @klass, @relation = klass, relation @readonly = false @associations_to_preload = [] + @eager_load_associations = [] end def preload(association) - @associations_to_preload << association - @associations_to_preload.flatten! + @associations_to_preload += association + self + end + + def eager_load(association) + @eager_load_associations += association + self end def readonly @@ -20,11 +26,23 @@ module ActiveRecord end def to_a - records = @klass.find_by_sql(@relation.to_sql) - - @klass.send :preload_associations, records, @associations_to_preload unless @associations_to_preload.empty? - - records.each { |record| record.readonly! } if @readonly + if @eager_load_associations.any? + records = catch :invalid_query do + @klass.send(:find_with_associations, { + :select => @relation.send(:select_clauses).join(', '), + :joins => @relation.joins(relation), + :group => @relation.send(:group_clauses).join(', '), + :order => @relation.send(:order_clauses).join(', '), + :conditions => @relation.send(:where_clauses).join("\n\tAND "), + :limit => @relation.taken + }, + ActiveRecord::Associations::ClassMethods::JoinDependency.new(@klass, @eager_load_associations, nil)) + end + else + records = @klass.find_by_sql(@relation.to_sql) + @klass.send(:preload_associations, records, @associations_to_preload) unless @associations_to_preload.empty? + records.each { |record| record.readonly! } if @readonly + end records end diff --git a/activerecord/test/cases/associations/cascaded_eager_loading_test.rb b/activerecord/test/cases/associations/cascaded_eager_loading_test.rb index 45e74ea024..ed2e2e9f8f 100644 --- a/activerecord/test/cases/associations/cascaded_eager_loading_test.rb +++ b/activerecord/test/cases/associations/cascaded_eager_loading_test.rb @@ -104,7 +104,7 @@ class CascadedEagerLoadingTest < ActiveRecord::TestCase authors.first.posts.first.special_comments.first.post.very_special_comment end end - + def test_eager_association_loading_where_first_level_returns_nil authors = Author.find(:all, :include => {:post_about_thinking => :comments}, :order => 'authors.id DESC') assert_equal [authors(:mary), authors(:david)], authors