Skip to content
This repository
Browse code

Merge pull request #8289 from semaperepelitsa/pg_adapter_refactoring_…

…squashed

Refactoring, testing and documenting pg_connection.distinct
  • Loading branch information...
commit 6b266c21b2cc649505e5f31c74e1f450f3287981 2 parents f058e56 + 2197e1f
Rafael Mendonça França authored November 21, 2012
23  activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb
@@ -429,20 +429,17 @@ def type_to_sql(type, limit = nil, precision = nil, scale = nil)
429 429
         # PostgreSQL requires the ORDER BY columns in the select list for distinct queries, and
430 430
         # requires that the ORDER BY include the distinct column.
431 431
         #
432  
-        #   distinct("posts.id", "posts.created_at desc")
  432
+        #   distinct("posts.id", ["posts.created_at desc"])
  433
+        #   # => "DISTINCT posts.id, posts.created_at AS alias_0"
433 434
         def distinct(columns, orders) #:nodoc:
434  
-          return "DISTINCT #{columns}" if orders.empty?
435  
-
436  
-          # Construct a clean list of column names from the ORDER BY clause, removing
437  
-          # any ASC/DESC modifiers
438  
-          order_columns = orders.collect do |s|
439  
-            s = s.to_sql unless s.is_a?(String)
440  
-            s.gsub(/\s+(ASC|DESC)\s*(NULLS\s+(FIRST|LAST)\s*)?/i, '')
441  
-          end
442  
-          order_columns.delete_if { |c| c.blank? }
443  
-          order_columns = order_columns.zip((0...order_columns.size).to_a).map { |s,i| "#{s} AS alias_#{i}" }
444  
-
445  
-          "DISTINCT #{columns}, #{order_columns * ', '}"
  435
+          order_columns = orders.map{ |s|
  436
+              # Convert Arel node to string
  437
+              s = s.to_sql unless s.is_a?(String)
  438
+              # Remove any ASC/DESC modifiers
  439
+              s.gsub(/\s+(ASC|DESC)\s*(NULLS\s+(FIRST|LAST)\s*)?/i, '')
  440
+            }.reject(&:blank?).map.with_index { |column, i| "#{column} AS alias_#{i}" }
  441
+
  442
+          [super].concat(order_columns).join(', ')
446 443
         end
447 444
       end
448 445
     end
29  activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb
@@ -216,6 +216,35 @@ def test_partial_index
216 216
         assert_equal "(number > 100)", index.where
217 217
       end
218 218
 
  219
+      def test_distinct_zero_orders
  220
+        assert_equal "DISTINCT posts.id",
  221
+          @connection.distinct("posts.id", [])
  222
+      end
  223
+
  224
+      def test_distinct_one_order
  225
+        assert_equal "DISTINCT posts.id, posts.created_at AS alias_0",
  226
+          @connection.distinct("posts.id", ["posts.created_at desc"])
  227
+      end
  228
+
  229
+      def test_distinct_few_orders
  230
+        assert_equal "DISTINCT posts.id, posts.created_at AS alias_0, posts.position AS alias_1",
  231
+          @connection.distinct("posts.id", ["posts.created_at desc", "posts.position asc"])
  232
+      end
  233
+
  234
+      def test_distinct_blank_not_nil_orders
  235
+        assert_equal "DISTINCT posts.id, posts.created_at AS alias_0",
  236
+          @connection.distinct("posts.id", ["posts.created_at desc", "", "   "])
  237
+      end
  238
+
  239
+      def test_distinct_with_arel_order
  240
+        order = Object.new
  241
+        def order.to_sql
  242
+          "posts.created_at desc"
  243
+        end
  244
+        assert_equal "DISTINCT posts.id, posts.created_at AS alias_0",
  245
+          @connection.distinct("posts.id", [order])
  246
+      end
  247
+
219 248
       def test_distinct_with_nulls
220 249
         assert_equal "DISTINCT posts.title, posts.updater_id AS alias_0", @connection.distinct("posts.title", ["posts.updater_id desc nulls first"])
221 250
         assert_equal "DISTINCT posts.title, posts.updater_id AS alias_0", @connection.distinct("posts.title", ["posts.updater_id desc nulls last"])

0 notes on commit 6b266c2

Please sign in to comment.
Something went wrong with that request. Please try again.