From 43ce1350f64b48c8de7cffab5d7cadc49fa51443 Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 28 Jun 2024 08:55:41 +0900 Subject: [PATCH 1/7] test: add test for select() with RawSql --- tests/system/Database/Builder/SelectTest.php | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/system/Database/Builder/SelectTest.php b/tests/system/Database/Builder/SelectTest.php index cd3f39029f6b..569d78803aee 100644 --- a/tests/system/Database/Builder/SelectTest.php +++ b/tests/system/Database/Builder/SelectTest.php @@ -67,6 +67,21 @@ public function testSelectAcceptsArray(): void $this->assertSame($expected, str_replace("\n", ' ', $builder->getCompiledSelect())); } + public function testSelectAcceptsArrayWithRawSql(): void + { + $builder = new BaseBuilder('employees', $this->db); + + $builder->select([ + 'employee_id', + new RawSql("IF(salary > 5000, 'High', 'Low') AS salary_level"), + ]); + + $expected = <<<'SQL' + SELECT "employee_id", IF(salary > 5000, 'High', 'Low') AS salary_level FROM "employees" + SQL; + $this->assertSame($expected, str_replace("\n", ' ', $builder->getCompiledSelect())); + } + public function testSelectAcceptsMultipleColumns(): void { $builder = new BaseBuilder('users', $this->db); From 1962e8c82c46b5c4d4ea3c0235cbae590efe59a6 Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 28 Jun 2024 08:56:39 +0900 Subject: [PATCH 2/7] fix: select() may cause TypeError TypeError: trim(): Argument #1 ($string) must be of type string, CodeIgniter\Database\RawSql given --- system/Database/BaseBuilder.php | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/system/Database/BaseBuilder.php b/system/Database/BaseBuilder.php index 4e6e422b2a6f..00d75b0b7873 100644 --- a/system/Database/BaseBuilder.php +++ b/system/Database/BaseBuilder.php @@ -390,7 +390,7 @@ public function ignore(bool $ignore = true) /** * Generates the SELECT portion of the query * - * @param array|RawSql|string $select + * @param list|RawSql|string $select * * @return $this */ @@ -402,16 +402,21 @@ public function select($select = '*', ?bool $escape = null) } if ($select instanceof RawSql) { - $this->QBSelect[] = $select; - - return $this; + $select = [$select]; } if (is_string($select)) { - $select = $escape === false ? [$select] : explode(',', $select); + $select = ($escape === false) ? [$select] : explode(',', $select); } foreach ($select as $val) { + if ($val instanceof RawSql) { + $this->QBSelect[] = $val; + $this->QBNoEscape[] = false; + + continue; + } + $val = trim($val); if ($val !== '') { From 73d67fb659b54b25d5d0ab51b919daebb678f9a7 Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 28 Jun 2024 09:17:18 +0900 Subject: [PATCH 3/7] refactor: rename variable name --- system/Database/BaseBuilder.php | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/system/Database/BaseBuilder.php b/system/Database/BaseBuilder.php index 00d75b0b7873..c892033fd9ae 100644 --- a/system/Database/BaseBuilder.php +++ b/system/Database/BaseBuilder.php @@ -124,9 +124,9 @@ class BaseBuilder protected array $QBUnion = []; /** - * QB NO ESCAPE data + * Whether to protect identifiers in SELECT * - * @var array + * @var list true=protect, false=not protect */ public $QBNoEscape = []; @@ -391,6 +391,7 @@ public function ignore(bool $ignore = true) * Generates the SELECT portion of the query * * @param list|RawSql|string $select + * @param bool|null $escape Whether to protect identifiers * * @return $this */ @@ -3066,8 +3067,8 @@ protected function compileSelect($selectOverride = false): string // The reason we protect identifiers here rather than in the select() function // is because until the user calls the from() function we don't know if there are aliases foreach ($this->QBSelect as $key => $val) { - $noEscape = $this->QBNoEscape[$key] ?? null; - $this->QBSelect[$key] = $this->db->protectIdentifiers($val, false, $noEscape); + $protect = $this->QBNoEscape[$key] ?? null; + $this->QBSelect[$key] = $this->db->protectIdentifiers($val, false, $protect); } $sql .= implode(', ', $this->QBSelect); From 51f94576e618cc881230bfd49b4a6ed107290d7b Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 28 Jun 2024 09:45:42 +0900 Subject: [PATCH 4/7] chore: update phpstan-baseline.php --- phpstan-baseline.php | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/phpstan-baseline.php b/phpstan-baseline.php index 5a39d840a54d..bd665e22f5f7 100644 --- a/phpstan-baseline.php +++ b/phpstan-baseline.php @@ -1819,12 +1819,6 @@ 'count' => 1, 'path' => __DIR__ . '/system/Database/BaseBuilder.php', ]; -$ignoreErrors[] = [ - // identifier: missingType.iterableValue - 'message' => '#^Method CodeIgniter\\\\Database\\\\BaseBuilder\\:\\:select\\(\\) has parameter \\$select with no value type specified in iterable type array\\.$#', - 'count' => 1, - 'path' => __DIR__ . '/system/Database/BaseBuilder.php', -]; $ignoreErrors[] = [ // identifier: missingType.iterableValue 'message' => '#^Method CodeIgniter\\\\Database\\\\BaseBuilder\\:\\:set\\(\\) has parameter \\$key with no value type specified in iterable type array\\.$#', @@ -1993,12 +1987,6 @@ 'count' => 1, 'path' => __DIR__ . '/system/Database/BaseBuilder.php', ]; -$ignoreErrors[] = [ - // identifier: missingType.iterableValue - 'message' => '#^Property CodeIgniter\\\\Database\\\\BaseBuilder\\:\\:\\$QBNoEscape type has no value type specified in iterable type array\\.$#', - 'count' => 1, - 'path' => __DIR__ . '/system/Database/BaseBuilder.php', -]; $ignoreErrors[] = [ // identifier: missingType.iterableValue 'message' => '#^Property CodeIgniter\\\\Database\\\\BaseBuilder\\:\\:\\$QBOptions type has no value type specified in iterable type array\\.$#', From fd5bf15ca5039174a3efd48535c552bc07d37ab6 Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 28 Jun 2024 19:20:15 +0900 Subject: [PATCH 5/7] fix: select() with first param as RawSql --- system/Database/BaseBuilder.php | 10 ++++++---- tests/system/Database/Builder/SelectTest.php | 4 ++-- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/system/Database/BaseBuilder.php b/system/Database/BaseBuilder.php index c892033fd9ae..02f288bd2fdf 100644 --- a/system/Database/BaseBuilder.php +++ b/system/Database/BaseBuilder.php @@ -3060,15 +3060,17 @@ protected function compileSelect($selectOverride = false): string if (empty($this->QBSelect)) { $sql .= '*'; - } elseif ($this->QBSelect[0] instanceof RawSql) { - $sql .= (string) $this->QBSelect[0]; } else { // Cycle through the "select" portion of the query and prep each column name. // The reason we protect identifiers here rather than in the select() function // is because until the user calls the from() function we don't know if there are aliases foreach ($this->QBSelect as $key => $val) { - $protect = $this->QBNoEscape[$key] ?? null; - $this->QBSelect[$key] = $this->db->protectIdentifiers($val, false, $protect); + if ($val instanceof RawSql) { + $this->QBSelect[$key] = (string) $this->QBSelect[0]; + } else { + $protect = $this->QBNoEscape[$key] ?? null; + $this->QBSelect[$key] = $this->db->protectIdentifiers($val, false, $protect); + } } $sql .= implode(', ', $this->QBSelect); diff --git a/tests/system/Database/Builder/SelectTest.php b/tests/system/Database/Builder/SelectTest.php index 569d78803aee..34cade794df8 100644 --- a/tests/system/Database/Builder/SelectTest.php +++ b/tests/system/Database/Builder/SelectTest.php @@ -72,12 +72,12 @@ public function testSelectAcceptsArrayWithRawSql(): void $builder = new BaseBuilder('employees', $this->db); $builder->select([ - 'employee_id', new RawSql("IF(salary > 5000, 'High', 'Low') AS salary_level"), + 'employee_id', ]); $expected = <<<'SQL' - SELECT "employee_id", IF(salary > 5000, 'High', 'Low') AS salary_level FROM "employees" + SELECT IF(salary > 5000, 'High', 'Low') AS salary_level, "employee_id" FROM "employees" SQL; $this->assertSame($expected, str_replace("\n", ' ', $builder->getCompiledSelect())); } From 8534e8fcb4bf7809d6f7377e7276454ad4524c3c Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 28 Jun 2024 20:20:20 +0900 Subject: [PATCH 6/7] fix: wrong variable and add tests --- system/Database/BaseBuilder.php | 2 +- tests/system/Database/Builder/SelectTest.php | 58 +++++++++++++++++--- 2 files changed, 51 insertions(+), 9 deletions(-) diff --git a/system/Database/BaseBuilder.php b/system/Database/BaseBuilder.php index 02f288bd2fdf..4d153fca1dc1 100644 --- a/system/Database/BaseBuilder.php +++ b/system/Database/BaseBuilder.php @@ -3066,7 +3066,7 @@ protected function compileSelect($selectOverride = false): string // is because until the user calls the from() function we don't know if there are aliases foreach ($this->QBSelect as $key => $val) { if ($val instanceof RawSql) { - $this->QBSelect[$key] = (string) $this->QBSelect[0]; + $this->QBSelect[$key] = (string) $val; } else { $protect = $this->QBNoEscape[$key] ?? null; $this->QBSelect[$key] = $this->db->protectIdentifiers($val, false, $protect); diff --git a/tests/system/Database/Builder/SelectTest.php b/tests/system/Database/Builder/SelectTest.php index 34cade794df8..5f1fe5fdc8da 100644 --- a/tests/system/Database/Builder/SelectTest.php +++ b/tests/system/Database/Builder/SelectTest.php @@ -19,6 +19,7 @@ use CodeIgniter\Database\SQLSRV\Builder as SQLSRVBuilder; use CodeIgniter\Test\CIUnitTestCase; use CodeIgniter\Test\Mock\MockConnection; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Group; /** @@ -67,21 +68,62 @@ public function testSelectAcceptsArray(): void $this->assertSame($expected, str_replace("\n", ' ', $builder->getCompiledSelect())); } - public function testSelectAcceptsArrayWithRawSql(): void + #[DataProvider('provideSelectAcceptsArrayWithRawSql')] + public function testSelectAcceptsArrayWithRawSql(array $select, string $expected): void { $builder = new BaseBuilder('employees', $this->db); - $builder->select([ - new RawSql("IF(salary > 5000, 'High', 'Low') AS salary_level"), - 'employee_id', - ]); + $builder->select($select); - $expected = <<<'SQL' - SELECT IF(salary > 5000, 'High', 'Low') AS salary_level, "employee_id" FROM "employees" - SQL; $this->assertSame($expected, str_replace("\n", ' ', $builder->getCompiledSelect())); } + /** + * @return list|string> + */ + public static function provideSelectAcceptsArrayWithRawSql(): iterable + { + yield from [ + [ + [ + new RawSql("IF(salary > 5000, 'High', 'Low') AS salary_level"), + 'employee_id', + ], + <<<'SQL' + SELECT IF(salary > 5000, 'High', 'Low') AS salary_level, "employee_id" FROM "employees" + SQL, + ], + [ + [ + 'employee_id', + new RawSql("IF(salary > 5000, 'High', 'Low') AS salary_level"), + ], + <<<'SQL' + SELECT "employee_id", IF(salary > 5000, 'High', 'Low') AS salary_level FROM "employees" + SQL, + ], + [ + [ + new RawSql("CONCAT(first_name, ' ', last_name) AS full_name"), + new RawSql("IF(salary > 5000, 'High', 'Low') AS salary_level"), + ], + <<<'SQL' + SELECT CONCAT(first_name, ' ', last_name) AS full_name, IF(salary > 5000, 'High', 'Low') AS salary_level FROM "employees" + SQL, + ], + [ + [ + new RawSql("CONCAT(first_name, ' ', last_name) AS full_name"), + 'employee_id', + new RawSql("IF(salary > 5000, 'High', 'Low') AS salary_level"), + ], + <<<'SQL' + SELECT CONCAT(first_name, ' ', last_name) AS full_name, "employee_id", IF(salary > 5000, 'High', 'Low') AS salary_level FROM "employees" + SQL, + ], + ]; + } + public function testSelectAcceptsMultipleColumns(): void { $builder = new BaseBuilder('users', $this->db); From c022dd32e55e3f8fc0655296fa1fca638dcf40d3 Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 28 Jun 2024 20:23:43 +0900 Subject: [PATCH 7/7] test: add @param --- tests/system/Database/Builder/SelectTest.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/system/Database/Builder/SelectTest.php b/tests/system/Database/Builder/SelectTest.php index 5f1fe5fdc8da..fe4ad5b80178 100644 --- a/tests/system/Database/Builder/SelectTest.php +++ b/tests/system/Database/Builder/SelectTest.php @@ -68,6 +68,9 @@ public function testSelectAcceptsArray(): void $this->assertSame($expected, str_replace("\n", ' ', $builder->getCompiledSelect())); } + /** + * @param list $select + */ #[DataProvider('provideSelectAcceptsArrayWithRawSql')] public function testSelectAcceptsArrayWithRawSql(array $select, string $expected): void {