Merge pull request #3248 from morozov/non-nullable-offset

Made the OFFSET in LIMIT queries non-nullable integer defaulting to 0

UPGRADE.md

 # Upgrade to 3.0
 
+## BC BREAK: The `NULL` value of `$offset` in LIMIT queries is not allowed
+
+The `NULL` value of the `$offset` argument in `AbstractPlatform::(do)?ModifyLimitQuery()` methods is no longer allowed. The absence of the offset should be indicated with a `0` which is now the default value.
+
 ## BC BREAK: Removed dbal:import CLI command
 
 The `dbal:import` CLI command has been removed since it only worked with PDO-based drivers by relying on a non-documented behavior of the extension, and it was impossible to make it work with other drivers.

lib/Doctrine/DBAL/Platforms/AbstractPlatform.php

@@ -3372,22 +3372,10 @@ abstract class AbstractPlatform
     /**
      * Adds an driver-specific LIMIT clause to the query.
      *
-     * @param string   $query
-     * @param int|null $limit
-     * @param int|null $offset
-     *
-     * @return string
-     *
      * @throws DBALException
      */
-    final public function modifyLimitQuery($query, $limit, $offset = null)
+    final public function modifyLimitQuery(string $query, ?int $limit, int $offset = 0) : string
     {
-        if ($limit !== null) {
-            $limit = (int) $limit;
-        }
-
-        $offset = (int) $offset;
-
         if ($offset < 0) {
             throw new DBALException(sprintf(
                 'Offset must be a positive integer or zero, %d given',
@@ -3407,21 +3395,15 @@ abstract class AbstractPlatform
 
     /**
      * Adds an platform-specific LIMIT clause to the query.
-     *
-     * @param string   $query
-     * @param int|null $limit
-     * @param int|null $offset
-     *
-     * @return string
      */
-    protected function doModifyLimitQuery($query, $limit, $offset)
+    protected function doModifyLimitQuery(string $query, ?int $limit, int $offset) : string
     {
         if ($limit !== null) {
-            $query .= ' LIMIT ' . $limit;
+            $query .= sprintf(' LIMIT %d', $limit);
         }
 
         if ($offset > 0) {
-            $query .= ' OFFSET ' . $offset;
+            $query .= sprintf(' OFFSET %d', $offset);
         }
 
         return $query;

lib/Doctrine/DBAL/Platforms/DB2Platform.php

@@ -789,7 +789,7 @@ class DB2Platform extends AbstractPlatform
     /**
      * {@inheritDoc}
      */
-    protected function doModifyLimitQuery($query, $limit, $offset = null)
+    protected function doModifyLimitQuery(string $query, ?int $limit, int $offset) : string
     {
         $where = [];
 

lib/Doctrine/DBAL/Platforms/MySqlPlatform.php

@@ -46,7 +46,7 @@ class MySqlPlatform extends AbstractPlatform
     /**
      * {@inheritDoc}
      */
-    protected function doModifyLimitQuery($query, $limit, $offset)
+    protected function doModifyLimitQuery(string $query, ?int $limit, int $offset) : string
     {
         if ($limit !== null) {
             $query .= ' LIMIT ' . $limit;

lib/Doctrine/DBAL/Platforms/OraclePlatform.php

@@ -979,7 +979,7 @@ SQL
     /**
      * {@inheritDoc}
      */
-    protected function doModifyLimitQuery($query, $limit, $offset = null)
+    protected function doModifyLimitQuery(string $query, ?int $limit, int $offset) : string
     {
         if ($limit === null && $offset <= 0) {
             return $query;

lib/Doctrine/DBAL/Platforms/SQLAnywherePlatform.php

@@ -1354,7 +1354,7 @@ SQL
     /**
      * {@inheritdoc}
      */
-    protected function doModifyLimitQuery($query, $limit, $offset)
+    protected function doModifyLimitQuery(string $query, ?int $limit, int $offset) : string
     {
         $limitOffsetClause = $this->getTopClauseSQL($limit, $offset);
 

lib/Doctrine/DBAL/Platforms/SQLServer2012Platform.php

@@ -6,6 +6,7 @@ use Doctrine\DBAL\Schema\Sequence;
 use const PREG_OFFSET_CAPTURE;
 use function preg_match;
 use function preg_match_all;
+use function sprintf;
 use function substr_count;
 
 /**
@@ -92,7 +93,7 @@ class SQLServer2012Platform extends SQLServerPlatform
     /**
      * {@inheritdoc}
      */
-    protected function doModifyLimitQuery($query, $limit, $offset = null)
+    protected function doModifyLimitQuery(string $query, ?int $limit, int $offset) : string
     {
         if ($limit === null && $offset <= 0) {
             return $query;
@@ -125,17 +126,13 @@ class SQLServer2012Platform extends SQLServerPlatform
             }
         }
 
-        if ($offset === null) {
-            $offset = 0;
-        }
-
         // 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 .= sprintf(' OFFSET %d ROWS', $offset);
 
         if ($limit !== null) {
-            $query .= ' FETCH NEXT ' . (int) $limit . ' ROWS ONLY';
+            $query .= sprintf(' FETCH NEXT %d ROWS ONLY', $limit);
         }
 
         return $query;

lib/Doctrine/DBAL/Platforms/SQLServerPlatform.php

@@ -1247,7 +1247,7 @@ SQL
     /**
      * {@inheritDoc}
      */
-    protected function doModifyLimitQuery($query, $limit, $offset = null)
+    protected function doModifyLimitQuery(string $query, ?int $limit, int $offset) : string
     {
         $where = [];
 

lib/Doctrine/DBAL/Platforms/SqlitePlatform.php

@@ -707,10 +707,10 @@ class SqlitePlatform extends AbstractPlatform
     /**
      * {@inheritDoc}
      */
-    protected function doModifyLimitQuery($query, $limit, $offset)
+    protected function doModifyLimitQuery(string $query, ?int $limit, int $offset) : string
     {
         if ($limit === null && $offset > 0) {
-            return $query . ' LIMIT -1 OFFSET ' . $offset;
+            $limit = -1;
         }
 
         return parent::doModifyLimitQuery($query, $limit, $offset);

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

@@ -373,7 +373,7 @@ class DB2PlatformTest extends AbstractPlatformTestCase
     {
         self::assertEquals(
             'SELECT * FROM user',
-            $this->platform->modifyLimitQuery('SELECT * FROM user', null, null)
+            $this->platform->modifyLimitQuery('SELECT * FROM user', null, 0)
         );
 
         self::assertEquals(

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

@@ -62,7 +62,7 @@ class SQLServerPlatformTest extends AbstractSQLServerPlatformTestCase
             [
                 '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,
+                0,
                 '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 <= 30 ORDER BY doctrine_rownum ASC',
             ],
 
@@ -70,7 +70,7 @@ class SQLServerPlatformTest extends AbstractSQLServerPlatformTestCase
             [
                 '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,
+                0,
                 '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 <= 30 ORDER BY doctrine_rownum ASC',
             ],
         ];