Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

QueryCache will just dup an AR::Result, AR::Result can deep copy

  • Loading branch information...
commit d6e41f364a73fb0378dc29d63c15ef7c18b8e18e 1 parent c091ab0
Aaron Patterson tenderlove authored
24 activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb
View
@@ -65,18 +65,24 @@ def select_all(arel, name = nil, binds = [])
end
private
- def cache_sql(sql, binds)
- result =
- if @query_cache[sql].key?(binds)
- ActiveSupport::Notifications.instrument("sql.active_record",
- :sql => sql, :binds => binds, :name => "CACHE", :connection_id => object_id)
- @query_cache[sql][binds]
- else
- @query_cache[sql][binds] = yield
- end
+ def cache_sql(sql, binds)
+ result =
+ if @query_cache[sql].key?(binds)
+ ActiveSupport::Notifications.instrument("sql.active_record",
+ :sql => sql, :binds => binds, :name => "CACHE", :connection_id => object_id)
+ @query_cache[sql][binds]
+ else
+ @query_cache[sql][binds] = yield
+ end
+ # FIXME: we should guarantee that all cached items are Result

Just tested AR with sqlite3, mysql2 and postgresql, and they are all green without the else condition.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ # objects. Then we can avoid this conditional
+ if ActiveRecord::Result === result
+ result.dup
+ else
result.collect { |row| row.dup }
end
+ end
end
end
end
6 activerecord/lib/active_record/result.rb
View
@@ -43,6 +43,12 @@ def last
hash_rows.last
end
+ def initialize_copy(other)
+ @columns = columns.dup
+ @rows = rows.dup
+ @hash_rows = nil
+ end

Is it advisable to call super on initialize_copy?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+
private
def hash_rows
@hash_rows ||= @rows.map { |row|
Carlos Antonio da Silva

Just tested AR with sqlite3, mysql2 and postgresql, and they are all green without the else condition.

Carlos Antonio da Silva

Is it advisable to call super on initialize_copy?

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