Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Merge pull request #4216 from edgecase/master_fix_reorder_with_limite…

…d_ids

allow reorder to affect eager loading correctly
  • Loading branch information...
commit 780a222dc2e888646aba4b6908ea78dba9d50722 2 parents f06074f + 18fb2d4
@tenderlove tenderlove authored
View
2  activerecord/lib/active_record/relation/finder_methods.rb
@@ -249,7 +249,7 @@ def apply_join_dependency(relation, join_dependency)
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
View
10 activerecord/test/cases/associations/has_many_associations_test.rb
@@ -57,6 +57,16 @@ def test_should_count_distinct_results
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
Please sign in to comment.
Something went wrong with that request. Please try again.