From 15d6e4dce7126fe24bce5cdb91d2ffee68648420 Mon Sep 17 00:00:00 2001
From: Ben Woosley <ben.woosley@gmail.com>
Date: Mon, 28 May 2012 12:23:37 -0700
Subject: 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#distinct_value.

Rather than build a select distinct clause in #construct_limited_ids_condition,
I set #distinct! and pass just the columns into the select statement.
This requires introducing a #columns_for_distinct method to return the
select columns but not the statement itself.
---
 .../adapters/postgresql/postgresql_adapter_test.rb | 34 ++++++++++++++++++++++
 activerecord/test/cases/finder_test.rb             | 12 ++++++++
 2 files changed, 46 insertions(+)

(limited to 'activerecord/test/cases')

diff --git a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb
index 17d77c5454..ff7eb86969 100644
--- a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb
+++ b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb
@@ -230,21 +230,41 @@ module ActiveRecord
           @connection.distinct("posts.id", [])
       end
 
+      def test_columns_for_distinct_zero_orders
+        assert_equal "posts.id",
+          @connection.columns_for_distinct("posts.id", [])
+      end
+
       def test_distinct_one_order
         assert_equal "DISTINCT posts.id, posts.created_at AS alias_0",
           @connection.distinct("posts.id", ["posts.created_at desc"])
       end
 
+      def test_columns_for_distinct_one_order
+        assert_equal "posts.id, posts.created_at AS alias_0",
+          @connection.columns_for_distinct("posts.id", ["posts.created_at desc"])
+      end
+
       def test_distinct_few_orders
         assert_equal "DISTINCT posts.id, posts.created_at AS alias_0, posts.position AS alias_1",
           @connection.distinct("posts.id", ["posts.created_at desc", "posts.position asc"])
       end
 
+      def test_columns_for_distinct_few_orders
+        assert_equal "posts.id, posts.created_at AS alias_0, posts.position AS alias_1",
+          @connection.columns_for_distinct("posts.id", ["posts.created_at desc", "posts.position asc"])
+      end
+
       def test_distinct_blank_not_nil_orders
         assert_equal "DISTINCT posts.id, posts.created_at AS alias_0",
           @connection.distinct("posts.id", ["posts.created_at desc", "", "   "])
       end
 
+      def test_columns_for_distinct_blank_not_nil_orders
+        assert_equal "posts.id, posts.created_at AS alias_0",
+          @connection.columns_for_distinct("posts.id", ["posts.created_at desc", "", "   "])
+      end
+
       def test_distinct_with_arel_order
         order = Object.new
         def order.to_sql
@@ -254,11 +274,25 @@ module ActiveRecord
           @connection.distinct("posts.id", [order])
       end
 
+      def test_columns_for_distinct_with_arel_order
+        order = Object.new
+        def order.to_sql
+          "posts.created_at desc"
+        end
+        assert_equal "posts.id, posts.created_at AS alias_0",
+          @connection.columns_for_distinct("posts.id", [order])
+      end
+
       def test_distinct_with_nulls
         assert_equal "DISTINCT posts.title, posts.updater_id AS alias_0", @connection.distinct("posts.title", ["posts.updater_id desc nulls first"])
         assert_equal "DISTINCT posts.title, posts.updater_id AS alias_0", @connection.distinct("posts.title", ["posts.updater_id desc nulls last"])
       end
 
+      def test_columns_for_distinct_with_nulls
+        assert_equal "posts.title, posts.updater_id AS alias_0", @connection.columns_for_distinct("posts.title", ["posts.updater_id desc nulls first"])
+        assert_equal "posts.title, posts.updater_id AS alias_0", @connection.columns_for_distinct("posts.title", ["posts.updater_id desc nulls last"])
+      end
+
       def test_raise_error_when_cannot_translate_exception
         assert_raise TypeError do
           @connection.send(:log, nil) { @connection.execute(nil) }
diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb
index 7db9aef218..6f0de42aef 100644
--- a/activerecord/test/cases/finder_test.rb
+++ b/activerecord/test/cases/finder_test.rb
@@ -98,6 +98,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?
-- 
cgit v1.2.3