Simplified doModifyLimitQuery per @deeky666 comments, added test case for...

Simplified doModifyLimitQuery per @deeky666 comments, added test case for orderby with parenthetical expression

lib/Doctrine/DBAL/Platforms/SQLServer2012Platform.php

@@ -105,7 +105,7 @@ class SQLServer2012Platform extends SQLServer2008Platform
     }
 
     /**
-     * @inheritdoc
+     * {@inheritdoc}
      */
     protected function doModifyLimitQuery($query, $limit, $offset = null)
     {
@@ -114,13 +114,14 @@ class SQLServer2012Platform extends SQLServer2008Platform
         }
 
         // Queries using OFFSET... FETCH MUST have an ORDER BY clause
-        if (!preg_match("/ORDER BY ([a-z0-9\.\[\], \t_]|[a-z_]+\([a-z0-9\.\[\], \t_]+\))+\s*$/i", $query)) {
-            $query .= " ORDER BY dctrn_ver";
-
-            $from = $this->findOuterFrom($query);
-            //TODO handle $from === false
-
-            $query = substr_replace($query, ", @@version as dctrn_ver", $from, 0);
+        // Find the position of the last instance of ORDER BY and ensure it is not within a parenthetical statement
+        $orderByPos = strripos($query, "ORDER BY");
+        if ($orderByPos === false
+            || substr_count($query, "(", $orderByPos) - substr_count($query, ")", $orderByPos)
+        ) {
+            // In another DBMS, we could do ORDER BY 0, but SQL Server gets angry if you use constant expressions in
+            // the order by list.
+            $query .= " ORDER BY (SELECT 0)";
         }
 
         if ($offset === null) {
@@ -130,40 +131,12 @@ class SQLServer2012Platform extends SQLServer2008Platform
         // This looks somewhat like MYSQL, but limit/offset are in inverse positions
         // Supposedly SQL:2008 core standard.
         // Per TSQL spec, FETCH NEXT n ROWS ONLY is not valid without OFFSET n ROWS.
-        $query .= " OFFSET " . (int)$offset . " ROWS";
+        $query .= " OFFSET " . (int) $offset . " ROWS";
 
         if ($limit !== null) {
-            $query .= " FETCH NEXT " . (int)$limit . " ROWS ONLY";
+            $query .= " FETCH NEXT " . (int) $limit . " ROWS ONLY";
         }
 
         return $query;
     }
-
-    /**
-     * recursive function to find the outermost FROM clause in a SELECT query
-     *
-     * @param string $query the SQL query
-     * @param int $pos position of previous FROM instance, if any
-     * @return bool|int
-     */
-    private function findOuterFrom($query, $pos = 0)
-    {
-        $needle = " from ";
-        if (false === ($found = stripos($query, $needle, $pos))) {
-            return false;
-        }
-
-        $before = substr_count($query, "(", 0, $found) - substr_count($query, ")", 0, $found);
-        $after = substr_count($query, "(", $found + strlen($needle)) - substr_count(
-                $query,
-                ")",
-                $found + strlen($needle)
-            );
-
-        // $needle was found outside any parens.
-        if (!$before && !$after) {
-            return $found;
-        }
-        return $this->findOuterFrom($query, $found + strlen($needle));
-    }
 }

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

@@ -47,13 +47,13 @@ class SQLServer2012PlatformTest extends AbstractSQLServerPlatformTestCase
     public function testModifyLimitQuery()
     {
         $sql = $this->_platform->modifyLimitQuery('SELECT * FROM user', 10, 0);
-        $this->assertEquals('SELECT *, @@version as dctrn_ver FROM user ORDER BY dctrn_ver OFFSET 0 ROWS FETCH NEXT 10 ROWS ONLY', $sql);
+        $this->assertEquals('SELECT * FROM user ORDER BY (SELECT 0) OFFSET 0 ROWS FETCH NEXT 10 ROWS ONLY', $sql);
     }
 
     public function testModifyLimitQueryWithEmptyOffset()
     {
         $sql = $this->_platform->modifyLimitQuery('SELECT * FROM user', 10);
-        $this->assertEquals('SELECT *, @@version as dctrn_ver FROM user ORDER BY dctrn_ver OFFSET 0 ROWS FETCH NEXT 10 ROWS ONLY', $sql);
+        $this->assertEquals('SELECT * FROM user ORDER BY (SELECT 0) OFFSET 0 ROWS FETCH NEXT 10 ROWS ONLY', $sql);
     }
 
     public function testModifyLimitQueryWithOffset()
@@ -89,7 +89,7 @@ class SQLServer2012PlatformTest extends AbstractSQLServerPlatformTestCase
     public function testModifyLimitQueryWithSubSelect()
     {
         $sql = $this->_platform->modifyLimitQuery('SELECT * FROM (SELECT u.id as uid, u.name as uname) dctrn_result', 10);
-        $this->assertEquals('SELECT *, @@version as dctrn_ver FROM (SELECT u.id as uid, u.name as uname) dctrn_result ORDER BY dctrn_ver OFFSET 0 ROWS FETCH NEXT 10 ROWS ONLY', $sql);
+        $this->assertEquals('SELECT * FROM (SELECT u.id as uid, u.name as uname) dctrn_result ORDER BY (SELECT 0) OFFSET 0 ROWS FETCH NEXT 10 ROWS ONLY', $sql);
     }
 
     public function testModifyLimitQueryWithSubSelectAndOrder()
@@ -116,7 +116,7 @@ class SQLServer2012PlatformTest extends AbstractSQLServerPlatformTestCase
     public function testModifyLimitQueryWithFromColumnNames()
     {
         $sql = $this->_platform->modifyLimitQuery('SELECT a.fromFoo, fromBar FROM foo', 10);
-        $this->assertEquals('SELECT a.fromFoo, fromBar, @@version as dctrn_ver FROM foo ORDER BY dctrn_ver OFFSET 0 ROWS FETCH NEXT 10 ROWS ONLY', $sql);
+        $this->assertEquals('SELECT a.fromFoo, fromBar FROM foo ORDER BY (SELECT 0) OFFSET 0 ROWS FETCH NEXT 10 ROWS ONLY', $sql);
     }
 
     /**
@@ -131,11 +131,11 @@ class SQLServer2012PlatformTest extends AbstractSQLServerPlatformTestCase
 
         $sql = $this->_platform->modifyLimitQuery($query, 10);
 
-        $expected = 'SELECT table1.column1, table2.column2, table3.column3, table4.column4, table5.column5, table6.column6, table7.column7, table8.column8, @@version as dctrn_ver FROM table1, table2, table3, table4, table5, table6, table7, table8 ';
+        $expected = 'SELECT table1.column1, table2.column2, table3.column3, table4.column4, table5.column5, table6.column6, table7.column7, table8.column8 FROM table1, table2, table3, table4, table5, table6, table7, table8 ';
         $expected.= 'WHERE (table1.column1 = table2.column2) AND (table1.column1 = table3.column3) AND (table1.column1 = table4.column4) AND (table1.column1 = table5.column5) AND (table1.column1 = table6.column6) AND (table1.column1 = table7.column7) AND (table1.column1 = table8.column8) AND (table2.column2 = table3.column3) AND (table2.column2 = table4.column4) AND (table2.column2 = table5.column5) AND (table2.column2 = table6.column6) ';
         $expected.= 'AND (table2.column2 = table7.column7) AND (table2.column2 = table8.column8) AND (table3.column3 = table4.column4) AND (table3.column3 = table5.column5) AND (table3.column3 = table6.column6) AND (table3.column3 = table7.column7) AND (table3.column3 = table8.column8) AND (table4.column4 = table5.column5) AND (table4.column4 = table6.column6) AND (table4.column4 = table7.column7) AND (table4.column4 = table8.column8) ';
         $expected.= 'AND (table5.column5 = table6.column6) AND (table5.column5 = table7.column7) AND (table5.column5 = table8.column8) AND (table6.column6 = table7.column7) AND (table6.column6 = table8.column8) AND (table7.column7 = table8.column8) ';
-        $expected.= 'ORDER BY dctrn_ver OFFSET 0 ROWS FETCH NEXT 10 ROWS ONLY';
+        $expected.= 'ORDER BY (SELECT 0) OFFSET 0 ROWS FETCH NEXT 10 ROWS ONLY';
 
 
         $this->assertEquals($expected, $sql);
@@ -175,10 +175,10 @@ class SQLServer2012PlatformTest extends AbstractSQLServerPlatformTestCase
             "u.id, " .
             "(u.foo/2) foodiv, " .
             "CONCAT(u.bar, u.baz) barbaz, " .
-            "(SELECT (SELECT COUNT(*) FROM login l WHERE l.profile_id = p.id) FROM profile p WHERE p.user_id = u.id) login_count, @@version as dctrn_ver " .
+            "(SELECT (SELECT COUNT(*) FROM login l WHERE l.profile_id = p.id) FROM profile p WHERE p.user_id = u.id) login_count " .
             "FROM user u " .
             "WHERE u.status = 'disabled' " .
-            "ORDER BY dctrn_ver OFFSET 0 ROWS FETCH NEXT 10 ROWS ONLY",
+            "ORDER BY (SELECT 0) OFFSET 0 ROWS FETCH NEXT 10 ROWS ONLY",
             $sql
         );
     }
@@ -247,7 +247,7 @@ class SQLServer2012PlatformTest extends AbstractSQLServerPlatformTestCase
     {
         $sql = $this->_platform->modifyLimitQuery("SELECT DISTINCT id_0 FROM (SELECT k0_.id AS id_0 FROM key_measure k0_ WHERE (k0_.id_zone in(2))) dctrn_result", 10);
 
-        $expected = "SELECT DISTINCT id_0, @@version as dctrn_ver FROM (SELECT k0_.id AS id_0 FROM key_measure k0_ WHERE (k0_.id_zone in(2))) dctrn_result ORDER BY dctrn_ver OFFSET 0 ROWS FETCH NEXT 10 ROWS ONLY";
+        $expected = "SELECT DISTINCT id_0 FROM (SELECT k0_.id AS id_0 FROM key_measure k0_ WHERE (k0_.id_zone in(2))) dctrn_result ORDER BY (SELECT 0) OFFSET 0 ROWS FETCH NEXT 10 ROWS ONLY";
 
         $this->assertEquals($sql, $expected);
     }
@@ -260,4 +260,13 @@ class SQLServer2012PlatformTest extends AbstractSQLServerPlatformTestCase
 
         $this->assertEquals($sql, $expected);
     }
+
+    public function testModifyLimitQueryWithComplexOrderByExpression()
+    {
+        $sql = $this->_platform->modifyLimitQuery("SELECT * FROM table ORDER BY (table.x * table.y) DESC", 10);
+
+        $expected = "SELECT * FROM table ORDER BY (table.x * table.y) DESC OFFSET 0 ROWS FETCH NEXT 10 ROWS ONLY";
+
+        $this->assertEquals($sql, $expected);
+    }
 }