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.

1 files changed, 17 insertions, 0 deletions

diff --git a/test/visitors/test_to_sql.rb b/test/visitors/test_to_sql.rb
index 1d5f89280b..8084e6441c 100644
--- a/test/visitors/test_to_sql.rb
+++ b/test/visitors/test_to_sql.rb
@@ -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