From 9757bebb0c95654ff3054a43e4394df2b32a0d75 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 | 6 ++--- .../DBAL/Platforms/SQLServer2012Platform.php | 11 +++----- .../DBAL/Platforms/SQLServerPlatform.php | 2 +- .../DBAL/Platforms/SqlitePlatform.php | 4 +-- .../Tests/DBAL/Platforms/DB2PlatformTest.php | 2 +- .../Platforms/SQLAnywherePlatformTest.php | 2 +- .../DBAL/Platforms/SQLServerPlatformTest.php | 4 +-- 12 files changed, 25 insertions(+), 42 deletions(-) diff --git a/UPGRADE.md b/UPGRADE.md index b2a770783c6..fc0a68dfa2e 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 96070a5ded1..22c0be8089e 100644 --- a/lib/Doctrine/DBAL/Platforms/AbstractPlatform.php +++ b/lib/Doctrine/DBAL/Platforms/AbstractPlatform.php @@ -3340,22 +3340,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', @@ -3375,21 +3363,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 2544c0e966d..d8f2a724553 100644 --- a/lib/Doctrine/DBAL/Platforms/DB2Platform.php +++ b/lib/Doctrine/DBAL/Platforms/DB2Platform.php @@ -782,7 +782,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 01ca70833a0..a70a89f0714 100644 --- a/lib/Doctrine/DBAL/Platforms/MySqlPlatform.php +++ b/lib/Doctrine/DBAL/Platforms/MySqlPlatform.php @@ -47,7 +47,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 d368b59e08b..61ca23ea1a7 100644 --- a/lib/Doctrine/DBAL/Platforms/OraclePlatform.php +++ b/lib/Doctrine/DBAL/Platforms/OraclePlatform.php @@ -971,7 +971,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 65097a4dcc0..d33477e8c51 100644 --- a/lib/Doctrine/DBAL/Platforms/SQLAnywherePlatform.php +++ b/lib/Doctrine/DBAL/Platforms/SQLAnywherePlatform.php @@ -1352,16 +1352,16 @@ protected function _getTransactionIsolationLevelSQL($level) /** * {@inheritdoc} */ - protected function doModifyLimitQuery($query, $limit, $offset) + protected function doModifyLimitQuery(string $query, ?int $limit, int $offset) : string { $limitOffsetClause = ''; - if ($limit > 0) { + if ($limit !== null) { $limitOffsetClause = 'TOP ' . $limit . ' '; } if ($offset > 0) { - if ($limit === 0) { + if ($limit === null) { $limitOffsetClause = 'TOP ALL '; } 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 f827779891c..77751f0fd99 100644 --- a/lib/Doctrine/DBAL/Platforms/SQLServerPlatform.php +++ b/lib/Doctrine/DBAL/Platforms/SQLServerPlatform.php @@ -1237,7 +1237,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 ccb9284fa79..7ae69a515ad 100644 --- a/lib/Doctrine/DBAL/Platforms/SqlitePlatform.php +++ b/lib/Doctrine/DBAL/Platforms/SqlitePlatform.php @@ -689,10 +689,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 134ef79cfd7..5b52552e81a 100644 --- a/tests/Doctrine/Tests/DBAL/Platforms/DB2PlatformTest.php +++ b/tests/Doctrine/Tests/DBAL/Platforms/DB2PlatformTest.php @@ -342,7 +342,7 @@ public function testModifiesLimitQuery() { 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/SQLAnywherePlatformTest.php b/tests/Doctrine/Tests/DBAL/Platforms/SQLAnywherePlatformTest.php index 1f0cf18b965..890e9d6f1d3 100644 --- a/tests/Doctrine/Tests/DBAL/Platforms/SQLAnywherePlatformTest.php +++ b/tests/Doctrine/Tests/DBAL/Platforms/SQLAnywherePlatformTest.php @@ -760,7 +760,7 @@ public function testModifiesLimitQueryWithOffset() ); self::assertEquals( 'SELECT TOP ALL START AT 6 * FROM user', - $this->platform->modifyLimitQuery('SELECT * FROM user', 0, 5) + $this->platform->modifyLimitQuery('SELECT * FROM user', null, 5) ); } diff --git a/tests/Doctrine/Tests/DBAL/Platforms/SQLServerPlatformTest.php b/tests/Doctrine/Tests/DBAL/Platforms/SQLServerPlatformTest.php index d86485866d0..f2f72719eff 100644 --- a/tests/Doctrine/Tests/DBAL/Platforms/SQLServerPlatformTest.php +++ b/tests/Doctrine/Tests/DBAL/Platforms/SQLServerPlatformTest.php @@ -53,7 +53,7 @@ public function getModifyLimitQueries() [ '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', ], @@ -61,7 +61,7 @@ public function getModifyLimitQueries() [ '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', ], ];