Skip to content

Commit 39f8402

Browse files
committed
Always prefer class types to query types when casting group
When `group` is used in combination with any calculation method, the resulting hash uses the grouping expression as the key. Currently we're incorrectly always favoring the type reported by the query, instead of the type known by the class. This causes differing behavior depending on whether the adaptor actually gives proper types with the query or not. After this change, the behavior will be the same on all adaptors -- we see if we know the type from the class, fall back to the type from the query, and finally fall back to the identity type. Fixes rails#25595
1 parent ca71ddf commit 39f8402

File tree

4 files changed

+23
-6
lines changed

4 files changed

+23
-6
lines changed

activerecord/CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,8 @@
1+
* Using `group` with an attribute that has a custom type will properly cast
2+
the hash keys after calling a calculation method like `count`. Fixes #25595.
3+
4+
*Sean Griffin*
5+
16
* Ensure concurrent invocations of the connection reaper cannot allocate the
27
same connection to two threads.
38

activerecord/lib/active_record/model_schema.rb

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -278,8 +278,12 @@ def attribute_types # :nodoc:
278278
#
279279
# +attr_name+ The name of the attribute to retrieve the type for. Must be
280280
# a string
281-
def type_for_attribute(attr_name)
282-
attribute_types[attr_name]
281+
def type_for_attribute(attr_name, &block)
282+
if block
283+
attribute_types.fetch(attr_name, &block)
284+
else
285+
attribute_types[attr_name]
286+
end
283287
end
284288

285289
# Returns a hash where the keys are column names and the values are

activerecord/lib/active_record/relation/calculations.rb

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -308,8 +308,10 @@ def execute_grouped_calculation(operation, column_name, distinct) #:nodoc:
308308

309309
Hash[calculated_data.map do |row|
310310
key = group_columns.map { |aliaz, col_name|
311-
column = calculated_data.column_types.fetch(aliaz) do
312-
type_for(col_name)
311+
column = type_for(col_name) do
312+
calculated_data.column_types.fetch(aliaz) do
313+
Type::Value.new
314+
end
313315
end
314316
type_cast_calculated_value(row[aliaz], column)
315317
}
@@ -342,9 +344,9 @@ def column_alias_for(keys)
342344
@klass.connection.table_alias_for(table_name)
343345
end
344346

345-
def type_for(field)
347+
def type_for(field, &block)
346348
field_name = field.respond_to?(:name) ? field.name.to_s : field.to_s.split('.').last
347-
@klass.type_for_attribute(field_name)
349+
@klass.type_for_attribute(field_name, &block)
348350
end
349351

350352
def type_cast_calculated_value(value, type, operation = nil)

activerecord/test/cases/calculations_test.rb

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
require "cases/helper"
2+
require "models/book"
23
require 'models/club'
34
require 'models/company'
45
require "models/contract"
@@ -789,4 +790,9 @@ def permit!
789790
assert_equal 50, result[1].credit_limit
790791
assert_equal 50, result[2].credit_limit
791792
end
793+
794+
def test_group_by_attribute_with_custom_type
795+
Book.create!(status: :published)
796+
assert_equal({ "published" => 1 }, Book.group(:status).count)
797+
end
792798
end

0 commit comments

Comments
 (0)