Minor code cleanup for efficiency, and tests to check the order by clause...

Minor code cleanup for efficiency, and tests to check the order by clause scrubbing is working correctly and not entering an infinite loop

lib/Doctrine/DBAL/Platforms/SQLServerPlatform.php

@@ -1222,11 +1222,11 @@ class SQLServerPlatform extends AbstractPlatform
      */
     private function scrubInnerOrderBy($query)
     {
-        $count = substr_count(strtoupper($query), "ORDER BY");
+        $count = substr_count(strtoupper($query), 'ORDER BY');
         $offset = 0;
 
         while ($count-- > 0) {
-            $orderByPos = stripos($query, " ORDER BY", $offset);
+            $orderByPos = stripos($query, ' ORDER BY', $offset);
             if ($orderByPos === false) {
                 break;
             }
@@ -1283,7 +1283,7 @@ class SQLServerPlatform extends AbstractPlatform
             }
 
             // Only yank query text on the same nesting level as the ORDER BY clause.
-            $subQueryBuffer = ($parenCount === 0 ? $query[$currentPosition] : " ") . $subQueryBuffer;
+            $subQueryBuffer = ($parenCount === 0 ? $query[$currentPosition] : ' ') . $subQueryBuffer;
 
             $currentPosition--;
         }

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

@@ -7,6 +7,7 @@ use Doctrine\DBAL\Platforms\SQLServerPlatform;
 
 class SQLServerPlatformTest extends AbstractSQLServerPlatformTestCase
 {
+
     public function createPlatform()
     {
         return new SQLServerPlatform;
@@ -24,6 +25,15 @@ class SQLServerPlatformTest extends AbstractSQLServerPlatformTestCase
         $this->assertSame($expectedResult, $this->_platform->appendLockHint($fromClause, $lockMode));
     }
 
+    /**
+     * @group DBAL-2408
+     * @dataProvider getModifyLimitQueries
+     */
+    public function testScrubInnerOrderBy($query, $limit, $offset, $expectedResult)
+    {
+        $this->assertSame($expectedResult, $this->_platform->modifyLimitQuery($query, $limit, $offset));
+    }
+
     public function getLockHints()
     {
         return array(
@@ -36,4 +46,16 @@ class SQLServerPlatformTest extends AbstractSQLServerPlatformTestCase
             array(LockMode::PESSIMISTIC_WRITE, ' WITH (UPDLOCK, ROWLOCK)'),
         );
     }
+
+    public function getModifyLimitQueries()
+    {
+        return array(
+            // Test re-ordered query with correctly-scrubbed ORDER BY clause
+            array('SELECT id_0, MIN(sclr_2) AS dctrn_minrownum FROM (SELECT c0_.id AS id_0, c0_.title AS title_1, ROW_NUMBER() OVER(ORDER BY c0_.title ASC) AS sclr_2 FROM TestTable c0_ ORDER BY c0_.title ASC) dctrn_result GROUP BY id_0 ORDER BY dctrn_minrownum ASC', 30, null, 'WITH dctrn_cte AS (SELECT TOP 30 id_0, MIN(sclr_2) AS dctrn_minrownum FROM (SELECT c0_.id AS id_0, c0_.title AS title_1, ROW_NUMBER() OVER(ORDER BY c0_.title ASC) AS sclr_2 FROM TestTable c0_) dctrn_result GROUP BY id_0 ORDER BY dctrn_minrownum ASC) SELECT * FROM (SELECT *, ROW_NUMBER() OVER (ORDER BY (SELECT 0)) AS doctrine_rownum FROM dctrn_cte) AS doctrine_tbl WHERE doctrine_rownum BETWEEN 1 AND 30 ORDER BY doctrine_rownum ASC'),
+
+            // Test re-ordered query with no scrubbed ORDER BY clause
+            array('SELECT id_0, MIN(sclr_2) AS dctrn_minrownum FROM (SELECT c0_.id AS id_0, c0_.title AS title_1, ROW_NUMBER() OVER(ORDER BY c0_.title ASC) AS sclr_2 FROM TestTable c0_) dctrn_result GROUP BY id_0 ORDER BY dctrn_minrownum ASC', 30, null, 'WITH dctrn_cte AS (SELECT TOP 30 id_0, MIN(sclr_2) AS dctrn_minrownum FROM (SELECT c0_.id AS id_0, c0_.title AS title_1, ROW_NUMBER() OVER(ORDER BY c0_.title ASC) AS sclr_2 FROM TestTable c0_) dctrn_result GROUP BY id_0 ORDER BY dctrn_minrownum ASC) SELECT * FROM (SELECT *, ROW_NUMBER() OVER (ORDER BY (SELECT 0)) AS doctrine_rownum FROM dctrn_cte) AS doctrine_tbl WHERE doctrine_rownum BETWEEN 1 AND 30 ORDER BY doctrine_rownum ASC'),
+        );
+    }
+
 }