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

@@ -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]

activerecord/lib/active_record/associations.rb

@@ -1193,12 +1193,17 @@ module ActiveRecord
  
         def construct_finder_sql_for_association_limiting(options, join_dependency)
           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)
+          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

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

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.
@@ -385,6 +384,16 @@ module ActiveRecord
         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
         NUMERIC_COLUMN_TYPE_OID = 1700

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