From 4754985266779da92fcbb9227c396dc44b7b8c6a Mon Sep 17 00:00:00 2001
From: Jean Boussier <jean.boussier@gmail.com>
Date: Tue, 7 May 2024 11:36:40 +0200
Subject: [PATCH] Simplify `ActiveRecord::Result#hash_rows`

Last big refactor was in https://github.com/rails/rails/pull/37614.
Somewhat extracted from https://github.com/rails/rails/pull/51744.

The concern about columns with the same name didn't make much sense
to me because in both code paths, the last one wins, so we can simplify
the whole methods.

Additionally by implementing `columns_index`, we have a decent template
always available.
---
 activerecord/lib/active_record/result.rb | 64 +++++++++---------------
 activerecord/test/cases/result_test.rb   | 14 ++++++
 2 files changed, 38 insertions(+), 40 deletions(-)

diff --git a/activerecord/lib/active_record/result.rb b/activerecord/lib/active_record/result.rb
index 2f49a47f17ca6..fda95f9546939 100644
--- a/activerecord/lib/active_record/result.rb
+++ b/activerecord/lib/active_record/result.rb
@@ -47,10 +47,13 @@ def self.empty(async: false) # :nodoc:
     end
 
     def initialize(columns, rows, column_types = nil)
-      @columns      = columns
+      # We freeze the strings to prevent them getting duped when
+      # used as keys in ActiveRecord::Base's @attributes hash
+      @columns      = columns.each(&:-@).freeze
       @rows         = rows
       @hash_rows    = nil
       @column_types = column_types || EMPTY_HASH
+      @column_indexes = nil
     end
 
     # Returns true if this result set includes the column named +name+
@@ -131,7 +134,7 @@ def cast_values(type_overrides = {}) # :nodoc:
     end
 
     def initialize_copy(other)
-      @columns      = columns.dup
+      @columns      = columns
       @rows         = rows.dup
       @column_types = column_types.dup
       @hash_rows    = nil
@@ -142,6 +145,19 @@ def freeze # :nodoc:
       super
     end
 
+    def column_indexes # :nodoc:
+      @column_indexes ||= begin
+        index = 0
+        hash = {}
+        length  = columns.length
+        while index < length
+          hash[columns[index]] = index
+          index += 1
+        end
+        hash
+      end
+    end
+
     private
       def column_type(name, index, type_overrides)
         type_overrides.fetch(name) do
@@ -152,44 +168,12 @@ def column_type(name, index, type_overrides)
       end
 
       def hash_rows
-        @hash_rows ||=
-          begin
-            # We freeze the strings to prevent them getting duped when
-            # used as keys in ActiveRecord::Base's @attributes hash
-            columns = @columns.map(&:-@)
-            length  = columns.length
-            template = nil
-
-            @rows.map { |row|
-              if template
-                # We use transform_values to build subsequent rows from the
-                # hash of the first row. This is faster because we avoid any
-                # reallocs and in Ruby 2.7+ avoid hashing entirely.
-                index = -1
-                template.transform_values do
-                  row[index += 1]
-                end
-              else
-                # In the past we used Hash[columns.zip(row)]
-                #  though elegant, the verbose way is much more efficient
-                #  both time and memory wise cause it avoids a big array allocation
-                #  this method is called a lot and needs to be micro optimised
-                hash = {}
-
-                index = 0
-                while index < length
-                  hash[columns[index]] = row[index]
-                  index += 1
-                end
-
-                # It's possible to select the same column twice, in which case
-                # we can't use a template
-                template = hash if hash.length == length
-
-                hash
-              end
-            }
-          end
+        # We use transform_values to build subsequent rows from the
+        # hash of the first row. This is faster because we avoid any
+        # reallocs and in Ruby 2.7+ avoid hashing entirely.
+        @hash_rows ||= @rows.map do |row|
+          column_indexes.transform_values { |index| row[index] }
+        end
       end
 
       empty_array = [].freeze
diff --git a/activerecord/test/cases/result_test.rb b/activerecord/test/cases/result_test.rb
index a64fb80336fbc..637d759fe6c1d 100644
--- a/activerecord/test/cases/result_test.rb
+++ b/activerecord/test/cases/result_test.rb
@@ -115,5 +115,19 @@ def result
 
       assert_equal [[1.1, 2.2], [3.3, 4.4]], result.cast_values("col1" => Type::Float.new)
     end
+
+    test "each when two columns have the same name" do
+      result = Result.new(["foo", "foo"], [
+        ["col 1", "col 2"],
+        ["col 1", "col 2"],
+        ["col 1", "col 2"],
+      ])
+
+      assert_equal 2, result.columns.size
+      result.each do |row|
+        assert_equal 1, row.size
+        assert_equal "col 2", row["foo"]
+      end
+    end
   end
 end