Skip to content
This repository
Browse code

Freeze columns just before using them as hash keys

  • Loading branch information...
commit 2068d300917ed95a82e7377ee77b397fc4084a61 1 parent 2004ef2
Santiago Pastorino spastorino authored

Showing 1 changed file with 5 additions and 2 deletions. Show diff stats Hide diff stats

  1. +5 2 activerecord/lib/active_record/result.rb
7 activerecord/lib/active_record/result.rb
@@ -11,7 +11,7 @@ class Result
11 11 attr_reader :columns, :rows, :column_types
12 12
13 13 def initialize(columns, rows, column_types = {})
14   - @columns = columns.map{|c| c.freeze}
  14 + @columns = columns
15 15 @rows = rows
16 16 @hash_rows = nil
17 17 @column_types = column_types
@@ -54,7 +54,10 @@ def initialize_copy(other)
54 54 private
55 55 def hash_rows
56 56 @hash_rows ||= @rows.map { |row|
57   - Hash[@columns.zip(row)]
  57 + # We freeze the strings to prevent them getting duped when
  58 + # used as keys in ActiveRecord::Model's @attributes hash
  59 + columns = @columns.map { |c| c.freeze }
  60 + Hash[columns.zip(row)]
58 61 }
59 62 end
60 63 end

4 comments on commit 2068d30

Ravil Bayramgalin

Well, it should be reverted, comment in code even explains why :)

Jeremy Evans

Agreed, this needs to be reverted or further patched. The freezing code needs to be moved outside of @rows.map (but within the ||=, probably using begin ... end), and it should either use columns = @columns.each{|c| c.freeze} (saves an array object creation, but modifies @columns) or columns = @columns.map{|c| c.dup.freeze} (allocates a single new array and one string per column, but doesn't modify @columns). #7631

Santiago Pastorino

Oooops you're right

Santiago Pastorino

Fixed da400fb

Please sign in to comment.
Something went wrong with that request. Please try again.