Skip to content

Commit 8edaf13

Browse files
kenjiskenjis
authored
Merge pull request #8280 from kenjis/fix-limit-zero
fix: QueryBuilder limit(0) bug
2 parents a55a9c0 + 20ea393 commit 8edaf13

File tree

12 files changed

+159
-14
lines changed

12 files changed

+159
-14
lines changed

‎app/Config/Feature.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,4 +18,12 @@ class Feature extends BaseConfig
1818
* Use filter execution order in 4.4 or before.
1919
*/
2020
public bool $oldFilterOrder = false;
21+
22+
/**
23+
* The behavior of `limit(0)` in Query Builder.
24+
*
25+
* If true, `limit(0)` returns all records. (the behavior of 4.4.x or before in version 4.x.)
26+
* If false, `limit(0)` returns no records. (the behavior of 3.1.9 or later in version 3.x.)
27+
*/
28+
public bool $limitZeroAsAll = true;
2129
}

‎phpstan-baseline.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -833,7 +833,7 @@
833833
];
834834
$ignoreErrors[] = [
835835
'message' => '#^Construct empty\\(\\) is not allowed\\. Use more strict comparison\\.$#',
836-
'count' => 40,
836+
'count' => 37,
837837
'path' => __DIR__ . '/system/Database/BaseBuilder.php',
838838
];
839839
$ignoreErrors[] = [

‎system/BaseModel.php

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
use CodeIgniter\I18n\Time;
2424
use CodeIgniter\Pager\Pager;
2525
use CodeIgniter\Validation\ValidationInterface;
26+
use Config\Feature;
2627
use Config\Services;
2728
use InvalidArgumentException;
2829
use ReflectionClass;
@@ -379,12 +380,12 @@ abstract protected function doFindColumn(string $columnName);
379380
* Fetches all results, while optionally limiting them.
380381
* This method works only with dbCalls.
381382
*
382-
* @param int $limit Limit
383-
* @param int $offset Offset
383+
* @param int|null $limit Limit
384+
* @param int $offset Offset
384385
*
385386
* @return array
386387
*/
387-
abstract protected function doFindAll(int $limit = 0, int $offset = 0);
388+
abstract protected function doFindAll(?int $limit = null, int $offset = 0);
388389

389390
/**
390391
* Returns the first row of the result set.
@@ -600,8 +601,12 @@ public function findColumn(string $columnName)
600601
*
601602
* @return array
602603
*/
603-
public function findAll(int $limit = 0, int $offset = 0)
604+
public function findAll(?int $limit = null, int $offset = 0)
604605
{
606+
if (config(Feature::class)->limitZeroAsAll) {
607+
$limit ??= 0;
608+
}
609+
605610
if ($this->tempAllowCallbacks) {
606611
// Call the before event and check for a return
607612
$eventData = $this->trigger('beforeFind', [

‎system/Database/BaseBuilder.php

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
use CodeIgniter\Database\Exceptions\DatabaseException;
1818
use CodeIgniter\Database\Exceptions\DataException;
1919
use CodeIgniter\Traits\ConditionalTrait;
20+
use Config\Feature;
2021
use InvalidArgumentException;
2122

2223
/**
@@ -1488,6 +1489,10 @@ public function orderBy(string $orderBy, string $direction = '', ?bool $escape =
14881489
*/
14891490
public function limit(?int $value = null, ?int $offset = 0)
14901491
{
1492+
if (config(Feature::class)->limitZeroAsAll && $value === 0) {
1493+
$value = null;
1494+
}
1495+
14911496
if ($value !== null) {
14921497
$this->QBLimit = $value;
14931498
}
@@ -1605,6 +1610,10 @@ protected function compileFinalQuery(string $sql): string
16051610
*/
16061611
public function get(?int $limit = null, int $offset = 0, bool $reset = true)
16071612
{
1613+
if (config(Feature::class)->limitZeroAsAll && $limit === 0) {
1614+
$limit = null;
1615+
}
1616+
16081617
if ($limit !== null) {
16091618
$this->limit($limit, $offset);
16101619
}
@@ -1738,7 +1747,11 @@ public function getWhere($where = null, ?int $limit = null, ?int $offset = 0, bo
17381747
$this->where($where);
17391748
}
17401749

1741-
if (! empty($limit)) {
1750+
if (config(Feature::class)->limitZeroAsAll && $limit === 0) {
1751+
$limit = null;
1752+
}
1753+
1754+
if ($limit !== null) {
17421755
$this->limit($limit, $offset);
17431756
}
17441757

@@ -2454,7 +2467,11 @@ public function update($set = null, $where = null, ?int $limit = null): bool
24542467
$this->where($where);
24552468
}
24562469

2457-
if (! empty($limit)) {
2470+
if (config(Feature::class)->limitZeroAsAll && $limit === 0) {
2471+
$limit = null;
2472+
}
2473+
2474+
if ($limit !== null) {
24582475
if (! $this->canLimitWhereUpdates) {
24592476
throw new DatabaseException('This driver does not allow LIMITs on UPDATE queries using WHERE.');
24602477
}
@@ -2495,10 +2512,17 @@ protected function _update(string $table, array $values): string
24952512
$valStr[] = $key . ' = ' . $val;
24962513
}
24972514

2515+
if (config(Feature::class)->limitZeroAsAll) {
2516+
return 'UPDATE ' . $this->compileIgnore('update') . $table . ' SET ' . implode(', ', $valStr)
2517+
. $this->compileWhereHaving('QBWhere')
2518+
. $this->compileOrderBy()
2519+
. ($this->QBLimit ? $this->_limit(' ', true) : '');
2520+
}
2521+
24982522
return 'UPDATE ' . $this->compileIgnore('update') . $table . ' SET ' . implode(', ', $valStr)
24992523
. $this->compileWhereHaving('QBWhere')
25002524
. $this->compileOrderBy()
2501-
. ($this->QBLimit ? $this->_limit(' ', true) : '');
2525+
. ($this->QBLimit !== false ? $this->_limit(' ', true) : '');
25022526
}
25032527

25042528
/**
@@ -2764,7 +2788,11 @@ public function delete($where = '', ?int $limit = null, bool $resetData = true)
27642788

27652789
$sql = $this->_delete($this->removeAlias($table));
27662790

2767-
if (! empty($limit)) {
2791+
if (config(Feature::class)->limitZeroAsAll && $limit === 0) {
2792+
$limit = null;
2793+
}
2794+
2795+
if ($limit !== null) {
27682796
$this->QBLimit = $limit;
27692797
}
27702798

@@ -3030,7 +3058,11 @@ protected function compileSelect($selectOverride = false): string
30303058
. $this->compileWhereHaving('QBHaving')
30313059
. $this->compileOrderBy();
30323060

3033-
if ($this->QBLimit) {
3061+
if (config(Feature::class)->limitZeroAsAll) {
3062+
if ($this->QBLimit) {
3063+
$sql = $this->_limit($sql . "\n");
3064+
}
3065+
} elseif ($this->QBLimit !== false || $this->QBOffset) {
30343066
$sql = $this->_limit($sql . "\n");
30353067
}
30363068

‎system/Database/SQLSRV/Builder.php

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
use CodeIgniter\Database\Exceptions\DataException;
1919
use CodeIgniter\Database\RawSql;
2020
use CodeIgniter\Database\ResultInterface;
21+
use Config\Feature;
2122

2223
/**
2324
* Builder for SQLSRV
@@ -308,6 +309,14 @@ private function addIdentity(string $fullTable, string $insert): string
308309
*/
309310
protected function _limit(string $sql, bool $offsetIgnore = false): string
310311
{
312+
// SQL Server cannot handle `LIMIT 0`.
313+
// DatabaseException:
314+
// [Microsoft][ODBC Driver 17 for SQL Server][SQL Server]The number of
315+
// rows provided for a FETCH clause must be greater then zero.
316+
if (! config(Feature::class)->limitZeroAsAll && $this->QBLimit === 0) {
317+
return "SELECT * \nFROM " . $this->_fromTables() . ' WHERE 1=0 ';
318+
}
319+
311320
if (empty($this->QBOrderBy)) {
312321
$sql .= ' ORDER BY (SELECT NULL) ';
313322
}
@@ -590,7 +599,11 @@ protected function compileSelect($selectOverride = false): string
590599
. $this->compileOrderBy(); // ORDER BY
591600

592601
// LIMIT
593-
if ($this->QBLimit) {
602+
if (config(Feature::class)->limitZeroAsAll) {
603+
if ($this->QBLimit) {
604+
$sql = $this->_limit($sql . "\n");
605+
}
606+
} elseif ($this->QBLimit !== false || $this->QBOffset) {
594607
$sql = $this->_limit($sql . "\n");
595608
}
596609

@@ -605,6 +618,10 @@ protected function compileSelect($selectOverride = false): string
605618
*/
606619
public function get(?int $limit = null, int $offset = 0, bool $reset = true)
607620
{
621+
if (config(Feature::class)->limitZeroAsAll && $limit === 0) {
622+
$limit = null;
623+
}
624+
608625
if ($limit !== null) {
609626
$this->limit($limit, $offset);
610627
}

‎system/Model.php

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
use CodeIgniter\Exceptions\ModelException;
2727
use CodeIgniter\Validation\ValidationInterface;
2828
use Config\Database;
29+
use Config\Feature;
2930
use ReflectionException;
3031

3132
/**
@@ -223,14 +224,18 @@ protected function doFindColumn(string $columnName)
223224
* all results, while optionally limiting them.
224225
* This method works only with dbCalls.
225226
*
226-
* @param int $limit Limit
227-
* @param int $offset Offset
227+
* @param int|null $limit Limit
228+
* @param int $offset Offset
228229
*
229230
* @return array
230231
* @phpstan-return list<row_array|object>
231232
*/
232-
protected function doFindAll(int $limit = 0, int $offset = 0)
233+
protected function doFindAll(?int $limit = null, int $offset = 0)
233234
{
235+
if (config(Feature::class)->limitZeroAsAll) {
236+
$limit ??= 0;
237+
}
238+
234239
$builder = $this->builder();
235240

236241
if ($this->tempUseSoftDeletes) {

‎tests/system/Database/Builder/GetTest.php

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
use CodeIgniter\Test\CIUnitTestCase;
1717
use CodeIgniter\Test\Mock\MockConnection;
18+
use Config\Feature;
1819

1920
/**
2021
* @internal
@@ -46,6 +47,25 @@ public function testGet(): void
4647
*/
4748
public function testGetWithReset(): void
4849
{
50+
$config = config(Feature::class);
51+
$config->limitZeroAsAll = false;
52+
53+
$builder = $this->db->table('users');
54+
$builder->testMode()->where('username', 'bogus');
55+
56+
$expectedSQL = 'SELECT * FROM "users" WHERE "username" = \'bogus\' LIMIT 50, 0';
57+
$expectedSQLafterreset = 'SELECT * FROM "users" LIMIT 50, 0';
58+
59+
$this->assertSame($expectedSQL, str_replace("\n", ' ', $builder->get(0, 50, false)));
60+
$this->assertSame($expectedSQL, str_replace("\n", ' ', $builder->get(0, 50, true)));
61+
$this->assertSame($expectedSQLafterreset, str_replace("\n", ' ', $builder->get(0, 50, true)));
62+
}
63+
64+
public function testGetWithResetWithLimitZeroAsAll(): void
65+
{
66+
$config = config(Feature::class);
67+
$config->limitZeroAsAll = true;
68+
4969
$builder = $this->db->table('users');
5070
$builder->testMode()->where('username', 'bogus');
5171

‎tests/system/Database/Live/GetTest.php

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use CodeIgniter\Database\Exceptions\DatabaseException;
1717
use CodeIgniter\Test\CIUnitTestCase;
1818
use CodeIgniter\Test\DatabaseTestTrait;
19+
use Config\Feature;
1920
use Tests\Support\Database\Seeds\CITestSeeder;
2021

2122
/**
@@ -50,6 +51,26 @@ public function testGetWitLimit(): void
5051
$this->assertSame('Musician', $jobs[1]->name);
5152
}
5253

54+
public function testGetWithLimitZero(): void
55+
{
56+
$config = config(Feature::class);
57+
$config->limitZeroAsAll = false;
58+
59+
$jobs = $this->db->table('job')->limit(0)->get()->getResult();
60+
61+
$this->assertCount(0, $jobs);
62+
}
63+
64+
public function testGetWithLimitZeroAsAll(): void
65+
{
66+
$config = config(Feature::class);
67+
$config->limitZeroAsAll = true;
68+
69+
$jobs = $this->db->table('job')->limit(0)->get()->getResult();
70+
71+
$this->assertCount(4, $jobs);
72+
}
73+
5374
public function testGetWhereArray(): void
5475
{
5576
$jobs = $this->db->table('job')

‎user_guide_src/source/changelogs/v4.5.0.rst

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,21 @@ Database
253253
Query Builder
254254
-------------
255255

256+
.. _v450-query-builder-limit-0-behavior:
257+
258+
limit(0) Behavior
259+
^^^^^^^^^^^^^^^^^
260+
261+
- Added a feature flag ``Feature::$limitZeroAsAll`` to fix the incorrect behavior
262+
of ``limit(0)``.
263+
- If ``LIMIT 0`` is specified in a SQL statement, 0 records are returned. However,
264+
there is a bug in the Query Builder, and if ``limit(0)`` is specified, the
265+
generated SQL statement will have no ``LIMIT`` clause and all records will be
266+
returned.
267+
- It is recommended that ``$limitZeroAsAll`` in **app/Config/Feature.php** be set
268+
to ``false`` as this incorrect behavior will be fixed in a future version. See
269+
also :ref:`v450-model-findall-limit-0-behavior`.
270+
256271
Forge
257272
-----
258273

@@ -262,6 +277,17 @@ Others
262277
Model
263278
=====
264279

280+
.. _v450-model-findall-limit-0-behavior:
281+
282+
findAll(0) Behavior
283+
-------------------
284+
285+
- Added a feature flag ``Feature::$limitZeroAsAll`` to fix the incorrect behavior
286+
of ``limit(0)`` for Query Builder. See :ref:`v450-query-builder-limit-0-behavior`
287+
for details.
288+
- If you disable this flag, you need to change code like ``findAll(0, $offset)``
289+
to ``findAll(null, $offset)``.
290+
265291
Libraries
266292
=========
267293

‎user_guide_src/source/database/query_builder.rst

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -681,6 +681,14 @@ Lets you limit the number of rows you would like returned by the query:
681681

682682
.. literalinclude:: query_builder/070.php
683683

684+
.. note:: If ``LIMIT 0`` is specified in a SQL statement, 0 records are returned.
685+
However, there is a bug in the Query Builder, and if ``limit(0)`` is specified,
686+
the generated SQL statement will have no ``LIMIT`` clause and all records will
687+
be returned. To fix the incorrect behavior, a setting was added in v4.5.0. See
688+
:ref:`v450-query-builder-limit-0-behavior` for details. The incorrect behavior
689+
will be fixed in a future version, so it is recommended that you change the
690+
default setting.
691+
684692
The second parameter lets you set a result offset.
685693

686694
.. literalinclude:: query_builder/071.php

‎user_guide_src/source/installation/upgrade_450.rst

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,8 @@ Others
262262
- The default value of ``DBCollat`` in ``$default`` has been change to ``utf8mb4_general_ci``.
263263
- The default value of ``DBCollat`` in ``$tests`` has been change to ``''``.
264264
- app/Config/Feature.php
265+
- ``Config\Feature::$limitZeroAsAll`` has been added. See
266+
:ref:`v450-query-builder-limit-0-behavior`.
265267
- ``Config\Feature::$multipleFilters`` has been removed, because now
266268
:ref:`multiple-filters` are always enabled.
267269
- app/Config/Kint.php

‎user_guide_src/source/installation/upgrade_database.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ Upgrade Guide
4343
- ``$this->db->having('user_id', 45);`` to ``$builder->having('user_id', 45);``
4444
6. CI4 does not provide `Database Caching <https://www.codeigniter.com/userguide3/database/caching.html>`_
4545
layer known from CI3, so if you need to cache the result, use :doc:`../libraries/caching` instead.
46+
7. If you use ``limit(0)`` in Query Builder, CI4 returns all records in stead of no records due to a bug. But since v4.5.0, you can change the incorrect behavior with a setting. So change the setting. See :ref:`v450-query-builder-limit-0-behavior` for details.
4647

4748
Code Example
4849
============

0 commit comments

Comments
 (0)