aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord
diff options
context:
space:
mode:
authorSean Griffin <sean@seantheprogrammer.com>2016-07-11 10:43:02 -0400
committerSean Griffin <sean@seantheprogrammer.com>2016-07-11 10:57:37 -0400
commita45363a2fb53e0a016f33dd211c00b5d81764379 (patch)
treea6000f5b52353c689b6e9867ab0b73a13df3143a /activerecord
parent79bc06647ccb77840877f2d8e3dcf620460d7306 (diff)
downloadrails-a45363a2fb53e0a016f33dd211c00b5d81764379.tar.gz
rails-a45363a2fb53e0a016f33dd211c00b5d81764379.tar.bz2
rails-a45363a2fb53e0a016f33dd211c00b5d81764379.zip
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 #25595
Diffstat (limited to 'activerecord')
-rw-r--r--activerecord/CHANGELOG.md5
-rw-r--r--activerecord/lib/active_record/model_schema.rb8
-rw-r--r--activerecord/lib/active_record/relation/calculations.rb10
-rw-r--r--activerecord/test/cases/calculations_test.rb6
4 files changed, 23 insertions, 6 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 5d04551125..c91e6662c9 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,8 @@
+* Using `group` with an attribute that has a custom type will properly cast
+ the hash keys after calling a calculation method like `count`. Fixes #25595.
+
+ *Sean Griffin*
+
* Fix the generated `#to_param` method to use `omission: ''` so that
the resulting output is actually up to 20 characters, not
effectively 17 to leave room for the default "...".
diff --git a/activerecord/lib/active_record/model_schema.rb b/activerecord/lib/active_record/model_schema.rb
index 7996c32bbc..114686c5d3 100644
--- a/activerecord/lib/active_record/model_schema.rb
+++ b/activerecord/lib/active_record/model_schema.rb
@@ -282,8 +282,12 @@ module ActiveRecord
#
# +attr_name+ The name of the attribute to retrieve the type for. Must be
# a string
- def type_for_attribute(attr_name)
- attribute_types[attr_name]
+ def type_for_attribute(attr_name, &block)
+ if block
+ attribute_types.fetch(attr_name, &block)
+ else
+ attribute_types[attr_name]
+ end
end
# Returns a hash where the keys are column names and the values are
diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb
index d6d92b8607..a97b71815a 100644
--- a/activerecord/lib/active_record/relation/calculations.rb
+++ b/activerecord/lib/active_record/relation/calculations.rb
@@ -312,8 +312,10 @@ module ActiveRecord
Hash[calculated_data.map do |row|
key = group_columns.map { |aliaz, col_name|
- column = calculated_data.column_types.fetch(aliaz) do
- type_for(col_name)
+ column = type_for(col_name) do
+ calculated_data.column_types.fetch(aliaz) do
+ Type::Value.new
+ end
end
type_cast_calculated_value(row[aliaz], column)
}
@@ -346,9 +348,9 @@ module ActiveRecord
@klass.connection.table_alias_for(table_name)
end
- def type_for(field)
+ def type_for(field, &block)
field_name = field.respond_to?(:name) ? field.name.to_s : field.to_s.split('.').last
- @klass.type_for_attribute(field_name)
+ @klass.type_for_attribute(field_name, &block)
end
def type_cast_calculated_value(value, type, operation = nil)
diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb
index cfae700159..06c2f10dc6 100644
--- a/activerecord/test/cases/calculations_test.rb
+++ b/activerecord/test/cases/calculations_test.rb
@@ -1,4 +1,5 @@
require "cases/helper"
+require "models/book"
require 'models/club'
require 'models/company'
require "models/contract"
@@ -793,4 +794,9 @@ class CalculationsTest < ActiveRecord::TestCase
assert_equal 50, result[1].credit_limit
assert_equal 50, result[2].credit_limit
end
+
+ def test_group_by_attribute_with_custom_type
+ Book.create!(status: :published)
+ assert_equal({ "published" => 1 }, Book.group(:status).count)
+ end
end