aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorBen Woosley <ben.woosley@gmail.com>2013-05-14 15:01:20 +0200
committerBen Woosley <ben.woosley@gmail.com>2013-05-14 16:03:17 +0200
commit23c656c1bc113e5f198464ad29d72c5238bfd796 (patch)
tree4dcbee09d374aa64614da8ad9b5f4e81465b71e1
parent010ea715f2a629059117054f8079a5a1f60f30f6 (diff)
downloadrails-23c656c1bc113e5f198464ad29d72c5238bfd796.tar.gz
rails-23c656c1bc113e5f198464ad29d72c5238bfd796.tar.bz2
rails-23c656c1bc113e5f198464ad29d72c5238bfd796.zip
Backport a super-simplified version of #6792, fixing
that #exists? and others 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#distinct_value. Where #6792 was the forever fix, this is the minimal fix. Instead of properly indicating the distinctness of the query through #uniq_value alone, we use a literal select statement and set #uniq_value to always be falsey
-rw-r--r--activerecord/CHANGELOG.md7
-rw-r--r--activerecord/lib/active_record/relation/finder_methods.rb1
-rw-r--r--activerecord/test/cases/finder_test.rb12
3 files changed, 20 insertions, 0 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index e679d4f781..f8c75c0fba 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,5 +1,12 @@
## 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
avoid circular constant loading issues.
diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb
index 08cfe4f70d..cdf18f8080 100644
--- a/activerecord/lib/active_record/relation/finder_methods.rb
+++ b/activerecord/lib/active_record/relation/finder_methods.rb
@@ -254,6 +254,7 @@ module ActiveRecord
values = @klass.connection.distinct("#{@klass.connection.quote_table_name table_name}.#{primary_key}", orders)
relation = relation.dup.select(values)
+ relation.uniq_value = nil
id_rows = @klass.connection.select_all(relation.arel, 'SQL', relation.bind_values)
ids_array = id_rows.map {|row| row[primary_key]}
diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb
index 21bd7b51d5..2efafe5c24 100644
--- a/activerecord/test/cases/finder_test.rb
+++ b/activerecord/test/cases/finder_test.rb
@@ -93,6 +93,18 @@ class FinderTest < ActiveRecord::TestCase
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?