Merge pull request rails#39255 from kamipo/fix_min_and_max_on_tz_aware_attributes

Fix `minimum` and `maximum` on time zone aware attributes

Author: kamipo

activerecord/CHANGELOG.md

@@ -62,10 +62,6 @@
 
     *Ryuta Kamizono*
 
-*   Fix aggregate functions to return numeric value consistently even on custom attribute type.
-
-    *Ryuta Kamizono*
-
 *   Support bulk insert/upsert on relation to preserve scope values.
 
     *Josef Šimánek*, *Ryuta Kamizono*

activerecord/lib/active_record/relation/calculations.rb

@@ -307,9 +307,7 @@ def execute_simple_calculation(operation, column_name, distinct) #:nodoc:
         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)
+          if type = klass.attribute_types[column_name.to_s]
             type.deserialize(value)
           else
             value
@@ -379,9 +377,7 @@ def execute_grouped_calculation(operation, column_name, distinct) #:nodoc:
           key = key_records[key] if associated
 
           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)
+            if type ||= klass.attribute_types[column_name.to_s]
               type.deserialize(value)
             else
               value

activerecord/test/cases/calculations_test.rb

@@ -1044,12 +1044,12 @@ def test_group_by_attribute_with_custom_type
   def test_aggregate_attribute_on_custom_type
     assert_equal 4, Book.sum(:status)
     assert_equal 1, Book.sum(:difficulty)
-    assert_equal 0, Book.minimum(:status)
-    assert_equal 1, Book.maximum(:difficulty)
+    assert_equal "easy", Book.minimum(:difficulty)
+    assert_equal "medium", Book.maximum(:difficulty)
     assert_equal({ "proposed" => 0, "published" => 4 }, Book.group(:status).sum(:status))
     assert_equal({ "proposed" => 0, "published" => 1 }, Book.group(:status).sum(:difficulty))
-    assert_equal({ "proposed" => 0, "published" => 2 }, Book.group(:status).minimum(:status))
-    assert_equal({ "proposed" => 0, "published" => 1 }, Book.group(:status).maximum(:difficulty))
+    assert_equal({ "proposed" => "easy", "published" => "easy" }, Book.group(:status).minimum(:difficulty))
+    assert_equal({ "proposed" => "easy", "published" => "medium" }, Book.group(:status).maximum(:difficulty))
   end
 
   def test_minimum_and_maximum_on_non_numeric_type
@@ -1059,6 +1059,48 @@ def test_minimum_and_maximum_on_non_numeric_type
     assert_equal({ false => Date.new(2004, 4, 15), true => nil }, Topic.group(:approved).maximum(:last_read))
   end
 
+  def test_minimum_and_maximum_on_time_attributes
+    assert_minimum_and_maximum_on_time_attributes(Time)
+  end
+
+  def test_minimum_and_maximum_on_tz_aware_attributes
+    with_timezone_config aware_attributes: true, zone: "Pacific Time (US & Canada)" do
+      Topic.reset_column_information
+      assert_minimum_and_maximum_on_time_attributes(ActiveSupport::TimeWithZone)
+    end
+  ensure
+    Topic.reset_column_information
+  end
+
+  def assert_minimum_and_maximum_on_time_attributes(time_class)
+    actual = Topic.minimum(:written_on)
+    assert_equal Time.utc(2003, 7, 16, 14, 28, 11, 223300), actual
+    assert_instance_of time_class, actual
+
+    actual = Topic.maximum(:written_on)
+    assert_equal Time.utc(2013, 7, 13, 11, 11, 0, 9900), actual
+    assert_instance_of time_class, actual
+
+    expected = {
+      false => Time.utc(2003, 7, 16, 14, 28, 11, 223300),
+      true => Time.utc(2004, 7, 15, 14, 28, 0, 9900),
+    }
+    actual = Topic.group(:approved).minimum(:written_on)
+    assert_equal expected, actual
+    assert_instance_of time_class, actual[true]
+    assert_instance_of time_class, actual[true]
+
+    expected = {
+      false => Time.utc(2003, 7, 16, 14, 28, 11, 223300),
+      true => Time.utc(2013, 7, 13, 11, 11, 0, 9900),
+    }
+    actual = Topic.group(:approved).maximum(:written_on)
+    assert_equal expected, actual
+    assert_instance_of time_class, actual[true]
+    assert_instance_of time_class, actual[true]
+  end
+  private :assert_minimum_and_maximum_on_time_attributes
+
   def test_select_avg_with_group_by_as_virtual_attribute_with_sql
     rails_core = companies(:rails_core)