Introduce preload query strategy for eager :includes. Closes #9640.

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@8672 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

9 files changed, 351 insertions, 15 deletions

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index 60e4a9c2ad..c6c2f00309 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,5 +1,7 @@
 *SVN*
 
+* Introduce preload query strategy for eager :includes.  #9640 [Frederick Cheung, Aleksey Kondratenko]
+
 * Support aggregations in finder conditions.  #10572 [Ryan Kinderman]
 
 * Organize and clean up the Active Record test suite.  #10742 [John Barnette]
diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb
index 36c1a2bd31..4d85c24584 100755
--- a/activerecord/lib/active_record.rb
+++ b/activerecord/lib/active_record.rb
@@ -43,6 +43,7 @@ require 'active_record/validations'
 require 'active_record/callbacks'
 require 'active_record/reflection'
 require 'active_record/associations'
+require 'active_record/association_preload'
 require 'active_record/aggregations'
 require 'active_record/transactions'
 require 'active_record/timestamp'
@@ -63,6 +64,7 @@ ActiveRecord::Base.class_eval do
   include ActiveRecord::Observing
   include ActiveRecord::Timestamp
   include ActiveRecord::Associations
+  include ActiveRecord::AssociationPreload
   include ActiveRecord::Aggregations
   include ActiveRecord::Transactions
   include ActiveRecord::Reflection
diff --git a/activerecord/lib/active_record/association_preload.rb b/activerecord/lib/active_record/association_preload.rb
new file mode 100644
index 0000000000..6fa49dfbe8
--- /dev/null
+++ b/activerecord/lib/active_record/association_preload.rb
@@ -0,0 +1,231 @@
+module ActiveRecord
+  module AssociationPreload #:nodoc:
+    def self.included(base)
+      base.extend(ClassMethods)
+    end
+
+    module ClassMethods
+
+      # Loads the named associations for the activerecord record (or records) given
+      # preload_options is passed only one level deep: don't pass to the child associations when associations is a Hash
+      protected
+      def preload_associations(records, associations, preload_options={})
+        records = [records].flatten.compact
+        return if records.empty?
+        case associations
+        when Array then associations.each {|association| preload_associations(records, association, preload_options)}
+        when Symbol, String then preload_one_association(records, associations.to_sym, preload_options)
+        when Hash then
+          associations.each do |parent, child|
+            raise "parent must be an association name" unless parent.is_a?(String) || parent.is_a?(Symbol)
+            preload_associations(records, parent, preload_options)
+            reflection = reflections[parent]
+            parents = records.map {|record| record.send(reflection.name)}.flatten
+            unless parents.empty?
+              parents.first.class.preload_associations(parents, child)
+            end
+          end
+        end
+      end
+
+      private
+
+      def preload_one_association(records, association, preload_options={})
+        reflection = reflections[association]
+        raise ConfigurationError, "Association named '#{ association }' was not found; perhaps you misspelled it?" unless reflection
+
+        send(:"preload_#{reflection.macro}_association", records, reflection, preload_options)
+      end
+
+      def add_preloaded_records_to_collection(parent_records, reflection_name, associated_record)
+        parent_records.each do |parent_record|
+          association_proxy = parent_record.send(reflection_name)
+          association_proxy.loaded
+          association_proxy.target.push(*[associated_record].flatten)
+        end
+      end
+
+      def set_association_collection_records(id_to_record_map, reflection_name, associated_records, key)
+        associated_records.each do |associated_record|
+          mapped_records = id_to_record_map[associated_record[key].to_i]
+          add_preloaded_records_to_collection(mapped_records, reflection_name, associated_record)
+        end
+      end
+
+      def set_association_single_records(id_to_record_map, reflection_name, associated_records, key)
+        associated_records.each do |associated_record|
+          mapped_records = id_to_record_map[associated_record[key].to_i]
+          mapped_records.each do |mapped_record|
+            mapped_record.send("set_#{reflection_name}_target", associated_record)
+          end
+        end
+      end
+
+      def construct_id_map(records)
+        id_to_record_map = {}
+        ids = []
+        records.each do |record|
+          ids << record.id
+          mapped_records = (id_to_record_map[record.id] ||= [])
+          mapped_records << record
+        end
+        ids.uniq!
+        return id_to_record_map, ids
+      end
+
+      # FIXME: quoting
+      def preload_has_and_belongs_to_many_association(records, reflection, preload_options={})
+        table_name = reflection.klass.table_name
+        id_to_record_map, ids = construct_id_map(records)
+        records.each {|record| record.send(reflection.name).loaded}
+        options = reflection.options
+
+        conditions = "t0.#{reflection.primary_key_name}  IN (?)"
+        conditions << append_conditions(options, preload_options)
+
+        associated_records = reflection.klass.find(:all, :conditions => [conditions, ids],
+        :include => options[:include],
+        :joins => "INNER JOIN #{options[:join_table]} as t0 ON #{reflection.klass.table_name}.#{reflection.klass.primary_key} = t0.#{reflection.association_foreign_key}",
+        :select => "#{options[:select] || table_name+'.*'}, t0.#{reflection.primary_key_name} as _parent_record_id",
+        :order => options[:order])
+
+        set_association_collection_records(id_to_record_map, reflection.name, associated_records, '_parent_record_id')
+      end
+
+      def preload_has_one_association(records, reflection, preload_options={})
+        id_to_record_map, ids = construct_id_map(records)
+        records.each {|record| record.send("set_#{reflection.name}_target", nil)}
+
+        set_association_single_records(id_to_record_map, reflection.name, find_associated_records(ids, reflection, preload_options),
+                                       reflection.primary_key_name)
+      end
+
+      def preload_has_many_association(records, reflection, preload_options={})
+        id_to_record_map, ids = construct_id_map(records)
+        records.each {|record| record.send(reflection.name).loaded}
+        options = reflection.options
+
+        if options[:through]
+          through_records = preload_through_records(records, reflection, options[:through])
+          through_reflection = reflections[options[:through]]
+          through_primary_key = through_reflection.primary_key_name
+          unless through_records.empty?
+            source = reflection.source_reflection.name
+            through_records.first.class.preload_associations(through_records, source)
+            through_records.compact.each do |through_record|
+              add_preloaded_records_to_collection(id_to_record_map[through_record[through_primary_key].to_i],
+                                                 reflection.name, through_record.send(source))
+            end
+          end
+        else
+          set_association_collection_records(id_to_record_map, reflection.name, find_associated_records(ids, reflection, preload_options),
+                                             reflection.primary_key_name)
+        end
+      end
+
+      def preload_through_records(records, reflection, through_association)
+        through_reflection = reflections[through_association]
+        through_primary_key = through_reflection.primary_key_name
+
+        if reflection.options[:source_type]
+          interface = reflection.source_reflection.options[:foreign_type]
+          preload_options = {:conditions => ["#{interface} = ?", reflection.options[:source_type]]}
+
+          records.first.class.preload_associations(records, through_association, preload_options)
+
+          # Dont cache the association - we would only be caching a subset
+          through_records = []
+          records.compact.each do |record|
+            proxy = record.send(through_association)
+            through_records << proxy.target
+            proxy.reset
+          end
+          through_records = through_records.flatten
+        else
+          records.first.class.preload_associations(records, through_association)
+          through_records = records.compact.map {|record| record.send(through_association)}.flatten
+        end
+      end
+
+      # FIXME: quoting
+      def preload_belongs_to_association(records, reflection, preload_options={})
+        options = reflection.options
+        primary_key_name = reflection.primary_key_name
+
+        if options[:polymorphic]
+          polymorph_type = options[:foreign_type]
+          klasses_and_ids = {}
+
+          # Construct a mapping from klass to a list of ids to load and a mapping of those ids back to their parent_records
+          records.each do |record|
+            klass = record.send(polymorph_type)
+            klass_id = record.send(primary_key_name)
+
+            id_map = klasses_and_ids[klass] ||= {}
+            id_list_for_klass_id = (id_map[klass_id] ||= [])
+            id_list_for_klass_id << record
+          end
+          klasses_and_ids = klasses_and_ids.to_a
+        else
+          id_map = {}
+          records.each do |record|
+            mapped_records = (id_map[record.send(primary_key_name)] ||= [])
+            mapped_records << record
+          end
+          klasses_and_ids = [[reflection.klass.name, id_map]]
+        end
+
+        klasses_and_ids.each do |klass_and_id|
+          klass_name, id_map = *klass_and_id
+          klass = klass_name.constantize
+
+          table_name = klass.table_name
+          conditions = "#{table_name}.#{primary_key} IN (?)"
+          conditions << append_conditions(options, preload_options)
+          associated_records = klass.find(:all, :conditions => [conditions, id_map.keys.uniq],
+                                          :include => options[:include],
+                                          :select => options[:select],
+                                          :joins => options[:joins],
+                                          :order => options[:order])
+          set_association_single_records(id_map, reflection.name, associated_records, 'id')
+        end
+      end
+
+      # FIXME: quoting
+      def find_associated_records(ids, reflection, preload_options)
+        options = reflection.options
+        table_name = reflection.klass.table_name
+
+        if interface = reflection.options[:as]
+          conditions = "#{reflection.klass.table_name}.#{interface}_id IN (?) and #{reflection.klass.table_name}.#{interface}_type = '#{self.base_class.name.demodulize}'"
+        else
+          foreign_key = reflection.primary_key_name
+          conditions = "#{reflection.klass.table_name}.#{foreign_key} IN (?)"
+        end
+
+        conditions << append_conditions(options, preload_options)
+
+        reflection.klass.find(:all,
+                              :select => (options[:select] || "#{table_name}.*"),
+                              :include => options[:include],
+                              :conditions => [conditions, ids],
+                              :joins => options[:joins],
+                              :group => options[:group],
+                              :order => options[:order])
+      end
+
+
+      def interpolate_sql_for_preload(sql)
+        instance_eval("%@#{sql.gsub('@', '\@')}@")
+      end
+
+      def append_conditions(options, preload_options)
+        sql = ""
+        sql << " AND (#{interpolate_sql_for_preload(sanitize_sql(options[:conditions]))})" if options[:conditions]
+        sql << " AND (#{sanitize_sql preload_options[:conditions]})" if preload_options[:conditions]
+        sql
+      end
+
+    end
+  end
+end
diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb
index 8434644325..4743d3be8f 100755
--- a/activerecord/lib/active_record/associations.rb
+++ b/activerecord/lib/active_record/associations.rb
@@ -1365,11 +1365,15 @@ module ActiveRecord
         def include_eager_order?(options)
           order = options[:order]
           return false unless order
-          order.scan(/([\.\w]+)\.\w+/).flatten.any? do |order_table_name|
+          order.to_s.scan(/([\.\w]+)\.\w+/).flatten.any? do |order_table_name|
             order_table_name != table_name
           end
         end
 
+        def references_eager_loaded_tables?(options)
+          include_eager_order?(options) || include_eager_conditions?(options)
+        end
+
         def using_limitable_reflections?(reflections)
           reflections.reject { |r| [ :belongs_to, :has_one ].include?(r.macro) }.length.zero?
         end
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index 2d2f2297ed..dfde2e6e1d 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -1237,9 +1237,16 @@ module ActiveRecord #:nodoc:
         end
 
         def find_every(options)
-          records = scoped?(:find, :include) || options[:include] ?
-            find_with_associations(options) :
-            find_by_sql(construct_finder_sql(options))
+          include_associations = merge_includes(scope(:find, :include), options[:include])
+
+          if include_associations.any? && references_eager_loaded_tables?(options)
+            records = find_with_associations(options)
+          else
+            records = find_by_sql(construct_finder_sql(options))
+            if include_associations.any?
+              preload_associations(records, include_associations)
+            end
+          end
 
           records.each { |record| record.readonly! } if options[:readonly]
 
diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb
index 0e351816d3..9ceb507e91 100644
--- a/activerecord/test/cases/associations/eager_test.rb
+++ b/activerecord/test/cases/associations/eager_test.rb
@@ -1,5 +1,6 @@
 require 'abstract_unit'
 require 'models/post'
+require 'models/tagging'
 require 'models/comment'
 require 'models/author'
 require 'models/category'
@@ -9,7 +10,7 @@ require 'models/reader'
 
 class EagerAssociationTest < ActiveSupport::TestCase
   fixtures :posts, :comments, :authors, :categories, :categories_posts,
-            :companies, :accounts, :tags, :people, :readers
+            :companies, :accounts, :tags, :taggings, :people, :readers
 
   def test_loading_with_one_association
     posts = Post.find(:all, :include => :comments)
@@ -56,6 +57,13 @@ class EagerAssociationTest < ActiveSupport::TestCase
     assert posts.first.comments.include?(comments(:greetings))
   end
 
+  def test_duplicate_middle_objects
+    comments = Comment.find :all, :conditions => 'post_id = 1', :include => [:post => :author]
+    assert_no_queries do
+      comments.each {|comment| comment.post.author.name}
+    end
+  end
+
   def test_loading_from_an_association
     posts = authors(:david).posts.find(:all, :include => :comments, :order => "posts.id")
     assert_equal 2, posts.first.comments.size
@@ -353,6 +361,17 @@ class EagerAssociationTest < ActiveSupport::TestCase
     assert_equal posts(:sti_post_and_comments, :sti_comments), Post.find(:all, :include => [:author, :comments], :conditions => "authors.name = 'David'", :order => 'UPPER(posts.title) DESC, posts.id', :limit => 2, :offset => 1)
   end
 
+  def test_preload_with_interpolation
+    assert_equal [comments(:greetings)], Post.find(posts(:welcome).id, :include => :comments_with_interpolated_conditions).comments_with_interpolated_conditions
+  end
+
+  def test_polymorphic_type_condition
+    post = Post.find(posts(:thinking).id, :include => :taggings)
+    assert post.taggings.include?(taggings(:thinking_general))
+    post = SpecialPost.find(posts(:thinking).id, :include => :taggings)
+    assert post.taggings.include?(taggings(:thinking_general))
+  end
+
   def test_eager_with_multiple_associations_with_same_table_has_many_and_habtm
     # Eager includes of has many and habtm associations aren't necessarily sorted in the same way
     def assert_equal_after_sort(item1, item2, item3 = nil)
@@ -405,34 +424,40 @@ class EagerAssociationTest < ActiveSupport::TestCase
 
   def test_preconfigured_includes_with_belongs_to
     author = posts(:welcome).author_with_posts
-    assert_equal 5, author.posts.size
+    assert_no_queries {assert_equal 5, author.posts.size}
   end
 
   def test_preconfigured_includes_with_has_one
     comment = posts(:sti_comments).very_special_comment_with_post
-    assert_equal posts(:sti_comments), comment.post
+    assert_no_queries {assert_equal posts(:sti_comments), comment.post}
   end
 
   def test_preconfigured_includes_with_has_many
     posts = authors(:david).posts_with_comments
     one = posts.detect { |p| p.id == 1 }
-    assert_equal 5, posts.size
-    assert_equal 2, one.comments.size
+    assert_no_queries do
+      assert_equal 5, posts.size
+      assert_equal 2, one.comments.size
+    end
   end
 
   def test_preconfigured_includes_with_habtm
     posts = authors(:david).posts_with_categories
     one = posts.detect { |p| p.id == 1 }
-    assert_equal 5, posts.size
-    assert_equal 2, one.categories.size
+    assert_no_queries do
+      assert_equal 5, posts.size
+      assert_equal 2, one.categories.size
+    end
   end
 
   def test_preconfigured_includes_with_has_many_and_habtm
     posts = authors(:david).posts_with_comments_and_categories
     one = posts.detect { |p| p.id == 1 }
-    assert_equal 5, posts.size
-    assert_equal 2, one.comments.size
-    assert_equal 2, one.categories.size
+    assert_no_queries do
+      assert_equal 5, posts.size
+      assert_equal 2, one.comments.size
+      assert_equal 2, one.categories.size
+    end
   end
 
   def test_count_with_include
diff --git a/activerecord/test/cases/associations/join_model_test.rb b/activerecord/test/cases/associations/join_model_test.rb
index 23ed9d1c5e..c8ae42d78c 100644
--- a/activerecord/test/cases/associations/join_model_test.rb
+++ b/activerecord/test/cases/associations/join_model_test.rb
@@ -317,7 +317,7 @@ class AssociationsJoinModelTest < ActiveSupport::TestCase
       assert_equal posts(:welcome, :thinking), tags(:general).taggables
     end
     assert_raise ActiveRecord::EagerLoadPolymorphicError do
-      assert_equal posts(:welcome, :thinking), tags(:general).taggings.find(:all, :include => :taggable)
+      assert_equal posts(:welcome, :thinking), tags(:general).taggings.find(:all, :include => :taggable, :conditions => 'bogus_table.column = 1')
     end
   end
 
@@ -331,6 +331,7 @@ class AssociationsJoinModelTest < ActiveSupport::TestCase
     assert_no_queries do
       assert_equal desired, tag_with_include.tagged_posts
     end
+    assert_equal 4, tag_with_include.taggings.length
   end
 
   def test_has_many_through_has_many_find_all
@@ -546,6 +547,62 @@ class AssociationsJoinModelTest < ActiveSupport::TestCase
     assert_equal comment_ids.sort.reverse, authors(:david).ordered_uniq_comments_desc.map(&:id)
   end
 
+  def test_polymorphic_has_many
+    expected = taggings(:welcome_general)
+    p = Post.find(posts(:welcome).id, :include => :taggings)
+    assert_no_queries {assert p.taggings.include?(expected)}
+    assert posts(:welcome).taggings.include?(taggings(:welcome_general))
+  end
+
+  def test_polymorphic_has_one
+    expected = posts(:welcome)
+
+    tagging  = Tagging.find(taggings(:welcome_general).id, :include => :taggable)
+    assert_no_queries { assert_equal expected, tagging.taggable}
+  end
+
+  def test_polymorphic_belongs_to
+    p = Post.find(posts(:welcome).id, :include => {:taggings => :taggable})
+    assert_no_queries {assert_equal posts(:welcome), p.taggings.first.taggable}
+  end
+
+  def test_preload_polymorphic_has_many_through
+    posts           = Post.find(:all, :order => 'posts.id')
+    posts_with_tags = Post.find(:all, :include => :tags, :order => 'posts.id')
+    assert_equal posts.length, posts_with_tags.length
+    posts.length.times do |i|
+      assert_equal posts[i].tags.length, assert_no_queries { posts_with_tags[i].tags.length }
+    end
+  end
+
+  def test_preload_polymorph_many_types
+    taggings = Tagging.find :all, :include => :taggable, :conditions => ['taggable_type != ?', 'FakeModel']
+    assert_no_queries do
+      taggings.first.taggable.id
+      taggings[1].taggable.id
+    end
+
+    taggables = taggings.map(&:taggable)
+    assert taggables.include?(items(:dvd))
+    assert taggables.include?(posts(:welcome))
+  end
+
+  def test_preload_polymorphic_has_many
+    posts               = Post.find(:all, :order => 'posts.id')
+    posts_with_taggings = Post.find(:all, :include => :taggings, :order => 'posts.id')
+    assert_equal posts.length, posts_with_taggings.length
+    posts.length.times do |i|
+      assert_equal posts[i].taggings.length, assert_no_queries { posts_with_taggings[i].taggings.length }
+    end
+  end
+
+  def test_belongs_to_shared_parent
+    comments = Comment.find(:all, :include => :post, :conditions => 'post_id = 1')
+    assert_no_queries do
+      assert_equal comments.first.post, comments[1].post
+    end
+  end
+
   private
     # create dynamic Post models to allow different dependency options
     def find_post_with_dependency(post_id, association, association_name, dependency)
diff --git a/activerecord/test/cases/associations_test.rb b/activerecord/test/cases/associations_test.rb
index 451a17b3bf..b21f0cc9e2 100755
--- a/activerecord/test/cases/associations_test.rb
+++ b/activerecord/test/cases/associations_test.rb
@@ -21,6 +21,11 @@ class AssociationsTest < ActiveSupport::TestCase
   fixtures :accounts, :companies, :developers, :projects, :developers_projects,
            :computers
 
+  def test_include_with_order_works
+    assert_nothing_raised {Account.find(:first, :order => 'id', :include => :firm)}
+    assert_nothing_raised {Account.find(:first, :order => :id, :include => :firm)}
+  end
+
   def test_bad_collection_keys
     assert_raise(ArgumentError, 'ActiveRecord should have barked on bad collection keys') do
       Class.new(ActiveRecord::Base).has_many(:wheels, :name => 'wheels')
diff --git a/activerecord/test/models/post.rb b/activerecord/test/models/post.rb
index 53dcbb74f3..176a0ddc1e 100644
--- a/activerecord/test/models/post.rb
+++ b/activerecord/test/models/post.rb
@@ -13,6 +13,9 @@ class Post < ActiveRecord::Base
     end
   end
 
+  has_many :comments_with_interpolated_conditions, :class_name => 'Comment',
+      :conditions => ['#{"#{aliased_table_name}." rescue ""}body = ?', 'Thank you for the welcome']
+
   has_one  :very_special_comment
   has_one  :very_special_comment_with_post, :class_name => "VerySpecialComment", :include => :post
   has_many :special_comments