Implement #add_lock! in a way that does not monkey patch and works with both normal and association finder sql. Also implement test (even the PostgresSQL dueling ones) for the pessimistic locking.

Author: metaskills

lib/active_record/connection_adapters/sqlserver_adapter.rb

@@ -351,10 +351,10 @@ def add_limit_offset!(sql, options)
 
       def add_lock!(sql, options)
         # http://blog.sqlauthority.com/2007/04/27/sql-server-2005-locking-hints-and-examples/
-        case lock = options[:lock]
-          when true then sql << ' WITH(HOLDLOCK, ROWLOCK) '
-          when String then sql << " #{lock} "
-        end
+        return unless options[:lock]
+        lock_type = options[:lock] == true ? 'WITH(HOLDLOCK, ROWLOCK)' : options[:lock]
+        from_table = sql.match(/FROM(.*)WHERE/im)[1]
+        sql.sub! from_table, "#{from_table}#{lock_type} "
       end
 
       def empty_insert_statement(table_name)

lib/core_ext/active_record.rb

@@ -18,37 +18,6 @@ def reset_column_information_with_sqlserver_columns_cache_support
 
         private
 
-        # Add basic support for SQL server locking hints
-        # In the case of SQL server, the lock value must follow the FROM clause
-        # Mysql:     SELECT * FROM tst where testID = 10 LOCK IN share mode
-        # SQLServer: SELECT * from tst WITH (HOLDLOCK, ROWLOCK) where testID = 10
-        # h-lame: OK, so these 2 methods should be a patch to rails ideally, so we don't
-        #         have to play catch up against rails itself should construct_finder_sql ever
-        #         change
-        def construct_finder_sql(options)
-          scope = scope(:find)
-          sql  = "SELECT #{options[:select] || (scope && scope[:select]) || ((options[:joins] || (scope && scope[:joins])) && quoted_table_name + '.*') || '*'} "
-          sql << "FROM #{(scope && scope[:from]) || options[:from] || quoted_table_name} "
-
-          add_lock!(sql, options, scope) if ActiveRecord::Base.connection.adapter_name == "SQLServer" && !options[:lock].blank? # SQLServer
-
-          # merge_joins isn't defined in 2.1.1, but appears in edge
-          if defined?(merge_joins)
-          # The next line may fail with a nil error under 2.1.1 or other non-edge rails versions - Use this instead: add_joins!(sql, options, scope)
-           add_joins!(sql, options[:joins], scope)
-          else
-           add_joins!(sql, options, scope)
-          end
-
-          add_conditions!(sql, options[:conditions], scope)
-
-          add_group!(sql, options[:group], scope)
-          add_order!(sql, options[:order], scope)
-          add_limit!(sql, options, scope)
-          add_lock!(sql, options, scope) unless ActiveRecord::Base.connection.adapter_name == "SQLServer" #  Not SQLServer
-          sql
-        end
-
         # Overwrite the ActiveRecord::Base method for SQL server.
         # GROUP BY is necessary for distinct orderings
         def construct_finder_sql_for_association_limiting(options, join_dependency)

test/cases/pessimistic_locking_test_sqlserver.rb

@@ -0,0 +1,100 @@
+require 'cases/sqlserver_helper'
+require 'models/person'
+require 'models/reader'
+
+class PessimisticLockingTestSqlserver < ActiveRecord::TestCase
+  
+  self.use_transactional_fixtures = false
+  fixtures :people, :readers
+  
+  def setup
+    Person.columns; Reader.columns # Avoid introspection queries during tests.
+  end
+  
+  context 'For simple finds with default lock option' do
+
+    should 'lock with simple find' do
+      assert_nothing_raised do
+        Person.transaction do
+          Person.find 1, :lock => true
+        end
+      end
+    end
+
+    should 'lock with scoped find' do
+      assert_nothing_raised do
+        Person.transaction do
+          Person.with_scope(:find => { :lock => true }) do
+            Person.find 1
+          end
+        end
+      end
+    end
+
+    should 'lock with eager find' do
+      assert_nothing_raised do
+        Person.transaction do
+          Person.find 1, :include => :readers, :lock => true
+        end
+      end
+    end
+
+    should 'reload with lock when #lock! called' do
+      assert_nothing_raised do
+        Person.transaction do
+          person = Person.find 1
+          old, person.first_name = person.first_name, 'fooman'
+          person.lock!
+          assert_equal old, person.first_name
+        end
+      end
+    end
+
+  end
+  
+  context 'For dueling concurrent connections' do
+    
+    use_concurrent_connections
+
+    should 'no locks does not wait' do
+      first, second = duel { Person.find 1 }
+      assert first.end > second.end
+    end
+    
+    should 'that second lock waits' do
+      assert [0.2, 1, 5].any? { |zzz|
+        first, second = duel(zzz) { Person.find 1, :lock => true }
+        second.end > first.end
+      }
+    end
+    
+  end
+  
+    
+  protected
+  
+  def duel(zzz = 5)
+    t0, t1, t2, t3 = nil, nil, nil, nil
+    a = Thread.new do
+      t0 = Time.now
+      Person.transaction do
+        yield
+        sleep zzz       # block thread 2 for zzz seconds
+      end
+      t1 = Time.now
+    end
+    b = Thread.new do
+      sleep zzz / 2.0   # ensure thread 1 tx starts first
+      t2 = Time.now
+      Person.transaction { yield }
+      t3 = Time.now
+    end
+    a.join
+    b.join
+    assert t1 > t0 + zzz
+    assert t2 > t0
+    assert t3 > t2
+    [t0.to_f..t1.to_f, t2.to_f..t3.to_f]
+  end
+  
+end