Merge pull request #508 from deeky666/fix-sql-server-nolock-hint

[DDC-2310] Fix evaluation of NOLOCK table hint on SQL Server

lib/Doctrine/DBAL/Platforms/AbstractPlatform.php

@@ -1110,8 +1110,9 @@ abstract class AbstractPlatform
     /**
      * Honors that some SQL vendors such as MsSql use table hints for locking instead of the ANSI SQL FOR UPDATE specification.
      *
-     * @param string  $fromClause
-     * @param integer $lockMode
+     * @param string       $fromClause The FROM clause to append the hint for the given lock mode to.
+     * @param integer|null $lockMode   One of the Doctrine\DBAL\LockMode::* constants. If null is given, nothing will
+     *                                 be appended to the FROM clause.
      *
      * @return string
      */

lib/Doctrine/DBAL/Platforms/SQLServerPlatform.php

@@ -1347,21 +1347,19 @@ class SQLServerPlatform extends AbstractPlatform
      */
     public function appendLockHint($fromClause, $lockMode)
     {
-        switch ($lockMode) {
-            case LockMode::NONE:
-                $lockClause = ' WITH (NOLOCK)';
-                break;
-            case LockMode::PESSIMISTIC_READ:
-                $lockClause = ' WITH (HOLDLOCK, ROWLOCK)';
-                break;
-            case LockMode::PESSIMISTIC_WRITE:
-                $lockClause = ' WITH (UPDLOCK, ROWLOCK)';
-                break;
+        switch (true) {
+            case LockMode::NONE === $lockMode:
+                return $fromClause . ' WITH (NOLOCK)';
+
+            case LockMode::PESSIMISTIC_READ === $lockMode:
+                return $fromClause . ' WITH (HOLDLOCK, ROWLOCK)';
+
+            case LockMode::PESSIMISTIC_WRITE === $lockMode:
+                return $fromClause . ' WITH (UPDLOCK, ROWLOCK)';
+
             default:
-                $lockClause = '';
+                return $fromClause;
         }
-
-        return $fromClause . $lockClause;
     }
 
     /**

tests/Doctrine/Tests/DBAL/Platforms/SQLServerPlatformTest.php

@@ -2,6 +2,7 @@
 
 namespace Doctrine\Tests\DBAL\Platforms;
 
+use Doctrine\DBAL\LockMode;
 use Doctrine\DBAL\Platforms\SQLServerPlatform;
 
 class SQLServerPlatformTest extends AbstractSQLServerPlatformTestCase
@@ -11,4 +12,28 @@ class SQLServerPlatformTest extends AbstractSQLServerPlatformTestCase
         return new SQLServerPlatform;
     }
 
+    /**
+     * @group DDC-2310
+     * @dataProvider getLockHints
+     */
+    public function testAppendsLockHint($lockMode, $lockHint)
+    {
+        $fromClause     = 'FROM users';
+        $expectedResult = $fromClause . $lockHint;
+
+        $this->assertSame($expectedResult, $this->_platform->appendLockHint($fromClause, $lockMode));
+    }
+
+    public function getLockHints()
+    {
+        return array(
+            array(null, ''),
+            array(false, ''),
+            array(true, ''),
+            array(LockMode::NONE, ' WITH (NOLOCK)'),
+            array(LockMode::OPTIMISTIC, ''),
+            array(LockMode::PESSIMISTIC_READ, ' WITH (HOLDLOCK, ROWLOCK)'),
+            array(LockMode::PESSIMISTIC_WRITE, ' WITH (UPDLOCK, ROWLOCK)'),
+        );
+    }
 }