From ccd28dc6f8c0c2af2327af320d02a880074ae743 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 684d5300d76..2ced4fb807d 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 31581c2bd4e..0ff7b4af934 100644 --- a/lib/Doctrine/DBAL/Platforms/AbstractPlatform.php +++ b/lib/Doctrine/DBAL/Platforms/AbstractPlatform.php @@ -3357,22 +3357,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', @@ -3392,21 +3380,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 5f31a612fda..886a64112e5 100644 --- a/lib/Doctrine/DBAL/Platforms/DB2Platform.php +++ b/lib/Doctrine/DBAL/Platforms/DB2Platform.php @@ -787,7 +787,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 a3cc8b730a2..6c5fb2e92dd 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 e38ac47e2d6..1614ed203ba 100644 --- a/lib/Doctrine/DBAL/Platforms/OraclePlatform.php +++ b/lib/Doctrine/DBAL/Platforms/OraclePlatform.php @@ -975,7 +975,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 94774068f1b..77e550de2f9 100644 --- a/lib/Doctrine/DBAL/Platforms/SQLAnywherePlatform.php +++ b/lib/Doctrine/DBAL/Platforms/SQLAnywherePlatform.php @@ -1351,7 +1351,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 aa4d2cdfd42..8f608391223 100644 --- a/lib/Doctrine/DBAL/Platforms/SQLServerPlatform.php +++ b/lib/Doctrine/DBAL/Platforms/SQLServerPlatform.php @@ -1243,7 +1243,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 5a1723248fe..1aadb10d56e 100644 --- a/lib/Doctrine/DBAL/Platforms/SqlitePlatform.php +++ b/lib/Doctrine/DBAL/Platforms/SqlitePlatform.php @@ -694,10 +694,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 206f328c180..4e6db64a966 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/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', ], ];