Ticket #3438: eager_loading_order_patch.diff

activerecord/test/associations/eager_test.rb

@@ -368 +368 @@
-    assert_equal 2, one.comments.size
-  end
-
+  def test_association_orders_work_with_eager_loading
+    { Author         => [ :posts,  :posts_with_comments, :posts_with_categories, :posts_with_comments_and_categories,
+                          :comments, :funky_comments, :special_posts, :hello_posts, :nonexistent_posts, 
+                          :posts_with_callbacks, :posts_with_proc_callbacks, :posts_with_multiple_callbacks,
+                          :unchangable_posts, :author_favorites ],
+      Category       => [ :posts, :special_posts, :hello_posts, :nonexistent_posts ],
+      DependentFirm  => [ :companies ],
+      Firm           => [ :clients, :clients_sorted_desc, :clients_of_firm, :dependent_clients_of_firm,
+                          :exclusively_dependent_clients_of_firm ],
+      Person         => [ :readers, :posts ],
+      Post           => [ :comments, :special_comments, :categories, :special_categories, :readers, :people ]
+    }.each do |model, associations|
+      associations.each do |association|
+        regular_records = model.find(:all).sort_by(&:id)
+        eager_records = model.find(:all, :include => [association]).sort_by(&:id)
+        regular_records.size.times do |i|
+          assert_equal regular_records[i].send(association).map(&:id), eager_records[i].send(association).map(&:id), "#{model}.#{association} has a different order when eagerly loaded."
+        end
+
+        expected_order = model.find(:all, :order => "#{model.table_name}.#{model.primary_key}").map(&association).flatten.map(&:id)
+        actual_order = model.find(:all, :include => [association], :order => "#{model.table_name}.#{model.primary_key}").map(&association).flatten.map(&:id)
+        assert_equal expected_order, actual_order, "#{model}.#{association} has a different order when eagerly loaded given an :order option"
+      end      
+    end
+  end
+
-  def test_preconfigured_includes_with_habtm
-    posts = authors(:david).posts_with_categories
-    one = posts.detect { |p| p.id == 1 }

activerecord/test/associations/cascaded_eager_loading_test.rb

@@ -104 +104 @@
-    end
-  end
-
+  def test_association_orders_work_with_cascaded_eager_loading
+    [ { :posts => :comments }, { :posts => :categories },
+      { :posts => [:comments, :categories] },
+      { :comments => :post }, { :funky_comments => :post } ].each do |association|
+      regular_records = Author.find(:all).sort_by(&:id)
+      eager_records = Author.find(:all, :include => association).sort_by(&:id)
+      regular_records.size.times do |i|
+        assert_equal regular_records[i].send(association.keys.first).map(&:id), eager_records[i].send(association.keys.first).map(&:id), "Author.#{association.keys.first} has a different order when eagerly loaded via #{association.inspect}."
+      end
+    end
+  end
+
-  def test_eager_association_loading_with_recursive_cascading_three_levels_has_many
-    root_node = RecursivelyCascadedTreeMixin.find(:first, :include=>{:children=>{:children=>:children}}, :order => 'mixins.id')
-    assert_equal mixins(:recursively_cascaded_tree_4), assert_no_queries { root_node.children.first.children.first.children.first }

activerecord/test/fixtures/post.rb

@@ -15 +15 @@
-
-  has_one  :very_special_comment
-  has_one  :very_special_comment_with_post, :class_name => "VerySpecialComment", :include => :post
-
+, :order => 'type'
-
-
-
+, :order => 'name'
+, :order => 'name DESC, type ASC'
-
-  has_many :taggings, :as => :taggable
-  has_many :tags, :through => :taggings, :include => :tagging do
@@ -38 +38 @@
-  has_many :categorizations, :foreign_key => :category_id
-  has_many :authors, :through => :categorizations
-
-
-
+, :order => 'readers.person_id'
+, :order => 'first_name DESC, id ASC'
-
-  def self.what_are_you
-    'a post...'

activerecord/test/fixtures/author.rb

@@ -1 +1 @@
-class Author < ActiveRecord::Base
-
-
-
+, :order => 'title DESC'
+, :order => 'title DESC'
+, :order => 'title'
-  has_many :posts_with_comments_and_categories, :include => [ :comments, :categories ], :order => "posts.id", :class_name => "Post"
-  has_many :posts_with_extension, :class_name => "Post" do #, :extend => ProxyTestExtension
-    def testing_proxy_owner
@@ -14 +14 @@
-      proxy_target
-    end
-  end
-
-
+, :order => 'body DESC'
+, :order => 'body'
-
-  has_many :special_posts
-  has_many :special_post_comments, :through => :special_posts, :source => :comments

activerecord/lib/active_record/associations.rb

@@ -1249 +1249 @@
-          add_conditions!(sql, options[:conditions], scope)
-          add_limited_ids_condition!(sql, options, join_dependency) if !using_limitable_reflections?(join_dependency.reflections) && ((scope && scope[:limit]) || options[:limit])
-
+          # Collect all eagerly-loaded assocations with any order options, and
+          # add in their table names unless it's already there.
+          associated_orders = join_dependency.join_associations.select do |assoc|
+            assoc.reflection.options[:order]
+          end.map do |assoc|
+            orders = assoc.reflection.options[:order].split(',').map(&:strip)
+            orders.map do |order|
+              if order =~ /\./
+                order
+              else
+                assoc.aliased_table_name + '.' + order
+              end
+            end
+          end
+
+          # Add the association orders to the SQL statement.
+          if associated_orders.any?
+            if options[:order]
+              sql << ", "
+            else
+              sql << "ORDER BY "
+            end
+            sql << associated_orders.flatten.join(', ')
+          end
+          sql << ' '
+
-          sql << "GROUP BY #{options[:group]} " if options[:group]
- 
-          add_order!(sql, options[:order], scope)