Skip to content

Commit 9652207

Browse files
committed
Simplify ActiveRecord::Result#hash_rows
Last big refactor was in rails#37614. Somewhat extracted from rails#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.
1 parent 9ad3685 commit 9652207

File tree

2 files changed

+37
-40
lines changed

2 files changed

+37
-40
lines changed

activerecord/lib/active_record/result.rb

Lines changed: 23 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,13 @@ def self.empty(async: false) # :nodoc:
4747
end
4848

4949
def initialize(columns, rows, column_types = nil)
50-
@columns = columns
50+
# We freeze the strings to prevent them getting duped when
51+
# used as keys in ActiveRecord::Base's @attributes hash
52+
@columns = columns.each(&:-@).freeze
5153
@rows = rows
5254
@hash_rows = nil
5355
@column_types = column_types || EMPTY_HASH
56+
@column_indexes = nil
5457
end
5558

5659
# Returns true if this result set includes the column named +name+
@@ -131,7 +134,7 @@ def cast_values(type_overrides = {}) # :nodoc:
131134
end
132135

133136
def initialize_copy(other)
134-
@columns = columns.dup
137+
@columns = columns
135138
@rows = rows.dup
136139
@column_types = column_types.dup
137140
@hash_rows = nil
@@ -142,6 +145,19 @@ def freeze # :nodoc:
142145
super
143146
end
144147

148+
def column_indexes # :nodoc:
149+
@column_indexes ||= begin
150+
index = 0
151+
hash = {}
152+
length = columns.length
153+
while index < length
154+
hash[columns[index]] = index
155+
index += 1
156+
end
157+
hash
158+
end
159+
end
160+
145161
private
146162
def column_type(name, index, type_overrides)
147163
type_overrides.fetch(name) do
@@ -152,44 +168,11 @@ def column_type(name, index, type_overrides)
152168
end
153169

154170
def hash_rows
155-
@hash_rows ||=
156-
begin
157-
# We freeze the strings to prevent them getting duped when
158-
# used as keys in ActiveRecord::Base's @attributes hash
159-
columns = @columns.map(&:-@)
160-
length = columns.length
161-
template = nil
162-
163-
@rows.map { |row|
164-
if template
165-
# We use transform_values to build subsequent rows from the
166-
# hash of the first row. This is faster because we avoid any
167-
# reallocs and in Ruby 2.7+ avoid hashing entirely.
168-
index = -1
169-
template.transform_values do
170-
row[index += 1]
171-
end
172-
else
173-
# In the past we used Hash[columns.zip(row)]
174-
# though elegant, the verbose way is much more efficient
175-
# both time and memory wise cause it avoids a big array allocation
176-
# this method is called a lot and needs to be micro optimised
177-
hash = {}
178-
179-
index = 0
180-
while index < length
181-
hash[columns[index]] = row[index]
182-
index += 1
183-
end
184-
185-
# It's possible to select the same column twice, in which case
186-
# we can't use a template
187-
template = hash if hash.length == length
188-
189-
hash
190-
end
191-
}
192-
end
171+
# We use transform_values to rows.
172+
# This is faster because we avoid any reallocs and avoid hashing entirely.
173+
@hash_rows ||= @rows.map do |row|
174+
column_indexes.transform_values { |index| row[index] }
175+
end
193176
end
194177

195178
empty_array = [].freeze

activerecord/test/cases/result_test.rb

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,5 +115,19 @@ def result
115115

116116
assert_equal [[1.1, 2.2], [3.3, 4.4]], result.cast_values("col1" => Type::Float.new)
117117
end
118+
119+
test "each when two columns have the same name" do
120+
result = Result.new(["foo", "foo"], [
121+
["col 1", "col 2"],
122+
["col 1", "col 2"],
123+
["col 1", "col 2"],
124+
])
125+
126+
assert_equal 2, result.columns.size
127+
result.each do |row|
128+
assert_equal 1, row.size
129+
assert_equal "col 2", row["foo"]
130+
end
131+
end
118132
end
119133
end

0 commit comments

Comments
 (0)