From ce726f40d0b9c986301a582a2ba3bfe6114634d5 Mon Sep 17 00:00:00 2001 From: kenjis Date: Wed, 17 Jan 2024 12:11:56 +0900 Subject: [PATCH 1/9] refactor: copy _updateBatch() to override --- system/Database/Postgre/Builder.php | 93 +++++++++++++++++++++++++++++ 1 file changed, 93 insertions(+) diff --git a/system/Database/Postgre/Builder.php b/system/Database/Postgre/Builder.php index 3e3ed68a9356..e31568d0672a 100644 --- a/system/Database/Postgre/Builder.php +++ b/system/Database/Postgre/Builder.php @@ -312,6 +312,99 @@ public function join(string $table, $cond, string $type = '', ?bool $escape = nu return parent::join($table, $cond, $type, $escape); } + /** + * Generates a platform-specific batch update string from the supplied data + * + * @used-by batchExecute + * + * @param string $table Protected table name + * @param list $keys QBKeys + * @param list> $values QBSet + */ + protected function _updateBatch(string $table, array $keys, array $values): string + { + $sql = $this->QBOptions['sql'] ?? ''; + + // if this is the first iteration of batch then we need to build skeleton sql + if ($sql === '') { + $constraints = $this->QBOptions['constraints'] ?? []; + + if ($constraints === []) { + if ($this->db->DBDebug) { + throw new DatabaseException('You must specify a constraint to match on for batch updates.'); // @codeCoverageIgnore + } + + return ''; // @codeCoverageIgnore + } + + $updateFields = $this->QBOptions['updateFields'] ?? + $this->updateFields($keys, false, $constraints)->QBOptions['updateFields'] ?? + []; + + $alias = $this->QBOptions['alias'] ?? '_u'; + + $sql = 'UPDATE ' . $this->compileIgnore('update') . $table . "\n"; + + $sql .= "SET\n"; + + $sql .= implode( + ",\n", + array_map( + static fn ($key, $value) => $key . ($value instanceof RawSql ? + ' = ' . $value : + ' = ' . $alias . '.' . $value), + array_keys($updateFields), + $updateFields + ) + ) . "\n"; + + $sql .= "FROM (\n{:_table_:}"; + + $sql .= ') ' . $alias . "\n"; + + $sql .= 'WHERE ' . implode( + ' AND ', + array_map( + static fn ($key, $value) => ( + ($value instanceof RawSql && is_string($key)) + ? + $table . '.' . $key . ' = ' . $value + : + ( + $value instanceof RawSql + ? + $value + : + $table . '.' . $value . ' = ' . $alias . '.' . $value + ) + ), + array_keys($constraints), + $constraints + ) + ); + + $this->QBOptions['sql'] = $sql; + } + + if (isset($this->QBOptions['setQueryAsData'])) { + $data = $this->QBOptions['setQueryAsData']; + } else { + $data = implode( + " UNION ALL\n", + array_map( + static fn ($value) => 'SELECT ' . implode(', ', array_map( + static fn ($key, $index) => $index . ' ' . $key, + $keys, + $value + )), + $values + ) + ) . "\n"; + } + + return str_replace('{:_table_:}', $data, $sql); + } + /** * Generates a platform-specific upsertBatch string from the supplied data * From 1505fcb8a550a46d8aa9c063795759a58983c8e1 Mon Sep 17 00:00:00 2001 From: kenjis Date: Wed, 17 Jan 2024 13:14:09 +0900 Subject: [PATCH 2/9] refactor: make if condition more strict --- phpstan-baseline.php | 5 ----- system/Database/Postgre/Builder.php | 2 +- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/phpstan-baseline.php b/phpstan-baseline.php index 48585d6fd969..22aae16b046e 100644 --- a/phpstan-baseline.php +++ b/phpstan-baseline.php @@ -1341,11 +1341,6 @@ 'count' => 7, 'path' => __DIR__ . '/system/Database/Postgre/Builder.php', ]; -$ignoreErrors[] = [ - 'message' => '#^Only booleans are allowed in a negated boolean, array\\\\|string\\> given\\.$#', - 'count' => 1, - 'path' => __DIR__ . '/system/Database/Postgre/Builder.php', -]; $ignoreErrors[] = [ 'message' => '#^Return type \\(CodeIgniter\\\\Database\\\\BaseBuilder\\) of method CodeIgniter\\\\Database\\\\Postgre\\\\Builder\\:\\:join\\(\\) should be covariant with return type \\(\\$this\\(CodeIgniter\\\\Database\\\\BaseBuilder\\)\\) of method CodeIgniter\\\\Database\\\\BaseBuilder\\:\\:join\\(\\)$#', 'count' => 1, diff --git a/system/Database/Postgre/Builder.php b/system/Database/Postgre/Builder.php index e31568d0672a..81c45520d6f1 100644 --- a/system/Database/Postgre/Builder.php +++ b/system/Database/Postgre/Builder.php @@ -146,7 +146,7 @@ public function replace(?array $set = null) $this->set($set); } - if (! $this->QBSet) { + if ($this->QBSet === []) { if ($this->db->DBDebug) { throw new DatabaseException('You must use the "set" method to update an entry.'); } From 3222cc3fca712907277ad77b9844b2bd17b07243 Mon Sep 17 00:00:00 2001 From: kenjis Date: Wed, 17 Jan 2024 13:11:32 +0900 Subject: [PATCH 3/9] fix: updateBatch() Postgre type error --- system/Database/BaseBuilder.php | 42 ++++++++++++++---- system/Database/Postgre/Builder.php | 15 +++++-- system/Database/SqlValue.php | 53 +++++++++++++++++++++++ tests/system/Database/Live/UpdateTest.php | 36 +++++++++++---- 4 files changed, 125 insertions(+), 21 deletions(-) create mode 100644 system/Database/SqlValue.php diff --git a/system/Database/BaseBuilder.php b/system/Database/BaseBuilder.php index 9d43e5577943..c4a3d851447c 100644 --- a/system/Database/BaseBuilder.php +++ b/system/Database/BaseBuilder.php @@ -130,7 +130,7 @@ class BaseBuilder /** * QB data sets * - * @var array|list> + * @var array|list> */ protected $QBSet = []; @@ -1852,8 +1852,16 @@ public function setData($set, ?bool $escape = null, string $alias = '') $clean = []; - foreach ($row as $rowValue) { - $clean[] = $escape ? $this->db->escape($rowValue) : $rowValue; + foreach ($row as $key => $rowValue) { + [$keyName, $keyType] = $this->parseKey($key); + + $escapedValue = $escape ? $this->db->escape($rowValue) : $rowValue; + + if ($keyType !== null) { + $clean[] = new SqlValue($escapedValue, $keyType); + } else { + $clean[] = $escapedValue; + } } $row = $clean; @@ -1862,16 +1870,32 @@ public function setData($set, ?bool $escape = null, string $alias = '') } foreach ($keys as $k) { - $k = $this->db->protectIdentifiers($k, false); + [$keyName] = $this->parseKey($k); - if (! in_array($k, $this->QBKeys, true)) { - $this->QBKeys[] = $k; + $keyName = $this->db->protectIdentifiers($keyName, false); + + if (! in_array($keyName, $this->QBKeys, true)) { + $this->QBKeys[] = $keyName; } } return $this; } + /** + * Parses column name (with type) and returns name and type + * Key examples: + * - 'updated_at::TIMESTAMP' + */ + private function parseKey(string $key): array + { + $keyInfo = explode('::', $key, 2); + $keyName = $keyInfo[0]; + $keyType = $keyInfo[1] ?? null; + + return [$keyName, $keyType]; + } + /** * Compiles an upsert query and returns the sql * @@ -2560,9 +2584,9 @@ public function updateBatch($set = null, $constraints = null, int $batchSize = 1 * * @used-by batchExecute * - * @param string $table Protected table name - * @param list $keys QBKeys - * @param list> $values QBSet + * @param string $table Protected table name + * @param list $keys QBKeys + * @param list> $values QBSet */ protected function _updateBatch(string $table, array $keys, array $values): string { diff --git a/system/Database/Postgre/Builder.php b/system/Database/Postgre/Builder.php index 81c45520d6f1..20fef129e852 100644 --- a/system/Database/Postgre/Builder.php +++ b/system/Database/Postgre/Builder.php @@ -14,6 +14,7 @@ use CodeIgniter\Database\BaseBuilder; use CodeIgniter\Database\Exceptions\DatabaseException; use CodeIgniter\Database\RawSql; +use CodeIgniter\Database\SqlValue; use InvalidArgumentException; /** @@ -317,9 +318,9 @@ public function join(string $table, $cond, string $type = '', ?bool $escape = nu * * @used-by batchExecute * - * @param string $table Protected table name - * @param list $keys QBKeys - * @param list> $values QBSet + * @param string $table Protected table name + * @param list $keys QBKeys + * @param list> $values QBSet */ protected function _updateBatch(string $table, array $keys, array $values): string { @@ -393,7 +394,13 @@ protected function _updateBatch(string $table, array $keys, array $values): stri " UNION ALL\n", array_map( static fn ($value) => 'SELECT ' . implode(', ', array_map( - static fn ($key, $index) => $index . ' ' . $key, + static function ($key, $index) { + if ($index instanceof SqlValue) { + return $index->getValue() . '::' . $index->getType() . ' ' . $key; + } + + return $index . ' ' . $key; + }, $keys, $value )), diff --git a/system/Database/SqlValue.php b/system/Database/SqlValue.php new file mode 100644 index 000000000000..d95b3dd79d29 --- /dev/null +++ b/system/Database/SqlValue.php @@ -0,0 +1,53 @@ + + * + * For the full copyright and license information, please view + * the LICENSE file that was distributed with this source code. + */ + +namespace CodeIgniter\Database; + +/** + * @interal + */ +class SqlValue +{ + /** + * @var string Escaped column value. + */ + private string $value; + + /** + * @var string|null Column type. + */ + private ?string $type; + + /** + * @param string $value Escaped column value. + * @param string|null $type Column type. + */ + public function __construct(string $value, ?string $type = null) + { + $this->value = $value; + $this->type = $type; + } + + public function getValue(): string + { + return $this->value; + } + + public function getType(): string + { + return $this->type; + } + + public function __toString(): string + { + return $this->value; + } +} diff --git a/tests/system/Database/Live/UpdateTest.php b/tests/system/Database/Live/UpdateTest.php index 1c6ae8a430b6..49a05630ea9d 100644 --- a/tests/system/Database/Live/UpdateTest.php +++ b/tests/system/Database/Live/UpdateTest.php @@ -115,24 +115,44 @@ public function testUpdateBatch(): void { $data = [ [ - 'name' => 'Derek Jones', - 'country' => 'Greece', + 'name' => 'Derek Jones', + 'country' => 'Greece', + 'updated_at' => '2023-12-02 18:47:52', ], [ - 'name' => 'Ahmadinejad', - 'country' => 'Greece', + 'name' => 'Ahmadinejad', + 'country' => 'Greece', + 'updated_at' => '2023-12-02 18:47:52', ], ]; + if ($this->db->DBDriver === 'Postgre') { + // PostgreSQL needs column type. + $data = [ + [ + 'name' => 'Derek Jones', + 'country' => 'Greece', + 'updated_at::TIMESTAMP' => '2023-12-02 18:47:52', + ], + [ + 'name' => 'Ahmadinejad', + 'country' => 'Greece', + 'updated_at::TIMESTAMP' => '2023-12-02 18:47:52', + ], + ]; + } + $this->db->table('user')->updateBatch($data, 'name'); $this->seeInDatabase('user', [ - 'name' => 'Derek Jones', - 'country' => 'Greece', + 'name' => 'Derek Jones', + 'country' => 'Greece', + 'updated_at' => '2023-12-02 18:47:52', ]); $this->seeInDatabase('user', [ - 'name' => 'Ahmadinejad', - 'country' => 'Greece', + 'name' => 'Ahmadinejad', + 'country' => 'Greece', + 'updated_at' => '2023-12-02 18:47:52', ]); } From 1173eac61041cc7f4d06f3d67dfd4d1c214d115b Mon Sep 17 00:00:00 2001 From: kenjis Date: Wed, 17 Jan 2024 19:00:07 +0900 Subject: [PATCH 4/9] chore: tweak rector parallel config --- rector.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rector.php b/rector.php index b1520c38e10a..6ee2738f2618 100644 --- a/rector.php +++ b/rector.php @@ -59,7 +59,7 @@ PHPUnitSetList::PHPUNIT_100, ]); - $rectorConfig->parallel(120, 8, 15); + $rectorConfig->parallel(120, 8, 10); // paths to refactor; solid alternative to CLI arguments $rectorConfig->paths([__DIR__ . '/app', __DIR__ . '/system', __DIR__ . '/tests', __DIR__ . '/utils']); From 4e29a3aed412d9162e6b3bea7705f75e910bc599 Mon Sep 17 00:00:00 2001 From: kenjis Date: Wed, 17 Jan 2024 19:13:10 +0900 Subject: [PATCH 5/9] refactor: by rector --- system/Database/BaseBuilder.php | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/system/Database/BaseBuilder.php b/system/Database/BaseBuilder.php index c4a3d851447c..eb3620fd7d2c 100644 --- a/system/Database/BaseBuilder.php +++ b/system/Database/BaseBuilder.php @@ -1857,11 +1857,7 @@ public function setData($set, ?bool $escape = null, string $alias = '') $escapedValue = $escape ? $this->db->escape($rowValue) : $rowValue; - if ($keyType !== null) { - $clean[] = new SqlValue($escapedValue, $keyType); - } else { - $clean[] = $escapedValue; - } + $clean[] = ($keyType !== null) ? new SqlValue($escapedValue, $keyType) : $escapedValue; } $row = $clean; From ac289ed9ded14b17dadde4387ce17c599825501c Mon Sep 17 00:00:00 2001 From: kenjis Date: Thu, 18 Jan 2024 10:49:18 +0900 Subject: [PATCH 6/9] docs: add sub section titles and tweaks --- .../source/database/query_builder.rst | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/user_guide_src/source/database/query_builder.rst b/user_guide_src/source/database/query_builder.rst index 0be07d71340b..5e6d54985a6c 100755 --- a/user_guide_src/source/database/query_builder.rst +++ b/user_guide_src/source/database/query_builder.rst @@ -1107,18 +1107,20 @@ $builder->updateBatch() .. note:: Since v4.3.0, the second parameter ``$index`` of ``updateBatch()`` has changed to ``$constraints``. It now accepts types array, string, or ``RawSql``. +Update from Data +^^^^^^^^^^^^^^^^ + Generates an update string based on the data you supply, and runs the query. You can either pass an **array** or an **object** to the method. Here is an example using an array: .. literalinclude:: query_builder/092.php -.. note:: Since v4.3.0, the generated SQL structure has been Improved. +The first parameter is an associative array of values, the second parameter is the where keys. -The first parameter is an associative array of values, the second parameter is the where key. +.. note:: Since v4.3.0, the generated SQL structure has been Improved. -Since v4.3.0, you can also use the ``setQueryAsData()``, ``onConstraint()``, and -``updateFields()`` methods: +Since v4.3.0, you can also use the ``onConstraint()`` and ``updateFields()`` methods: .. literalinclude:: query_builder/120.php @@ -1130,12 +1132,12 @@ Since v4.3.0, you can also use the ``setQueryAsData()``, ``onConstraint()``, and due to the very nature of how it works. Instead, ``updateBatch()`` returns the number of rows affected. -You can also update from a query: +Update from a Query +^^^^^^^^^^^^^^^^^^^ -.. literalinclude:: query_builder/116.php +Since v4.3.0, you can also update from a query with the ``setQueryAsData()`` method: -.. note:: The ``setQueryAsData()``, ``onConstraint()``, and ``updateFields()`` - methods can be used since v4.3.0. +.. literalinclude:: query_builder/116.php .. note:: It is required to alias the columns of the select query to match those of the target table. From ffa7610c1a93af6c678decae8e0cf4a6d9655a54 Mon Sep 17 00:00:00 2001 From: kenjis Date: Thu, 18 Jan 2024 11:31:47 +0900 Subject: [PATCH 7/9] docs: add note for PostgreSQL updateBatch() type error --- .../source/database/query_builder.rst | 7 +++++ .../source/database/query_builder/123.php | 27 +++++++++++++++++++ 2 files changed, 34 insertions(+) create mode 100644 user_guide_src/source/database/query_builder/123.php diff --git a/user_guide_src/source/database/query_builder.rst b/user_guide_src/source/database/query_builder.rst index 5e6d54985a6c..0a79d158d0c3 100755 --- a/user_guide_src/source/database/query_builder.rst +++ b/user_guide_src/source/database/query_builder.rst @@ -1120,6 +1120,13 @@ The first parameter is an associative array of values, the second parameter is t .. note:: Since v4.3.0, the generated SQL structure has been Improved. + As a result, PostgreSQL may generate SQL errors if the column type is not + specified. In that case, specify the column name and its type as + ``::`` (e.g., ``updated_at::TIMESTAMP``). This feature + can be used since v4.4.5. + + .. literalinclude:: query_builder/123.php + Since v4.3.0, you can also use the ``onConstraint()`` and ``updateFields()`` methods: .. literalinclude:: query_builder/120.php diff --git a/user_guide_src/source/database/query_builder/123.php b/user_guide_src/source/database/query_builder/123.php new file mode 100644 index 000000000000..a594bda559d4 --- /dev/null +++ b/user_guide_src/source/database/query_builder/123.php @@ -0,0 +1,27 @@ + 'Derek Jones', + 'country' => 'Greece', + 'updated_at::TIMESTAMP' => '2023-12-02 18:47:52', + ], + [ + 'name' => 'Ahmadinejad', + 'country' => 'Greece', + 'updated_at::TIMESTAMP' => '2023-12-02 18:47:52', + ], +]; +$builder->updateBatch($data, 'name'); +/* + * Produces: + * UPDATE "db_user" + * SET + * "country" = _u."country", + * "updated_at" = _u."updated_at" + * FROM ( + * SELECT 'Greece' "country", 'Derek Jones' "name", '2023-12-02 18:47:52'::TIMESTAMP "updated_at" UNION ALL + * SELECT 'Greece' "country", 'Ahmadinejad' "name", '2023-12-02 18:47:52'::TIMESTAMP "updated_at" + * ) _u + * WHERE "db_user"."name" = _u."name" + */ From 135754b49596a57f0feea807fc127d9759265e4e Mon Sep 17 00:00:00 2001 From: kenjis Date: Thu, 18 Jan 2024 11:40:59 +0900 Subject: [PATCH 8/9] docs: add changelog --- user_guide_src/source/changelogs/v4.4.5.rst | 3 +++ user_guide_src/source/database/query_builder.rst | 2 ++ 2 files changed, 5 insertions(+) diff --git a/user_guide_src/source/changelogs/v4.4.5.rst b/user_guide_src/source/changelogs/v4.4.5.rst index e029a1d9f3a5..2f73a084e8a3 100644 --- a/user_guide_src/source/changelogs/v4.4.5.rst +++ b/user_guide_src/source/changelogs/v4.4.5.rst @@ -30,6 +30,9 @@ Deprecations Bugs Fixed ********** +- **QueryBuilder:** Fixed a bug that the ``updateBatch()`` method does not work + due to type errors on PostgreSQL. See the note in :ref:`update-from-data`. + See the repo's `CHANGELOG.md `_ for a complete list of bugs fixed. diff --git a/user_guide_src/source/database/query_builder.rst b/user_guide_src/source/database/query_builder.rst index 0a79d158d0c3..9ca77a134ed2 100755 --- a/user_guide_src/source/database/query_builder.rst +++ b/user_guide_src/source/database/query_builder.rst @@ -1107,6 +1107,8 @@ $builder->updateBatch() .. note:: Since v4.3.0, the second parameter ``$index`` of ``updateBatch()`` has changed to ``$constraints``. It now accepts types array, string, or ``RawSql``. +.. _update-from-data: + Update from Data ^^^^^^^^^^^^^^^^ From fce14e92f61a85292924a7b4c8ce6878d6b5bc53 Mon Sep 17 00:00:00 2001 From: kenjis Date: Thu, 18 Jan 2024 20:28:59 +0900 Subject: [PATCH 9/9] docs: make it clear that :: can be used on Postgre --- user_guide_src/source/database/query_builder.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/user_guide_src/source/database/query_builder.rst b/user_guide_src/source/database/query_builder.rst index 9ca77a134ed2..285552a60192 100755 --- a/user_guide_src/source/database/query_builder.rst +++ b/user_guide_src/source/database/query_builder.rst @@ -1125,7 +1125,7 @@ The first parameter is an associative array of values, the second parameter is t As a result, PostgreSQL may generate SQL errors if the column type is not specified. In that case, specify the column name and its type as ``::`` (e.g., ``updated_at::TIMESTAMP``). This feature - can be used since v4.4.5. + can be used since v4.4.5 and only for ``updateBatch()`` on PostgreSQL. .. literalinclude:: query_builder/123.php