Merge pull request #4216 from edgecase/master_fix_reorder_with_limited_ids

allow reorder to affect eager loading correctly

activerecord/lib/active_record/relation/finder_methods.rb

@@ -249,7 +249,7 @@ module ActiveRecord
     end
 
     def construct_limited_ids_condition(relation)
-      orders = relation.order_values.map { |val| val.presence }.compact
+      orders = (relation.reorder_value || relation.order_values).map { |val| val.presence }.compact
       values = @klass.connection.distinct("#{@klass.connection.quote_table_name table_name}.#{primary_key}", orders)
 
       relation = relation.dup

activerecord/test/cases/associations/has_many_associations_test.rb

@@ -57,6 +57,16 @@ class HasManyAssociationsTestForCountDistinctWithFinderSql < ActiveRecord::TestC
   end
 end
 
+class HasManyAssociationsTestForReorderWithJoinDependency < ActiveRecord::TestCase
+  fixtures :authors, :posts, :comments
+
+  def test_should_generate_valid_sql
+    author = authors(:david)
+    # this can fail on adapters which require ORDER BY expressions to be included in the SELECT expression
+    # if the reorder clauses are not correctly handled
+    assert author.posts_with_comments_sorted_by_comment_id.where('comments.id > 0').reorder('posts.comments_count DESC', 'posts.taggings_count DESC').last
+  end
+end
 
 
 class HasManyAssociationsTest < ActiveRecord::TestCase