1 files changed, 53 insertions, 25 deletions

diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb
index 35c670f1a1..cb0b06cfdc 100644
--- a/activerecord/lib/active_record/relation/calculations.rb
+++ b/activerecord/lib/active_record/relation/calculations.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 module ActiveRecord
   module Calculations
     # Count the records.
@@ -38,10 +40,16 @@ module ActiveRecord
     # between databases. In invalid cases, an error from the database is thrown.
     def count(column_name = nil)
       if block_given?
-        to_a.count { |*block_args| yield(*block_args) }
-      else
-        calculate(:count, column_name)
+        unless column_name.nil?
+          ActiveSupport::Deprecation.warn \
+            "When `count' is called with a block, it ignores other arguments. " \
+            "This behavior is now deprecated and will result in an ArgumentError in Rails 6.0."
+        end
+
+        return super()
       end
+
+      calculate(:count, column_name)
     end
 
     # Calculates the average value on a given column. Returns +nil+ if there's
@@ -75,8 +83,17 @@ module ActiveRecord
     # #calculate for examples with options.
     #
     #   Person.sum(:age) # => 4562
-    def sum(column_name = nil, &block)
-      return super(&block) if block_given?
+    def sum(column_name = nil)
+      if block_given?
+        unless column_name.nil?
+          ActiveSupport::Deprecation.warn \
+            "When `sum' is called with a block, it ignores other arguments. " \
+            "This behavior is now deprecated and will result in an ArgumentError in Rails 6.0."
+        end
+
+        return super()
+      end
+
       calculate(:sum, column_name)
     end
 
@@ -113,8 +130,8 @@ module ActiveRecord
     #      end
     def calculate(operation, column_name)
       if has_include?(column_name)
-        relation = construct_relation_for_association_calculations
-        relation = relation.distinct if operation.to_s.downcase == "count"
+        relation = apply_join_dependency
+        relation.distinct! if operation.to_s.downcase == "count"
 
         relation.calculate(operation, column_name)
       else
@@ -163,13 +180,15 @@ module ActiveRecord
       end
 
       if has_include?(column_names.first)
-        construct_relation_for_association_calculations.pluck(*column_names)
+        relation = apply_join_dependency
+        relation.pluck(*column_names)
       else
+        enforce_raw_sql_whitelist(column_names)
         relation = spawn
         relation.select_values = column_names.map { |cn|
           @klass.has_attribute?(cn) || @klass.attribute_alias?(cn) ? arel_attribute(cn) : cn
         }
-        result = klass.connection.select_all(relation.arel, nil, bound_attributes)
+        result = skip_query_cache_if_necessary { klass.connection.select_all(relation.arel, nil) }
         result.cast_values(klass.attribute_types)
       end
     end
@@ -197,8 +216,13 @@ module ActiveRecord
 
         if operation == "count"
           column_name ||= select_for_count
-          column_name = primary_key if column_name == :all && distinct
-          distinct = nil if column_name =~ /\s*DISTINCT[\s(]+/i
+          if column_name == :all
+            if distinct && (group_values.any? || select_values.empty? && order_values.empty?)
+              column_name = primary_key
+            end
+          elsif column_name =~ /\s*DISTINCT[\s(]+/i
+            distinct = nil
+          end
         end
 
         if group_values.any?
@@ -225,18 +249,21 @@ module ActiveRecord
       def execute_simple_calculation(operation, column_name, distinct) #:nodoc:
         column_alias = column_name
 
-        if operation == "count" && (limit_value || offset_value)
+        if operation == "count" && (column_name == :all && distinct || has_limit_or_offset?)
           # Shortcut when limit is zero.
           return 0 if limit_value == 0
 
           query_builder = build_count_subquery(spawn, column_name, distinct)
         else
           # PostgreSQL doesn't like ORDER BY when there are no GROUP BY
-          relation = unscope(:order)
+          relation = unscope(:order).distinct!(false)
 
           column = aggregate_column(column_name)
 
           select_value = operation_over_aggregate_column(column, operation, distinct)
+          if operation == "sum" && distinct
+            select_value.distinct = true
+          end
 
           column_alias = select_value.alias
           column_alias ||= @klass.connection.column_name_for_operation(operation, select_value)
@@ -245,7 +272,7 @@ module ActiveRecord
           query_builder = relation.arel
         end
 
-        result = @klass.connection.select_all(query_builder, nil, bound_attributes)
+        result = skip_query_cache_if_necessary { @klass.connection.select_all(query_builder, nil) }
         row    = result.first
         value  = row && row.values.first
         type   = result.column_types.fetch(column_alias) do
@@ -282,7 +309,7 @@ module ActiveRecord
             operation,
             distinct).as(aggregate_alias)
         ]
-        select_values += select_values unless having_clause.empty?
+        select_values += self.select_values unless having_clause.empty?
 
         select_values.concat group_columns.map { |aliaz, field|
           if field.respond_to?(:as)
@@ -292,11 +319,11 @@ module ActiveRecord
           end
         }
 
-        relation = except(:group)
+        relation = except(:group).distinct!(false)
         relation.group_values  = group_fields
         relation.select_values = select_values
 
-        calculated_data = @klass.connection.select_all(relation, nil, relation.bound_attributes)
+        calculated_data = skip_query_cache_if_necessary { @klass.connection.select_all(relation.arel, nil) }
 
         if association
           key_ids     = calculated_data.collect { |row| row[group_aliases.first] }
@@ -364,16 +391,17 @@ module ActiveRecord
       end
 
       def build_count_subquery(relation, column_name, distinct)
-        column_alias = Arel.sql("count_column")
-        subquery_alias = Arel.sql("subquery_for_count")
+        if column_name == :all
+          relation.select_values = [ Arel.sql(FinderMethods::ONE_AS_ONE) ] unless distinct
+        else
+          column_alias = Arel.sql("count_column")
+          relation.select_values = [ aggregate_column(column_name).as(column_alias) ]
+        end
 
-        aliased_column = aggregate_column(column_name == :all ? 1 : column_name).as(column_alias)
-        relation.select_values = [aliased_column]
-        subquery = relation.arel.as(subquery_alias)
+        subquery = relation.arel.as(Arel.sql("subquery_for_count"))
+        select_value = operation_over_aggregate_column(column_alias || Arel.star, "count", false)
 
-        sm = Arel::SelectManager.new relation.engine
-        select_value = operation_over_aggregate_column(column_alias, "count", distinct)
-        sm.project(select_value).from(subquery)
+        Arel::SelectManager.new(subquery).project(select_value)
       end
   end
 end