Merge pull request #3157 from morozov/zero-offset-without-limit

When building a limit query, zero offset without a limit should be ignored

lib/Doctrine/DBAL/Platforms/AbstractPlatform.php

@@ -3394,22 +3394,27 @@ abstract class AbstractPlatform
             $limit = (int) $limit;
         }
 
-        if ($offset !== null) {
-            $offset = (int) $offset;
+        $offset = (int) $offset;
 
-            if ($offset < 0) {
-                throw new DBALException("LIMIT argument offset=$offset is not valid");
-            }
-            if ($offset > 0 && ! $this->supportsLimitOffset()) {
-                throw new DBALException(sprintf("Platform %s does not support offset values in limit queries.", $this->getName()));
-            }
+        if ($offset < 0) {
+            throw new DBALException(sprintf(
+                'Offset must be a positive integer of zero, %d given',
+                $offset
+            ));
+        }
+
+        if ($offset > 0 && ! $this->supportsLimitOffset()) {
+            throw new DBALException(sprintf(
+                'Platform %s does not support offset values in limit queries.',
+                $this->getName()
+            ));
         }
 
         return $this->doModifyLimitQuery($query, $limit, $offset);
     }
 
     /**
-     * Adds an driver-specific LIMIT clause to the query.
+     * Adds an platform-specific LIMIT clause to the query.
      *
      * @param string   $query
      * @param int|null $limit
@@ -3423,7 +3428,7 @@ abstract class AbstractPlatform
             $query .= ' LIMIT ' . $limit;
         }
 
-        if ($offset !== null) {
+        if ($offset > 0) {
             $query .= ' OFFSET ' . $offset;
         }
 

lib/Doctrine/DBAL/Platforms/MySqlPlatform.php

@@ -63,23 +63,18 @@ class MySqlPlatform extends AbstractPlatform
     const LENGTH_LIMIT_MEDIUMBLOB = 16777215;
 
     /**
-     * Adds MySQL-specific LIMIT clause to the query
-     * 18446744073709551615 is 2^64-1 maximum of unsigned BIGINT the biggest limit possible
-     *
-     * @param string $query
-     * @param int    $limit
-     * @param int    $offset
-     *
-     * @return string
+     * {@inheritDoc}
      */
     protected function doModifyLimitQuery($query, $limit, $offset)
     {
         if ($limit !== null) {
             $query .= ' LIMIT ' . $limit;
-            if ($offset !== null) {
+
+            if ($offset > 0) {
                 $query .= ' OFFSET ' . $offset;
             }
-        } elseif ($offset !== null) {
+        } elseif ($offset > 0) {
+            // 2^64-1 is the maximum of unsigned BIGINT, the biggest limit possible
             $query .= ' LIMIT 18446744073709551615 OFFSET ' . $offset;
         }
 

lib/Doctrine/DBAL/Platforms/OraclePlatform.php

@@ -987,7 +987,7 @@ END;';
      */
     protected function doModifyLimitQuery($query, $limit, $offset = null)
     {
-        if ($limit === null && $offset === null) {
+        if ($limit === null && $offset <= 0) {
             return $query;
         }
 

lib/Doctrine/DBAL/Platforms/SQLServer2012Platform.php

@@ -113,7 +113,7 @@ class SQLServer2012Platform extends SQLServer2008Platform
      */
     protected function doModifyLimitQuery($query, $limit, $offset = null)
     {
-        if ($limit === null && $offset === null) {
+        if ($limit === null && $offset <= 0) {
             return $query;
         }
 

lib/Doctrine/DBAL/Platforms/SqlitePlatform.php

@@ -719,7 +719,7 @@ class SqlitePlatform extends AbstractPlatform
      */
     protected function doModifyLimitQuery($query, $limit, $offset)
     {
-        if (null === $limit && null !== $offset) {
+        if ($limit === null && $offset > 0) {
             return $query . ' LIMIT -1 OFFSET ' . $offset;
         }
 

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

@@ -179,7 +179,7 @@ abstract class AbstractMySQLPlatformTestCase extends AbstractPlatformTestCase
     public function testModifyLimitQuery()
     {
         $sql = $this->_platform->modifyLimitQuery('SELECT * FROM user', 10, 0);
-        self::assertEquals('SELECT * FROM user LIMIT 10 OFFSET 0', $sql);
+        self::assertEquals('SELECT * FROM user LIMIT 10', $sql);
     }
 
     public function testModifyLimitQueryWithEmptyOffset()

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

@@ -1480,4 +1480,14 @@ abstract class AbstractPlatformTestCase extends \Doctrine\Tests\DbalTestCase
             $this->_platform->escapeStringForLike('_25% off_ your next purchase \o/', '\\')
         );
     }
+
+    public function testZeroOffsetWithoutLimitIsIgnored() : void
+    {
+        $query = 'SELECT * FROM user';
+
+        self::assertSame(
+            $query,
+            $this->_platform->modifyLimitQuery($query, null, 0)
+        );
+    }
 }

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

@@ -296,7 +296,7 @@ abstract class AbstractPostgreSqlPlatformTestCase extends AbstractPlatformTestCa
     public function testModifyLimitQuery()
     {
         $sql = $this->_platform->modifyLimitQuery('SELECT * FROM user', 10, 0);
-        self::assertEquals('SELECT * FROM user LIMIT 10 OFFSET 0', $sql);
+        self::assertEquals('SELECT * FROM user LIMIT 10', $sql);
     }
 
     public function testModifyLimitQueryWithEmptyOffset()

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

@@ -280,7 +280,7 @@ class SqlitePlatformTest extends AbstractPlatformTestCase
     public function testModifyLimitQuery()
     {
         $sql = $this->_platform->modifyLimitQuery('SELECT * FROM user', 10, 0);
-        self::assertEquals('SELECT * FROM user LIMIT 10 OFFSET 0', $sql);
+        self::assertEquals('SELECT * FROM user LIMIT 10', $sql);
     }
 
     public function testModifyLimitQueryWithEmptyOffset()