Merge pull request rails#51752 from Shopify/result-duplicated-column

Simplify `ActiveRecord::Result#hash_rows`

Author: byroot

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,11 @@ 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 rows.
+        # This is faster because we avoid any reallocs and avoid hashing entirely.
+        @hash_rows ||= @rows.map do |row|
+          column_indexes.transform_values { |index| row[index] }
+        end
       end
 
       empty_array = [].freeze

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