mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
Make ToSql more thread safe.
Because the ToSql visitor instance is shared across all threads, there is a race condition around column types for binary nodes. It's possible, for instance, to end up with ActiveRecord converting a string value in the final SQL to an integer during heavy concurrent operations.
This commit is contained in:
parent
a3d9c393b4
commit
73ca3932d3
2 changed files with 21 additions and 5 deletions
|
@ -8,14 +8,13 @@ module Arel
|
|||
@engine = engine
|
||||
@connection = nil
|
||||
@pool = nil
|
||||
@last_column = nil
|
||||
@quoted_tables = {}
|
||||
@quoted_columns = {}
|
||||
end
|
||||
|
||||
def accept object
|
||||
@last_column = nil
|
||||
@pool = @engine.connection_pool
|
||||
Thread.current[:arel_visitors_to_sql_last_column] = nil
|
||||
@pool = @engine.connection_pool
|
||||
@pool.with_connection do |conn|
|
||||
@connection = conn
|
||||
super
|
||||
|
@ -356,7 +355,7 @@ key on UpdateManager using UpdateManager#key=
|
|||
end
|
||||
|
||||
def visit_Arel_Attributes_Attribute o
|
||||
@last_column = column_for o
|
||||
Thread.current[:arel_visitors_to_sql_last_column] = column_for o
|
||||
join_name = o.relation.table_alias || o.relation.name
|
||||
"#{quote_table_name join_name}.#{quote_column_name o.name}"
|
||||
end
|
||||
|
@ -374,7 +373,7 @@ key on UpdateManager using UpdateManager#key=
|
|||
alias :visit_Bignum :literal
|
||||
alias :visit_Fixnum :literal
|
||||
|
||||
def quoted o; quote(o, @last_column) end
|
||||
def quoted o; quote(o, Thread.current[:arel_visitors_to_sql_last_column]) end
|
||||
|
||||
alias :visit_ActiveSupport_Multibyte_Chars :quoted
|
||||
alias :visit_ActiveSupport_StringInquirer :quoted
|
||||
|
|
|
@ -1,5 +1,9 @@
|
|||
require 'helper'
|
||||
|
||||
class Arel::Visitors::ToSql
|
||||
def last_column; Thread.current[:arel_visitors_to_sql_last_column] || @last_column; end
|
||||
end
|
||||
|
||||
module Arel
|
||||
module Visitors
|
||||
describe 'the to_sql visitor' do
|
||||
|
@ -9,6 +13,19 @@ module Arel
|
|||
@attr = @table[:id]
|
||||
end
|
||||
|
||||
it "should be thread safe around usage of last_column" do
|
||||
visit_integer_column = Thread.new do
|
||||
Thread.stop
|
||||
@visitor.send(:visit_Arel_Attributes_Attribute, @attr)
|
||||
end
|
||||
|
||||
@visitor.accept(@table[:name])
|
||||
assert_equal(:string, @visitor.last_column.type)
|
||||
visit_integer_column.run
|
||||
visit_integer_column.join
|
||||
assert_equal(:string, @visitor.last_column.type)
|
||||
end
|
||||
|
||||
it 'should not quote sql literals' do
|
||||
node = @table[Arel.star]
|
||||
sql = @visitor.accept node
|
||||
|
|
Loading…
Reference in a new issue