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

[DDC-2310] [2.4] Fix evaluation of NOLOCK table hint on SQL Server
doctrine · Aug 20, 2014 · f15c482 · f15c482
2 parents 1b62e9f + 18f140a
commit f15c482
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 15 deletions.
diff --git a/lib/Doctrine/DBAL/Platforms/AbstractPlatform.php b/lib/Doctrine/DBAL/Platforms/AbstractPlatform.php
@@ -1038,8 +1038,9 @@ public function getForUpdateSQL()
     /**
      * 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
      */

diff --git a/lib/Doctrine/DBAL/Platforms/SQLServerPlatform.php b/lib/Doctrine/DBAL/Platforms/SQLServerPlatform.php
@@ -1012,21 +1012,19 @@ public function rollbackSavePoint($savepoint)
      */
     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;
     }
 
     /**

diff --git a/tests/Doctrine/Tests/DBAL/Platforms/SQLServerPlatformTest.php b/tests/Doctrine/Tests/DBAL/Platforms/SQLServerPlatformTest.php
@@ -4,6 +4,7 @@
 
 use Doctrine\DBAL\Platforms\SQLServer2008Platform;
 use Doctrine\DBAL\Types\Type;
+use Doctrine\DBAL\LockMode;
 
 class SQLServerPlatformTest extends AbstractPlatformTestCase
 {
@@ -318,4 +319,28 @@ protected function getQuotedColumnInForeignKeySQL()
             'ALTER TABLE [quoted] ADD CONSTRAINT FK_WITH_INTENDED_QUOTATION FOREIGN KEY ([create], foo, [bar]) REFERENCES [foo-bar] ([create], bar, [foo-bar])',
         );
     }
+    /**
+     * @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)'),
+        );
+    }
 }