From 48e0cbd168e9daf710321cb7d1335011a53e88c9 Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 1 Dec 2023 10:16:37 +0900 Subject: [PATCH] feat: add feature flag for limit(0) --- app/Config/Feature.php | 8 ++++++++ system/BaseModel.php | 5 +++++ system/Database/BaseBuilder.php | 18 +++++++++++++++++- system/Database/SQLSRV/Builder.php | 9 +++++++-- system/Model.php | 5 +++++ tests/system/Database/Builder/GetTest.php | 17 +++++++++++++++++ tests/system/Database/Live/GetTest.php | 11 +++++++++++ 7 files changed, 70 insertions(+), 3 deletions(-) diff --git a/app/Config/Feature.php b/app/Config/Feature.php index 39a37676183c..8c1b2715c8b1 100644 --- a/app/Config/Feature.php +++ b/app/Config/Feature.php @@ -18,4 +18,12 @@ class Feature extends BaseConfig * Use filter execution order in 4.4 or before. */ public bool $oldFilterOrder = false; + + /** + * Keep the behavior of `limit(0)` in Query Builder in 4.4 or before. + * + * If true, `limit(0)` returns all records. (the behavior in 4.4 or before) + * If false, `limit(0)` returns no records. + */ + public bool $limitZeroAsAll = false; } diff --git a/system/BaseModel.php b/system/BaseModel.php index a5f7409ac1bb..b70ab4cc8d9a 100644 --- a/system/BaseModel.php +++ b/system/BaseModel.php @@ -21,6 +21,7 @@ use CodeIgniter\I18n\Time; use CodeIgniter\Pager\Pager; use CodeIgniter\Validation\ValidationInterface; +use Config\Feature; use Config\Services; use InvalidArgumentException; use ReflectionClass; @@ -596,6 +597,10 @@ public function findColumn(string $columnName) */ public function findAll(?int $limit = null, int $offset = 0) { + if (config(Feature::class)->limitZeroAsAll) { + $limit ??= 0; + } + if ($this->tempAllowCallbacks) { // Call the before event and check for a return $eventData = $this->trigger('beforeFind', [ diff --git a/system/Database/BaseBuilder.php b/system/Database/BaseBuilder.php index e8325bd3a487..065a3f503698 100644 --- a/system/Database/BaseBuilder.php +++ b/system/Database/BaseBuilder.php @@ -15,6 +15,7 @@ use CodeIgniter\Database\Exceptions\DatabaseException; use CodeIgniter\Database\Exceptions\DataException; use CodeIgniter\Traits\ConditionalTrait; +use Config\Feature; use InvalidArgumentException; /** @@ -1603,6 +1604,10 @@ protected function compileFinalQuery(string $sql): string */ public function get(?int $limit = null, int $offset = 0, bool $reset = true) { + if (config(Feature::class)->limitZeroAsAll) { + $limit ??= 0; + } + if ($limit !== null) { $this->limit($limit, $offset); } @@ -2493,6 +2498,13 @@ protected function _update(string $table, array $values): string $valStr[] = $key . ' = ' . $val; } + if (config(Feature::class)->limitZeroAsAll) { + return 'UPDATE ' . $this->compileIgnore('update') . $table . ' SET ' . implode(', ', $valStr) + . $this->compileWhereHaving('QBWhere') + . $this->compileOrderBy() + . ($this->QBLimit ? $this->_limit(' ', true) : ''); + } + return 'UPDATE ' . $this->compileIgnore('update') . $table . ' SET ' . implode(', ', $valStr) . $this->compileWhereHaving('QBWhere') . $this->compileOrderBy() @@ -3028,7 +3040,11 @@ protected function compileSelect($selectOverride = false): string . $this->compileWhereHaving('QBHaving') . $this->compileOrderBy(); - if ($this->QBLimit !== false || $this->QBOffset) { + if (config(Feature::class)->limitZeroAsAll) { + if ($this->QBLimit) { + $sql = $this->_limit($sql . "\n"); + } + } elseif ($this->QBLimit !== false || $this->QBOffset) { $sql = $this->_limit($sql . "\n"); } diff --git a/system/Database/SQLSRV/Builder.php b/system/Database/SQLSRV/Builder.php index 18c80e666002..6588438a361b 100755 --- a/system/Database/SQLSRV/Builder.php +++ b/system/Database/SQLSRV/Builder.php @@ -16,6 +16,7 @@ use CodeIgniter\Database\Exceptions\DataException; use CodeIgniter\Database\RawSql; use CodeIgniter\Database\ResultInterface; +use Config\Feature; /** * Builder for SQLSRV @@ -310,7 +311,7 @@ protected function _limit(string $sql, bool $offsetIgnore = false): string // DatabaseException: // [Microsoft][ODBC Driver 17 for SQL Server][SQL Server]The number of // rows provided for a FETCH clause must be greater then zero. - if ($this->QBLimit === 0) { + if (! config(Feature::class)->limitZeroAsAll && $this->QBLimit === 0) { return "SELECT * \nFROM " . $this->_fromTables() . ' WHERE 1=0 '; } @@ -596,7 +597,11 @@ protected function compileSelect($selectOverride = false): string . $this->compileOrderBy(); // ORDER BY // LIMIT - if ($this->QBLimit !== false || $this->QBOffset) { + if (config(Feature::class)->limitZeroAsAll) { + if ($this->QBLimit) { + $sql = $this->_limit($sql . "\n"); + } + } elseif ($this->QBLimit !== false || $this->QBOffset) { $sql = $this->_limit($sql . "\n"); } diff --git a/system/Model.php b/system/Model.php index 0b1bbdd18155..7d505d503aad 100644 --- a/system/Model.php +++ b/system/Model.php @@ -24,6 +24,7 @@ use CodeIgniter\Exceptions\ModelException; use CodeIgniter\Validation\ValidationInterface; use Config\Database; +use Config\Feature; use ReflectionException; /** @@ -228,6 +229,10 @@ protected function doFindColumn(string $columnName) */ protected function doFindAll(?int $limit = null, int $offset = 0) { + if (config(Feature::class)->limitZeroAsAll) { + $limit ??= 0; + } + $builder = $this->builder(); if ($this->tempUseSoftDeletes) { diff --git a/tests/system/Database/Builder/GetTest.php b/tests/system/Database/Builder/GetTest.php index 5dfa89b43bd7..bffaa4a19c41 100644 --- a/tests/system/Database/Builder/GetTest.php +++ b/tests/system/Database/Builder/GetTest.php @@ -13,6 +13,7 @@ use CodeIgniter\Test\CIUnitTestCase; use CodeIgniter\Test\Mock\MockConnection; +use Config\Feature; /** * @internal @@ -55,6 +56,22 @@ public function testGetWithReset(): void $this->assertSame($expectedSQLafterreset, str_replace("\n", ' ', $builder->get(0, 50, true))); } + public function testGetWithResetWithLimitZeroAsAll(): void + { + $config = config(Feature::class); + $config->limitZeroAsAll = true; + + $builder = $this->db->table('users'); + $builder->testMode()->where('username', 'bogus'); + + $expectedSQL = 'SELECT * FROM "users" WHERE "username" = \'bogus\''; + $expectedSQLafterreset = 'SELECT * FROM "users"'; + + $this->assertSame($expectedSQL, str_replace("\n", ' ', $builder->get(0, 50, false))); + $this->assertSame($expectedSQL, str_replace("\n", ' ', $builder->get(0, 50, true))); + $this->assertSame($expectedSQLafterreset, str_replace("\n", ' ', $builder->get(0, 50, true))); + } + /** * @see https://github.com/codeigniter4/CodeIgniter4/issues/2143 */ diff --git a/tests/system/Database/Live/GetTest.php b/tests/system/Database/Live/GetTest.php index 7d9e26f04849..375fbeea4d6b 100644 --- a/tests/system/Database/Live/GetTest.php +++ b/tests/system/Database/Live/GetTest.php @@ -14,6 +14,7 @@ use CodeIgniter\Database\Exceptions\DatabaseException; use CodeIgniter\Test\CIUnitTestCase; use CodeIgniter\Test\DatabaseTestTrait; +use Config\Feature; use Tests\Support\Database\Seeds\CITestSeeder; /** @@ -55,6 +56,16 @@ public function testGetWithLimitZero(): void $this->assertCount(0, $jobs); } + public function testGetWithLimitZeroAsAll(): void + { + $config = config(Feature::class); + $config->limitZeroAsAll = true; + + $jobs = $this->db->table('job')->limit(0)->get()->getResult(); + + $this->assertCount(4, $jobs); + } + public function testGetWhereArray(): void { $jobs = $this->db->table('job')