From 130444767779e83c89b79dc14f167010bfb6f3a1 Mon Sep 17 00:00:00 2001 From: Sergei Morozov Date: Fri, 10 Aug 2018 20:55:34 -0700 Subject: [PATCH] Made the OFFSET in LIMIT queries non-nullable --- UPGRADE.md | 4 +++ .../DBAL/Platforms/AbstractPlatform.php | 26 +++---------------- lib/Doctrine/DBAL/Platforms/DB2Platform.php | 2 +- lib/Doctrine/DBAL/Platforms/MySqlPlatform.php | 2 +- .../DBAL/Platforms/OraclePlatform.php | 2 +- .../DBAL/Platforms/SQLAnywherePlatform.php | 2 +- .../DBAL/Platforms/SQLServer2012Platform.php | 11 +++----- .../DBAL/Platforms/SQLServerPlatform.php | 2 +- .../DBAL/Platforms/SqlitePlatform.php | 4 +-- .../Tests/DBAL/Platforms/DB2PlatformTest.php | 2 +- .../DBAL/Platforms/SQLServerPlatformTest.php | 4 +-- 11 files changed, 22 insertions(+), 39 deletions(-) diff --git a/UPGRADE.md b/UPGRADE.md index 06e6774f3d1..84b96710ee8 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -1,5 +1,9 @@ # 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. diff --git a/lib/Doctrine/DBAL/Platforms/AbstractPlatform.php b/lib/Doctrine/DBAL/Platforms/AbstractPlatform.php index b423b03a242..b0a8ccc96fa 100644 --- a/lib/Doctrine/DBAL/Platforms/AbstractPlatform.php +++ b/lib/Doctrine/DBAL/Platforms/AbstractPlatform.php @@ -3372,22 +3372,10 @@ public function getTimeFormatString() /** * 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 @@ final public function modifyLimitQuery($query, $limit, $offset = null) /** * 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; diff --git a/lib/Doctrine/DBAL/Platforms/DB2Platform.php b/lib/Doctrine/DBAL/Platforms/DB2Platform.php index 58e9490527a..0a1e16c6834 100644 --- a/lib/Doctrine/DBAL/Platforms/DB2Platform.php +++ b/lib/Doctrine/DBAL/Platforms/DB2Platform.php @@ -789,7 +789,7 @@ public function getTemporaryTableName($tableName) /** * {@inheritDoc} */ - protected function doModifyLimitQuery($query, $limit, $offset = null) + protected function doModifyLimitQuery(string $query, ?int $limit, int $offset) : string { $where = []; diff --git a/lib/Doctrine/DBAL/Platforms/MySqlPlatform.php b/lib/Doctrine/DBAL/Platforms/MySqlPlatform.php index ebc9563684e..239dd6f3c8e 100644 --- a/lib/Doctrine/DBAL/Platforms/MySqlPlatform.php +++ b/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; diff --git a/lib/Doctrine/DBAL/Platforms/OraclePlatform.php b/lib/Doctrine/DBAL/Platforms/OraclePlatform.php index 02413c2041a..b4d096bfa54 100644 --- a/lib/Doctrine/DBAL/Platforms/OraclePlatform.php +++ b/lib/Doctrine/DBAL/Platforms/OraclePlatform.php @@ -979,7 +979,7 @@ public function getName() /** * {@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; diff --git a/lib/Doctrine/DBAL/Platforms/SQLAnywherePlatform.php b/lib/Doctrine/DBAL/Platforms/SQLAnywherePlatform.php index c0abd1ef4ec..f3a2bf3c185 100644 --- a/lib/Doctrine/DBAL/Platforms/SQLAnywherePlatform.php +++ b/lib/Doctrine/DBAL/Platforms/SQLAnywherePlatform.php @@ -1354,7 +1354,7 @@ protected function _getTransactionIsolationLevelSQL($level) /** * {@inheritdoc} */ - protected function doModifyLimitQuery($query, $limit, $offset) + protected function doModifyLimitQuery(string $query, ?int $limit, int $offset) : string { $limitOffsetClause = $this->getTopClauseSQL($limit, $offset); diff --git a/lib/Doctrine/DBAL/Platforms/SQLServer2012Platform.php b/lib/Doctrine/DBAL/Platforms/SQLServer2012Platform.php index 1262e2be3e5..afc6a32b060 100644 --- a/lib/Doctrine/DBAL/Platforms/SQLServer2012Platform.php +++ b/lib/Doctrine/DBAL/Platforms/SQLServer2012Platform.php @@ -6,6 +6,7 @@ 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 @@ protected function getReservedKeywordsClass() /** * {@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 @@ protected function doModifyLimitQuery($query, $limit, $offset = null) } } - 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; diff --git a/lib/Doctrine/DBAL/Platforms/SQLServerPlatform.php b/lib/Doctrine/DBAL/Platforms/SQLServerPlatform.php index 8944a80dbfd..6900721486d 100644 --- a/lib/Doctrine/DBAL/Platforms/SQLServerPlatform.php +++ b/lib/Doctrine/DBAL/Platforms/SQLServerPlatform.php @@ -1247,7 +1247,7 @@ public function getBooleanTypeDeclarationSQL(array $field) /** * {@inheritDoc} */ - protected function doModifyLimitQuery($query, $limit, $offset = null) + protected function doModifyLimitQuery(string $query, ?int $limit, int $offset) : string { $where = []; diff --git a/lib/Doctrine/DBAL/Platforms/SqlitePlatform.php b/lib/Doctrine/DBAL/Platforms/SqlitePlatform.php index ed6e852298a..df133d749aa 100644 --- a/lib/Doctrine/DBAL/Platforms/SqlitePlatform.php +++ b/lib/Doctrine/DBAL/Platforms/SqlitePlatform.php @@ -707,10 +707,10 @@ protected function getPostAlterTableIndexForeignKeySQL(TableDiff $diff) /** * {@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); diff --git a/tests/Doctrine/Tests/DBAL/Platforms/DB2PlatformTest.php b/tests/Doctrine/Tests/DBAL/Platforms/DB2PlatformTest.php index e2495e0f2f6..36ef0d6b2da 100644 --- a/tests/Doctrine/Tests/DBAL/Platforms/DB2PlatformTest.php +++ b/tests/Doctrine/Tests/DBAL/Platforms/DB2PlatformTest.php @@ -373,7 +373,7 @@ public function testModifiesLimitQuery() : void { self::assertEquals( 'SELECT * FROM user', - $this->platform->modifyLimitQuery('SELECT * FROM user', null, null) + $this->platform->modifyLimitQuery('SELECT * FROM user', null, 0) ); self::assertEquals( diff --git a/tests/Doctrine/Tests/DBAL/Platforms/SQLServerPlatformTest.php b/tests/Doctrine/Tests/DBAL/Platforms/SQLServerPlatformTest.php index a5d27823b38..cdbf83e88f3 100644 --- a/tests/Doctrine/Tests/DBAL/Platforms/SQLServerPlatformTest.php +++ b/tests/Doctrine/Tests/DBAL/Platforms/SQLServerPlatformTest.php @@ -62,7 +62,7 @@ public static function getModifyLimitQueries() : iterable [ '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 @@ public static function getModifyLimitQueries() : iterable [ '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', ], ];