Merge pull request #15847 from sgrif/sg-encapsulate-result-types

Encapsulate knowledge of type objects on `ActiveRecord::Result`

5 files changed, 59 insertions, 28 deletions

diff --git a/activerecord/lib/active_record/associations/join_dependency.rb b/activerecord/lib/active_record/associations/join_dependency.rb
index fbb4551b22..ec5c189cd3 100644
--- a/activerecord/lib/active_record/associations/join_dependency.rb
+++ b/activerecord/lib/active_record/associations/join_dependency.rb
@@ -131,7 +131,6 @@ module ActiveRecord
 
       def instantiate(result_set, aliases)
         primary_key = aliases.column_alias(join_root, join_root.primary_key)
-        type_caster = result_set.column_type primary_key
 
         seen = Hash.new { |h,parent_klass|
           h[parent_klass] = Hash.new { |i,parent_id|
@@ -144,8 +143,7 @@ module ActiveRecord
         column_aliases = aliases.column_aliases join_root
 
         result_set.each { |row_hash|
-          primary_id = type_caster.type_cast_from_database row_hash[primary_key]
-          parent = parents[primary_id] ||= join_root.instantiate(row_hash, column_aliases)
+          parent = parents[row_hash[primary_key]] ||= join_root.instantiate(row_hash, column_aliases)
           construct(parent, join_root, row_hash, result_set, seen, model_cache, aliases)
         }
 
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
index 6d5151dfe4..be4ae47d09 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -344,14 +344,13 @@ module ActiveRecord
         if supports_extensions?
           res = exec_query "SELECT EXISTS(SELECT * FROM pg_available_extensions WHERE name = '#{name}' AND installed_version IS NOT NULL) as enabled",
             'SCHEMA'
-          res.column_types['enabled'].type_cast_from_database res.rows.first.first
+          res.cast_values.first
         end
       end
 
       def extensions
         if supports_extensions?
-          res = exec_query "SELECT extname from pg_extension", "SCHEMA"
-          res.rows.map { |r| res.column_types['extname'].type_cast_from_database r.first }
+          exec_query("SELECT extname from pg_extension", "SCHEMA").cast_values
         else
           super
         end
diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb
index c80ffbae92..b4ae204813 100644
--- a/activerecord/lib/active_record/relation/calculations.rb
+++ b/activerecord/lib/active_record/relation/calculations.rb
@@ -178,16 +178,7 @@ module ActiveRecord
           columns_hash.key?(cn) ? arel_table[cn] : cn
         }
         result = klass.connection.select_all(relation.arel, nil, bind_values)
-        columns = result.columns.map do |key|
-          klass.column_types.fetch(key) {
-            result.column_types.fetch(key) { result.identity_type }
-          }
-        end
-
-        result = result.rows.map do |values|
-          columns.zip(values).map { |column, value| column.type_cast_from_database value }
-        end
-        columns.one? ? result.map!(&:first) : result
+        result.cast_values(klass.column_types)
       end
     end
 
diff --git a/activerecord/lib/active_record/result.rb b/activerecord/lib/active_record/result.rb
index 8c29c69a9b..8405fdaeb9 100644
--- a/activerecord/lib/active_record/result.rb
+++ b/activerecord/lib/active_record/result.rb
@@ -42,14 +42,6 @@ module ActiveRecord
       @column_types = column_types
     end
 
-    def identity_type # :nodoc:
-      IDENTITY_TYPE
-    end
-
-    def column_type(name)
-      @column_types[name] || identity_type
-    end
-
     def each
       if block_given?
         hash_rows.each { |row| yield row }
@@ -82,6 +74,15 @@ module ActiveRecord
       hash_rows.last
     end
 
+    def cast_values(type_overrides = {}) # :nodoc:
+      types = columns.map { |name| column_type(name, type_overrides) }
+      result = rows.map do |values|
+        types.zip(values).map { |type, value| type.type_cast_from_database(value) }
+      end
+
+      columns.one? ? result.map!(&:first) : result
+    end
+
     def initialize_copy(other)
       @columns      = columns.dup
       @rows         = rows.dup
@@ -91,6 +92,12 @@ module ActiveRecord
 
     private
 
+    def column_type(name, type_overrides = {})
+      type_overrides.fetch(name) do
+        column_types.fetch(name, IDENTITY_TYPE)
+      end
+    end
+
     def hash_rows
       @hash_rows ||=
         begin
diff --git a/activerecord/test/cases/result_test.rb b/activerecord/test/cases/result_test.rb
index 2131b32a0c..d6decafad9 100644
--- a/activerecord/test/cases/result_test.rb
+++ b/activerecord/test/cases/result_test.rb
@@ -10,7 +10,7 @@ module ActiveRecord
       ])
     end
 
-    def test_to_hash_returns_row_hashes
+    test "to_hash returns row_hashes" do
       assert_equal [
         {'col_1' => 'row 1 col 1', 'col_2' => 'row 1 col 2'},
         {'col_1' => 'row 2 col 1', 'col_2' => 'row 2 col 2'},
@@ -18,13 +18,13 @@ module ActiveRecord
       ], result.to_hash
     end
 
-    def test_each_with_block_returns_row_hashes
+    test "each with block returns row hashes" do
       result.each do |row|
         assert_equal ['col_1', 'col_2'], row.keys
       end
     end
 
-    def test_each_without_block_returns_an_enumerator
+    test "each without block returns an enumerator" do
       result.each.with_index do |row, index|
         assert_equal ['col_1', 'col_2'], row.keys
         assert_kind_of Integer, index
@@ -32,9 +32,45 @@ module ActiveRecord
     end
 
     if Enumerator.method_defined? :size
-      def test_each_without_block_returns_a_sized_enumerator
+      test "each without block returns a sized enumerator" do
         assert_equal 3, result.each.size
       end
     end
+
+    test "cast_values returns rows after type casting" do
+      values = [["1.1", "2.2"], ["3.3", "4.4"]]
+      columns = ["col1", "col2"]
+      types = { "col1" => Type::Integer.new, "col2" => Type::Float.new }
+      result = Result.new(columns, values, types)
+
+      assert_equal [[1, 2.2], [3, 4.4]], result.cast_values
+    end
+
+    test "cast_values uses identity type for unknown types" do
+      values = [["1.1", "2.2"], ["3.3", "4.4"]]
+      columns = ["col1", "col2"]
+      types = { "col1" => Type::Integer.new }
+      result = Result.new(columns, values, types)
+
+      assert_equal [[1, "2.2"], [3, "4.4"]], result.cast_values
+    end
+
+    test "cast_values returns single dimensional array if single column" do
+      values = [["1.1"], ["3.3"]]
+      columns = ["col1"]
+      types = { "col1" => Type::Integer.new }
+      result = Result.new(columns, values, types)
+
+      assert_equal [1, 3], result.cast_values
+    end
+
+    test "cast_values can receive types to use instead" do
+      values = [["1.1", "2.2"], ["3.3", "4.4"]]
+      columns = ["col1", "col2"]
+      types = { "col1" => Type::Integer.new, "col2" => Type::Float.new }
+      result = Result.new(columns, values, types)
+
+      assert_equal [[1.1, 2.2], [3.3, 4.4]], result.cast_values("col1" => Type::Float.new)
+    end
   end
 end