From 767edafc69a56996ec5517f96bd90bbbdba35fed Mon Sep 17 00:00:00 2001
From: Ryuta Kamizono <kamipo@gmail.com>
Date: Sat, 2 May 2020 14:19:54 +0900
Subject: [PATCH] Fix `minimum` and `maximum` on non numeric column

I supposed all aggregation functions will return numeric result in
#39039, but that assumption was incorrect for `minimum` and `maximum`,
if an aggregated column is non numeric type.

I've restored type casting aggregated result for `minimum` and `maximum`.

Fixes #39110.
---
 .../active_record/relation/calculations.rb    | 54 ++++++++++++++-----
 activerecord/test/cases/calculations_test.rb  |  7 +++
 2 files changed, 48 insertions(+), 13 deletions(-)

diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb
index f2d17e090d..decf847144 100644
--- a/activerecord/lib/active_record/relation/calculations.rb
+++ b/activerecord/lib/active_record/relation/calculations.rb
@@ -304,8 +304,17 @@ module ActiveRecord
           query_builder = relation.arel
         end
 
-        value = skip_query_cache_if_necessary { @klass.connection.select_value(query_builder) }
-        type_cast_calculated_value(value, operation)
+        result = skip_query_cache_if_necessary { @klass.connection.select_all(query_builder) }
+
+        type_cast_calculated_value(result.cast_values.first, operation) do |value|
+          if value.is_a?(String) &&
+              column = klass.columns_hash[column_name.to_s]
+            type = connection.lookup_cast_type_from_column(column)
+            type.deserialize(value)
+          else
+            value
+          end
+        end
       end
 
       def execute_grouped_calculation(operation, column_name, distinct) #:nodoc:
@@ -351,17 +360,33 @@ module ActiveRecord
           key_records = key_records.index_by(&:id)
         end
 
-        calculated_data.each_with_object({}) do |row, result|
-          key = group_columns.map { |aliaz, col_name|
-            type = type_for(col_name) do
-              calculated_data.column_types.fetch(aliaz, Type.default_value)
-            end
-            type.deserialize(row[aliaz])
-          }
+        key_types = group_columns.each_with_object({}) do |(aliaz, col_name), types|
+          types[aliaz] = type_for(col_name) do
+            calculated_data.column_types.fetch(aliaz, Type.default_value)
+          end
+        end
+
+        hash_rows = calculated_data.cast_values(key_types).map! do |row|
+          calculated_data.columns.each_with_object({}).with_index do |(column, hash), i|
+            hash[column] = row[i]
+          end
+        end
+
+        type = nil
+        hash_rows.each_with_object({}) do |row, result|
+          key = group_aliases.map { |aliaz| row[aliaz] }
           key = key.first if key.size == 1
           key = key_records[key] if associated
 
-          result[key] = type_cast_calculated_value(row[column_alias], operation)
+          result[key] = type_cast_calculated_value(row[column_alias], operation) do |value|
+            if value.is_a?(String) &&
+                (type || column = klass.columns_hash[column_name.to_s])
+              type ||= connection.lookup_cast_type_from_column(column)
+              type.deserialize(value)
+            else
+              value
+            end
+          end
         end
       end
 
@@ -389,9 +414,12 @@ module ActiveRecord
 
       def type_cast_calculated_value(value, operation)
         case operation
-        when "sum"     then value || 0
-        when "average" then value&.respond_to?(:to_d) ? value.to_d : value
-        else value
+        when "count", "sum"
+          value || 0
+        when "average"
+          value&.respond_to?(:to_d) ? value.to_d : value
+        else # "minimum", "maximum"
+          yield value
         end
       end
 
diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb
index 3c226669e9..abf051e625 100644
--- a/activerecord/test/cases/calculations_test.rb
+++ b/activerecord/test/cases/calculations_test.rb
@@ -1023,6 +1023,13 @@ class CalculationsTest < ActiveRecord::TestCase
     assert_equal({ "proposed" => 0, "published" => 1 }, Book.group(:status).maximum(:difficulty))
   end
 
+  def test_minimum_and_maximum_on_non_numeric_type
+    assert_equal Date.new(2004, 4, 15), Topic.minimum(:last_read)
+    assert_equal Date.new(2004, 4, 15), Topic.maximum(:last_read)
+    assert_equal({ false => Date.new(2004, 4, 15), true => nil }, Topic.group(:approved).minimum(:last_read))
+    assert_equal({ false => Date.new(2004, 4, 15), true => nil }, Topic.group(:approved).maximum(:last_read))
+  end
+
   def test_select_avg_with_group_by_as_virtual_attribute_with_sql
     rails_core = companies(:rails_core)