From e789b26e652d9f0ee6ca538615b8d570cb4c91b5 Mon Sep 17 00:00:00 2001
From: Rick Olson <technoweenie@gmail.com>
Date: Fri, 13 Oct 2006 08:29:00 +0000
Subject: [PATCH] fix select_limited_ids_list issues in postgresql, retain
 current behavior in other adapters [Rick]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@5291 5ecf4fe2-1ee6-0310-87b1-e25e094e27de
---
 activerecord/CHANGELOG                              |  2 ++
 activerecord/lib/active_record/associations.rb      | 13 +++++++++----
 .../abstract/schema_statements.rb                   |  5 +++++
 .../connection_adapters/postgresql_adapter.rb       | 11 ++++++++++-
 activerecord/test/associations_test.rb              |  3 +--
 5 files changed, 27 insertions(+), 7 deletions(-)

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index 14fc21778a..cef8bccba1 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,5 +1,7 @@
 *SVN*
 
+* fix select_limited_ids_list issues in postgresql, retain current behavior in other adapters [Rick]
+
 * Restore eager condition interpolation, document it's differences [Rick]
 
 * Don't rollback in teardown unless a transaction was started. Don't start a transaction in create_fixtures if a transaction is started.  #6282 [Jacob Fugal, Jeremy Kemper]
diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb
index e075ecaf3c..d52695d162 100755
--- a/activerecord/lib/active_record/associations.rb
+++ b/activerecord/lib/active_record/associations.rb
@@ -1192,13 +1192,18 @@ module ActiveRecord
         end
  
         def construct_finder_sql_for_association_limiting(options, join_dependency)
-          scope = scope(:find)
+          scope       = scope(:find)
+          is_distinct = include_eager_conditions?(options) || include_eager_order?(options)
           sql = "SELECT "
-          sql << "DISTINCT #{table_name}." if include_eager_conditions?(options) || include_eager_order?(options)
-          sql << primary_key
+          if is_distinct
+            ordered_columns = options[:order].to_s.split(',').collect! { |s| s.split.first }
+            sql << connection.distinct("#{table_name}.#{primary_key}", ordered_columns)
+          else
+            sql << primary_key
+          end
           sql << " FROM #{table_name} "
           
-          if include_eager_conditions?(options) || include_eager_order?(options)
+          if is_distinct
             sql << join_dependency.join_associations.collect(&:association_join).join
             add_joins!(sql, options, scope)
           end
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
index 1ed761b126..a098c02829 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
@@ -279,6 +279,11 @@ module ActiveRecord
         sql << " DEFAULT #{quote(options[:default], options[:column])}" unless options[:default].nil?
         sql << " NOT NULL" if options[:null] == false
       end
+
+      # SELECT DISTINCT clause for a given set of columns.  PostgreSQL overrides this for custom DISTINCT syntax.
+      def distinct(columns, *order_columns)
+        "DISTINCT #{columns}"
+      end
     end
   end
 end
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
index a2eaac549e..975768a758 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -186,7 +186,6 @@ module ActiveRecord
         execute "ROLLBACK"
       end
 
-
       # SCHEMA STATEMENTS ========================================
 
       # Return the list of all tables in the schema search path.
@@ -384,6 +383,16 @@ module ActiveRecord
           'bigint'
         end
       end
+      
+      # PostgreSQL requires the ORDER BY columns in the select list for distinct queries.
+      # If you select distinct by a column though, you must pass that column in the order by clause too:
+      #
+      #   distinct("posts.id", 'posts.id', 'posts.created_at')
+      def distinct(columns, *order_columns)
+        order_columns.delete_if &:blank?
+        sql = "DISTINCT ON (#{columns}) #{columns}"
+        sql << (order_columns.any? ? ", #{order_columns * ', '}" : '')
+      end
 
       private
         BYTEA_COLUMN_TYPE_OID = 17
diff --git a/activerecord/test/associations_test.rb b/activerecord/test/associations_test.rb
index d0a80a0f0f..a955a29fc1 100755
--- a/activerecord/test/associations_test.rb
+++ b/activerecord/test/associations_test.rb
@@ -1783,7 +1783,6 @@ class HasAndBelongsToManyAssociationsTest < Test::Unit::TestCase
     assert_equal projects(:action_controller), developer.projects[1] 
   end
   
-  # this bug highlights an issue in postgresql. fixed in edge, but i had to rollback in [5265].
   def test_select_limited_ids_list
     # Set timestamps
     Developer.transaction do
@@ -1794,7 +1793,7 @@ class HasAndBelongsToManyAssociationsTest < Test::Unit::TestCase
     
     join_base = ActiveRecord::Associations::ClassMethods::JoinDependency::JoinBase.new(Project)
     join_dep  = ActiveRecord::Associations::ClassMethods::JoinDependency.new(join_base, :developers, nil)
-    projects  = Project.send(:select_limited_ids_list, {:order => 'developers.created_at'}, join_dep)
+    projects  = Project.send(:select_limited_ids_list, {:order => 'projects.id, developers.created_at'}, join_dep)
     assert_equal "'1', '2'", projects
   end
 end