Skip to content

Commit

Permalink
Simplify ActiveRecord::Result#hash_rows
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
byroot authored and fractaledmind committed May 13, 2024
1 parent 4351c89 commit 17dec62
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 40 deletions.
63 changes: 23 additions & 40 deletions activerecord/lib/active_record/result.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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+
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
14 changes: 14 additions & 0 deletions activerecord/test/cases/result_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 17dec62

Please sign in to comment.