Merge pull request #8429 from sminnee/fix-2950

FIX: Throw deprecation on limit=0
This commit is contained in:
Loz Calver 2018-10-08 15:11:52 +02:00 committed by GitHub
commit e829ad78b1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 23 additions and 17 deletions

View File

@ -238,8 +238,8 @@ class SQLSelect extends SQLConditionalExpression
* Pass LIMIT clause either as SQL snippet or in array format. * Pass LIMIT clause either as SQL snippet or in array format.
* Internally, limit will always be stored as a map containing the keys 'start' and 'limit' * Internally, limit will always be stored as a map containing the keys 'start' and 'limit'
* *
* @param int|string|array $limit If passed as a string or array, assumes SQL escaped data. * @param int|string|array|null $limit If passed as a string or array, assumes SQL escaped data.
* Only applies for positive values, or if an $offset is set as well. * Only applies for positive values.
* @param int $offset * @param int $offset
* @throws InvalidArgumentException * @throws InvalidArgumentException
* @return $this Self reference * @return $this Self reference
@ -250,10 +250,18 @@ class SQLSelect extends SQLConditionalExpression
throw new InvalidArgumentException("SQLSelect::setLimit() only takes positive values"); throw new InvalidArgumentException("SQLSelect::setLimit() only takes positive values");
} }
if ($limit === 0) {
Deprecation::notice(
'4.3',
"setLimit(0) is deprecated in SS4. To clear limit, call setLimit(null). " .
"In SS5 a limit of 0 will instead return no records."
);
}
if (is_numeric($limit) && ($limit || $offset)) { if (is_numeric($limit) && ($limit || $offset)) {
$this->limit = array( $this->limit = array(
'start' => $offset, 'start' => (int)$offset,
'limit' => $limit, 'limit' => (int)$limit,
); );
} elseif ($limit && is_string($limit)) { } elseif ($limit && is_string($limit)) {
if (strpos($limit, ',') !== false) { if (strpos($limit, ',') !== false) {
@ -263,12 +271,12 @@ class SQLSelect extends SQLConditionalExpression
} }
$this->limit = array( $this->limit = array(
'start' => trim($start), 'start' => (int)$start,
'limit' => trim($innerLimit), 'limit' => (int)$innerLimit,
); );
} elseif ($limit === null && $offset) { } elseif ($limit === null && $offset) {
$this->limit = array( $this->limit = array(
'start' => $offset, 'start' => (int)$offset,
'limit' => $limit 'limit' => $limit
); );
} else { } else {

View File

@ -267,20 +267,23 @@ class SQLSelectTest extends SapphireTest
); );
} }
/**
* @expectedException PHPUnit_Framework_Error
*/
public function testZeroLimit() public function testZeroLimit()
{ {
Deprecation::notification_version('4.3.0');
$query = new SQLSelect(); $query = new SQLSelect();
$query->setFrom("MyTable"); $query->setFrom("MyTable");
$query->setLimit(0); $query->setLimit(0);
$this->assertSQLEquals(
'SELECT * FROM MyTable',
$query->sql($parameters)
);
} }
/**
* @expectedException PHPUnit_Framework_Error
*/
public function testZeroLimitWithOffset() public function testZeroLimitWithOffset()
{ {
Deprecation::notification_version('4.3.0');
if (!(DB::get_conn() instanceof MySQLDatabase || DB::get_conn() instanceof SQLite3Database if (!(DB::get_conn() instanceof MySQLDatabase || DB::get_conn() instanceof SQLite3Database
|| DB::get_conn() instanceof PostgreSQLDatabase) || DB::get_conn() instanceof PostgreSQLDatabase)
) { ) {
@ -290,11 +293,6 @@ class SQLSelectTest extends SapphireTest
$query = new SQLSelect(); $query = new SQLSelect();
$query->setFrom("MyTable"); $query->setFrom("MyTable");
$query->setLimit(0, 99); $query->setLimit(0, 99);
$this->assertSQLEquals(
'SELECT * FROM MyTable LIMIT 0 OFFSET 99',
$query->sql($parameters)
);
} }
/** /**