Skip to content

Commit

Permalink
Merge pull request #10616 from Empact/backport-distinct-distinct
Browse files Browse the repository at this point in the history
Backport a super-simplified version of #6792, fixing that #exists? can produce invalid SQL: "SELECT DISTINCT DISTINCT"
  • Loading branch information
rafaelfranca committed May 14, 2013
2 parents 010ea71 + 23c656c commit 5b020fa
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 0 deletions.
7 changes: 7 additions & 0 deletions activerecord/CHANGELOG.md
@@ -1,5 +1,12 @@
## unreleased ## ## unreleased ##


* Fix that under some conditions, Active Record could produce invalid SQL of the sort:
"SELECT DISTINCT DISTINCT".

Backport of #6792.

*Ben Woosley*

* Require `ActiveRecord::Base` in railtie hooks for rake_tasks, console and runner to * Require `ActiveRecord::Base` in railtie hooks for rake_tasks, console and runner to
avoid circular constant loading issues. avoid circular constant loading issues.


Expand Down
1 change: 1 addition & 0 deletions activerecord/lib/active_record/relation/finder_methods.rb
Expand Up @@ -254,6 +254,7 @@ def construct_limited_ids_condition(relation)
values = @klass.connection.distinct("#{@klass.connection.quote_table_name table_name}.#{primary_key}", orders) values = @klass.connection.distinct("#{@klass.connection.quote_table_name table_name}.#{primary_key}", orders)


relation = relation.dup.select(values) relation = relation.dup.select(values)
relation.uniq_value = nil


id_rows = @klass.connection.select_all(relation.arel, 'SQL', relation.bind_values) id_rows = @klass.connection.select_all(relation.arel, 'SQL', relation.bind_values)
ids_array = id_rows.map {|row| row[primary_key]} ids_array = id_rows.map {|row| row[primary_key]}
Expand Down
12 changes: 12 additions & 0 deletions activerecord/test/cases/finder_test.rb
Expand Up @@ -93,6 +93,18 @@ def test_exists_with_includes_limit_and_empty_result
assert !Topic.includes(:replies).limit(1).where('0 = 1').exists? assert !Topic.includes(:replies).limit(1).where('0 = 1').exists?
end 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 def test_exists_with_empty_table_and_no_args_given
Topic.delete_all Topic.delete_all
assert !Topic.exists? assert !Topic.exists?
Expand Down

0 comments on commit 5b020fa

Please sign in to comment.