Merge pull request #9683 from senny/deprecate_count_distinct_option

rename `Relation#uniq` to `Relation#distinct`

8 files changed, 42 insertions, 25 deletions

diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb
index 06da5f2e0d..0c670bdaa1 100644
--- a/activerecord/lib/active_record/associations.rb
+++ b/activerecord/lib/active_record/associations.rb
@@ -241,6 +241,7 @@ module ActiveRecord
     #   others.destroy_all                |   X   |    X     |    X
     #   others.find(*args)                |   X   |    X     |    X
     #   others.exists?                    |   X   |    X     |    X
+    #   others.distinct                   |   X   |    X     |    X
     #   others.uniq                       |   X   |    X     |    X
     #   others.reset                      |   X   |    X     |    X
     #
diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb
index c992f51dbb..18b7dc3668 100644
--- a/activerecord/lib/active_record/associations/collection_association.rb
+++ b/activerecord/lib/active_record/associations/collection_association.rb
@@ -174,13 +174,14 @@ module ActiveRecord
 
           reflection.klass.count_by_sql(custom_counter_sql)
         else
-          if association_scope.uniq_value
+          relation = scope
+          if association_scope.distinct_value
             # This is needed because 'SELECT count(DISTINCT *)..' is not valid SQL.
             column_name ||= reflection.klass.primary_key
-            count_options[:distinct] = true
+            relation = relation.distinct
           end
 
-          value = scope.count(column_name, count_options)
+          value = relation.count(column_name)
 
           limit  = options[:limit]
           offset = options[:offset]
@@ -246,14 +247,14 @@ module ActiveRecord
       # +count_records+, which is a method descendants have to provide.
       def size
         if !find_target? || loaded?
-          if association_scope.uniq_value
+          if association_scope.distinct_value
             target.uniq.size
           else
             target.size
           end
         elsif !loaded? && !association_scope.group_values.empty?
           load_target.size
-        elsif !loaded? && !association_scope.uniq_value && target.is_a?(Array)
+        elsif !loaded? && !association_scope.distinct_value && target.is_a?(Array)
           unsaved_records = target.select { |r| r.new_record? }
           unsaved_records.size + count_records
         else
@@ -306,12 +307,13 @@ module ActiveRecord
         end
       end
 
-      def uniq
+      def distinct
         seen = {}
         load_target.find_all do |record|
           seen[record.id] = true unless seen.key?(record.id)
         end
       end
+      alias uniq distinct
 
       # Replace this collection with +other_array+. This will perform a diff
       # and delete/add only records that have changed.
@@ -352,7 +354,7 @@ module ActiveRecord
         callback(:before_add, record)
         yield(record) if block_given?
 
-        if association_scope.uniq_value && index = @target.index(record)
+        if association_scope.distinct_value && index = @target.index(record)
           @target[index] = record
         else
           @target << record
diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb
index 543204abac..c2add32aa6 100644
--- a/activerecord/lib/active_record/associations/collection_proxy.rb
+++ b/activerecord/lib/active_record/associations/collection_proxy.rb
@@ -649,11 +649,12 @@ module ActiveRecord
       #   #      #<Pet name: "Fancy-Fancy">
       #   #    ]
       #
-      #   person.pets.select(:name).uniq
+      #   person.pets.select(:name).distinct
       #   # => [#<Pet name: "Fancy-Fancy">]
-      def uniq
-        @association.uniq
+      def distinct
+        @association.distinct
       end
+      alias uniq distinct
 
       # Count all records using SQL.
       #
diff --git a/activerecord/lib/active_record/associations/preloader/has_many_through.rb b/activerecord/lib/active_record/associations/preloader/has_many_through.rb
index 9a662d3f53..38bc7ce7da 100644
--- a/activerecord/lib/active_record/associations/preloader/has_many_through.rb
+++ b/activerecord/lib/active_record/associations/preloader/has_many_through.rb
@@ -6,7 +6,7 @@ module ActiveRecord
 
         def associated_records_by_owner
           super.each do |owner, records|
-            records.uniq! if reflection_scope.uniq_value
+            records.uniq! if reflection_scope.distinct_value
           end
         end
       end
diff --git a/activerecord/lib/active_record/querying.rb b/activerecord/lib/active_record/querying.rb
index e04a3d0976..902fd90c54 100644
--- a/activerecord/lib/active_record/querying.rb
+++ b/activerecord/lib/active_record/querying.rb
@@ -8,7 +8,7 @@ module ActiveRecord
     delegate :find_each, :find_in_batches, :to => :all
     delegate :select, :group, :order, :except, :reorder, :limit, :offset, :joins,
              :where, :preload, :eager_load, :includes, :from, :lock, :readonly,
-             :having, :create_with, :uniq, :references, :none, :to => :all
+             :having, :create_with, :uniq, :distinct, :references, :none, :to => :all
     delegate :count, :average, :minimum, :maximum, :sum, :calculate, :pluck, :ids, :to => :all
 
     # Executes a custom SQL query against your database and returns all the results. The results will
diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb
index ad54ba55b6..50723b4848 100644
--- a/activerecord/lib/active_record/relation.rb
+++ b/activerecord/lib/active_record/relation.rb
@@ -10,7 +10,7 @@ module ActiveRecord
                             :extending]
 
     SINGLE_VALUE_METHODS = [:limit, :offset, :lock, :readonly, :from, :reordering,
-                            :reverse_order, :uniq, :create_with]
+                            :reverse_order, :distinct, :create_with]
 
     VALUE_METHODS = MULTI_VALUE_METHODS + SINGLE_VALUE_METHODS
 
@@ -506,6 +506,12 @@ module ActiveRecord
       includes_values & joins_values
     end
 
+    # +uniq+ and +uniq!+ are silently deprecated. +uniq_value+ delegates to +distinct_value+
+    # to maintain backwards compatibility. Use +distinct_value+ instead.
+    def uniq_value
+      distinct_value
+    end
+
     # Compares two relations for equality.
     def ==(other)
       case other
diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb
index 7f95181c67..be011b22af 100644
--- a/activerecord/lib/active_record/relation/calculations.rb
+++ b/activerecord/lib/active_record/relation/calculations.rb
@@ -11,7 +11,7 @@ module ActiveRecord
     #   Person.count(:all)
     #   # => performs a COUNT(*) (:all is an alias for '*')
     #
-    #   Person.count(:age, distinct: true)
+    #   Person.distinct.count(:age)
     #   # => counts the number of different age values
     #
     # If +count+ is used with +group+, it returns a Hash whose keys represent the aggregated column,
@@ -198,8 +198,13 @@ module ActiveRecord
     def perform_calculation(operation, column_name, options = {})
       operation = operation.to_s.downcase
 
-      # If #count is used in conjuction with #uniq it is considered distinct. (eg. relation.uniq.count)
-      distinct = options[:distinct] || self.uniq_value
+      # If #count is used with #distinct / #uniq it is considered distinct. (eg. relation.distinct.count)
+      distinct = self.distinct_value
+      if options.has_key?(:distinct)
+        ActiveSupport::Deprecation.warn "The :distinct option for `Relation#count` is deprecated. " \
+          "Please use `Relation#distinct` instead. (eg. `relation.distinct.count`)"
+        distinct = options[:distinct]
+      end
 
       if operation == "count"
         column_name ||= (select_for_count || :all)
diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb
index b7960936cf..10a31109d5 100644
--- a/activerecord/lib/active_record/relation/query_methods.rb
+++ b/activerecord/lib/active_record/relation/query_methods.rb
@@ -710,20 +710,22 @@ module ActiveRecord
     #   User.select(:name)
     #   # => Might return two records with the same name
     #
-    #   User.select(:name).uniq
-    #   # => Returns 1 record per unique name
+    #   User.select(:name).distinct
+    #   # => Returns 1 record per distinct name
     #
-    #   User.select(:name).uniq.uniq(false)
+    #   User.select(:name).distinct.distinct(false)
     #   # => You can also remove the uniqueness
-    def uniq(value = true)
-      spawn.uniq!(value)
+    def distinct(value = true)
+      spawn.distinct!(value)
     end
+    alias uniq distinct
 
-    # Like #uniq, but modifies relation in place.
-    def uniq!(value = true) # :nodoc:
-      self.uniq_value = value
+    # Like #distinct, but modifies relation in place.
+    def distinct!(value = true) # :nodoc:
+      self.distinct_value = value
       self
     end
+    alias uniq! distinct!
 
     # Used to extend a scope with additional methods, either through
     # a module or through a block provided.
@@ -814,7 +816,7 @@ module ActiveRecord
 
       build_select(arel, select_values.uniq)
 
-      arel.distinct(uniq_value)
+      arel.distinct(distinct_value)
       arel.from(build_from) if from_value
       arel.lock(lock_value) if lock_value