Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Fix that #exists? can produce invalid SQL: "SELECT DISTINCT DISTINCT"

The combination of a :uniq => true association and the #distinct call
in #construct_limited_ids_condition combine to create invalid SQL, because
we're explicitly selecting DISTINCT, and also sending #distinct on to AREL,
via the relation#uniq_value.

I unset relation#uniq_value in the limited ids lookup so that Arel doesn't
apply the DISTINCT that we're already applying. This only affects the limited
ids lookup query, and only in the problematic case where uniq_value was true.
  • Loading branch information...
commit 8aeeef05d7c85492e6c8cb0fdc0150bfb2311b2c 1 parent f48e767
@Empact authored
View
1  activerecord/lib/active_record/relation/finder_methods.rb
@@ -240,6 +240,7 @@ def construct_limited_ids_condition(relation)
values = @klass.connection.distinct("#{@klass.connection.quote_table_name table_name}.#{primary_key}", orders)
relation = relation.dup
+ relation.uniq_value = false # avoids SELECT DISTINCT DISTINCT
ids_array = relation.select(values).collect {|row| row[primary_key]}
ids_array.empty? ? raise(ThrowResult) : table[primary_key].in(ids_array)
View
12 activerecord/test/cases/finder_test.rb
@@ -74,6 +74,18 @@ def test_exists_with_includes_limit_and_empty_result
assert !Topic.includes(:replies).limit(1).where('0 = 1').exists?
end
+ def test_exists_with_distinct_association_includes_and_limit
+ author = Author.first
+ assert !author.unique_categorized_posts.includes(:special_comments).limit(0).exists?
+ assert author.unique_categorized_posts.includes(:special_comments).limit(1).exists?
+ end
+
+ def test_exists_with_distinct_association_includes_limit_and_order
+ author = Author.first
+ assert !author.unique_categorized_posts.includes(:special_comments).order('comments.taggings_count DESC').limit(0).exists?
+ assert author.unique_categorized_posts.includes(:special_comments).order('comments.taggings_count DESC').limit(1).exists?
+ end
+
def test_exists_with_empty_table_and_no_args_given
Topic.delete_all
assert !Topic.exists?
Please sign in to comment.
Something went wrong with that request. Please try again.