Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: [Postgre] QueryBuilder::updateBatch() does not work <column_name>::<type> #8426

Closed
wants to merge 9 commits into from
5 changes: 0 additions & 5 deletions phpstan-baseline.php
Original file line number Diff line number Diff line change
@@ -1341,11 +1341,6 @@
'count' => 7,
'path' => __DIR__ . '/system/Database/Postgre/Builder.php',
];
$ignoreErrors[] = [
'message' => '#^Only booleans are allowed in a negated boolean, array\\<int\\|string, array\\<int, int\\|string\\>\\|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,
2 changes: 1 addition & 1 deletion rector.php
Original file line number Diff line number Diff line change
@@ -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']);
38 changes: 29 additions & 9 deletions system/Database/BaseBuilder.php
Original file line number Diff line number Diff line change
@@ -130,7 +130,7 @@ class BaseBuilder
/**
* QB data sets
*
* @var array<string, string>|list<list<int|string>>
* @var array<string, string>|list<list<int|SqlValue|string>>
*/
protected $QBSet = [];

@@ -1852,8 +1852,12 @@ 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;

$clean[] = ($keyType !== null) ? new SqlValue($escapedValue, $keyType) : $escapedValue;
}

$row = $clean;
@@ -1862,16 +1866,32 @@ public function setData($set, ?bool $escape = null, string $alias = '')
}

foreach ($keys as $k) {
$k = $this->db->protectIdentifiers($k, false);
[$keyName] = $this->parseKey($k);

$keyName = $this->db->protectIdentifiers($keyName, false);

if (! in_array($k, $this->QBKeys, true)) {
$this->QBKeys[] = $k;
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 +2580,9 @@ public function updateBatch($set = null, $constraints = null, int $batchSize = 1
*
* @used-by batchExecute
*
* @param string $table Protected table name
* @param list<string> $keys QBKeys
* @param list<list<int|string>> $values QBSet
* @param string $table Protected table name
* @param list<string> $keys QBKeys
* @param list<list<int|SqlValue|string>> $values QBSet
*/
protected function _updateBatch(string $table, array $keys, array $values): string
{
102 changes: 101 additions & 1 deletion system/Database/Postgre/Builder.php
Original file line number Diff line number Diff line change
@@ -14,6 +14,7 @@
use CodeIgniter\Database\BaseBuilder;
use CodeIgniter\Database\Exceptions\DatabaseException;
use CodeIgniter\Database\RawSql;
use CodeIgniter\Database\SqlValue;
use InvalidArgumentException;

/**
@@ -146,7 +147,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.');
}
@@ -312,6 +313,105 @@ 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<string> $keys QBKeys
* @param list<list<int|SqlValue|string>> $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';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alias name is not getting quoted like:

"alias"."column"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems $this->QBOptions['alias'] is quoted.

$this->QBOptions['alias'] = $this->db->protectIdentifiers($alias);


$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 function ($key, $index) {
if ($index instanceof SqlValue) {
return $index->getValue() . '::' . $index->getType() . ' ' . $key;
}

return $index . ' ' . $key;
},
$keys,
$value
)),
$values
)
) . "\n";
}

return str_replace('{:_table_:}', $data, $sql);
}

/**
* Generates a platform-specific upsertBatch string from the supplied data
*
53 changes: 53 additions & 0 deletions system/Database/SqlValue.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
<?php

/**
* This file is part of CodeIgniter 4 framework.
*
* (c) CodeIgniter Foundation <admin@codeigniter.com>
*
* 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;
}
}
36 changes: 28 additions & 8 deletions tests/system/Database/Live/UpdateTest.php
Original file line number Diff line number Diff line change
@@ -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',
]);
}

3 changes: 3 additions & 0 deletions user_guide_src/source/changelogs/v4.4.5.rst
Original file line number Diff line number Diff line change
@@ -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 <https://github.com/codeigniter4/CodeIgniter4/blob/develop/CHANGELOG.md>`_
for a complete list of bugs fixed.
25 changes: 18 additions & 7 deletions user_guide_src/source/database/query_builder.rst
Original file line number Diff line number Diff line change
@@ -1107,18 +1107,29 @@ $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
^^^^^^^^^^^^^^^^

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

The first parameter is an associative array of values, the second parameter is the where keys.

.. 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 key.
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
``<column_name>::<type>`` (e.g., ``updated_at::TIMESTAMP``). This feature
can be used since v4.4.5 and only for ``updateBatch()`` on PostgreSQL.

.. literalinclude:: query_builder/123.php
michalsn marked this conversation as resolved.
Show resolved Hide resolved

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 +1141,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.

27 changes: 27 additions & 0 deletions user_guide_src/source/database/query_builder/123.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?php

$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',
],
];
$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"
*/

Unchanged files with check annotations Beta

$this->assertSame(exif_imagetype($this->root . 'ci-logo.png'), IMAGETYPE_PNG);
}
public function testImageReorientLandscape(): void

Check warning on line 410 in tests/system/Images/ImageMagickHandlerTest.php

GitHub Actions / Others (8.1) / Sanity Tests

Took 0.54s from 0.50s limit to run CodeIgniter\\Images\\ImageMagickHandlerTest::testImageReorientLandscape
{
for ($i = 0; $i <= 8; $i++) {
$source = $this->origin . 'EXIFsamples/landscape_' . $i . '.jpg';
}
}
public function testImageReorientPortrait(): void

Check warning on line 429 in tests/system/Images/ImageMagickHandlerTest.php

GitHub Actions / Others (8.1) / Sanity Tests

Took 0.52s from 0.50s limit to run CodeIgniter\\Images\\ImageMagickHandlerTest::testImageReorientPortrait
{
for ($i = 0; $i <= 8; $i++) {
$source = $this->origin . 'EXIFsamples/portrait_' . $i . '.jpg';
$this->assertTrue($databaseCreated);
}
public function testCreateDatabaseIfNotExists(): void

Check warning on line 60 in tests/system/Database/Live/ForgeTest.php

GitHub Actions / DatabaseLive (8.1, SQLSRV, 8.0) / tests

Took 0.60s from 0.50s limit to run CodeIgniter\\Database\\Live\\ForgeTest::testCreateDatabaseIfNotExists
{
if ($this->db->DBDriver === 'OCI8') {
$this->markTestSkipped('OCI8 does not support create database.');
$this->forge->createTable('forge_test_table');
}
public function testRenameTable(): void

Check warning on line 387 in tests/system/Database/Live/ForgeTest.php

GitHub Actions / DatabaseLive (8.1, SQLSRV, 8.0) / tests

Took 0.53s from 0.50s limit to run CodeIgniter\\Database\\Live\\ForgeTest::testRenameTable
{
$this->forge->dropTable('forge_test_table_dummy', true);
$this->forge->dropTable('', true);
}
public function testForeignKey(): void

Check warning on line 443 in tests/system/Database/Live/ForgeTest.php

GitHub Actions / DatabaseLive (8.1, OCI8, 8.0) / tests

Took 3.10s from 0.50s limit to run CodeIgniter\\Database\\Live\\ForgeTest::testForeignKey

Check warning on line 443 in tests/system/Database/Live/ForgeTest.php

GitHub Actions / DatabaseLive (8.1, SQLSRV, 8.0) / tests

Took 0.53s from 0.50s limit to run CodeIgniter\\Database\\Live\\ForgeTest::testForeignKey
{
$this->forge->dropTable('forge_test_invoices', true);
$this->forge->dropTable('forge_test_users', true);
$this->forge->createTable('forge_test_invoices', true, $attributes);
}
public function testDropForeignKey(): void

Check warning on line 733 in tests/system/Database/Live/ForgeTest.php

GitHub Actions / DatabaseLive (8.1, SQLSRV, 8.0) / tests

Took 0.55s from 0.50s limit to run CodeIgniter\\Database\\Live\\ForgeTest::testDropForeignKey
{
$this->forge->dropTable('forge_test_invoices', true);
$this->forge->dropTable('forge_test_users', true);
$this->forge->dropTable('forge_test_1', true);
}
public function testSetKeyNames(): void

Check warning on line 1185 in tests/system/Database/Live/ForgeTest.php

GitHub Actions / DatabaseLive (8.1, OCI8, 8.0) / tests

Took 1.32s from 0.50s limit to run CodeIgniter\\Database\\Live\\ForgeTest::testSetKeyNames
{
$this->forge->dropTable('forge_test_1', true);
$this->forge->dropTable('forge_test_four', true);
}
public function testDropKey(): void

Check warning on line 1509 in tests/system/Database/Live/ForgeTest.php

GitHub Actions / DatabaseLive (8.1, OCI8, 8.0) / tests

Took 1.14s from 0.50s limit to run CodeIgniter\\Database\\Live\\ForgeTest::testDropKey
{
$this->forge->dropTable('key_test_users', true);
$keyName = 'key_test_users_id';
$this->forge->dropTable('forge_test_users', true);
}
public function testProcessIndexes(): void

Check warning on line 1596 in tests/system/Database/Live/ForgeTest.php

GitHub Actions / DatabaseLive (8.1, OCI8, 8.0) / tests

Took 1.73s from 0.50s limit to run CodeIgniter\\Database\\Live\\ForgeTest::testProcessIndexes

Check warning on line 1596 in tests/system/Database/Live/ForgeTest.php

GitHub Actions / DatabaseLive (8.1, SQLSRV, 8.0) / tests

Took 0.52s from 0.50s limit to run CodeIgniter\\Database\\Live\\ForgeTest::testProcessIndexes
{
// make sure tables don't exist
$this->forge->dropTable('actions', true);
$this->assertSame($expected, $result);
}
public function testMigrateWithWithTwoNamespaces(): void

Check warning on line 97 in tests/system/Commands/Database/MigrateStatusTest.php

GitHub Actions / DatabaseLive (8.1, OCI8, 8.0) / tests

Took 1.75s from 0.50s limit to run CodeIgniter\\Commands\\Database\\MigrateStatusTest::testMigrateWithWithTwoNamespaces
{
command('migrate -n App');
command('migrate -n Tests\\\\Support');
$this->assertSame('MySQLi', $this->getPrivateProperty($db1, 'DBDriver'));
}
public function testConnectWithFailover(): void

Check warning on line 95 in tests/system/Database/Live/ConnectTest.php

GitHub Actions / DatabaseLive (8.1, OCI8, 8.0) / tests

Took 1.34s from 0.50s limit to run CodeIgniter\\Database\\Live\\ConnectTest::testConnectWithFailover
{
$this->tests['failover'][] = $this->tests;
unset($this->tests['failover'][0]['failover']);
*/
final class WhenWhenNotModelTest extends LiveModelTestCase
{
public function testWhenWithTrueCondition(): void

Check warning on line 23 in tests/system/Models/WhenWhenNotModelTest.php

GitHub Actions / DatabaseLive (8.1, OCI8, 8.0) / tests

Took 1.19s from 0.50s limit to run CodeIgniter\\Models\\WhenWhenNotModelTest::testWhenWithTrueCondition
{
$secondaryData = [
[
$this->seeInDatabase('job', ['name' => 'Grocery Sales']);
}
public function testInsertBatch(): void

Check warning on line 50 in tests/system/Database/Live/InsertTest.php

GitHub Actions / DatabaseLive (8.1, OCI8, 8.0) / tests

Took 1.12s from 0.50s limit to run CodeIgniter\\Database\\Live\\InsertTest::testInsertBatch
{
$jobData = [
[
$this->createModel(UserModel::class)->insert([]);
}
public function testInsertPermitInsertNoData(): void

Check warning on line 237 in tests/system/Models/InsertModelTest.php

GitHub Actions / DatabaseLive (8.1, OCI8, 8.0) / tests

Took 1.09s from 0.50s limit to run CodeIgniter\\Models\\InsertModelTest::testInsertPermitInsertNoData
{
$forge = Database::forge();
$forge->addField([
protected $refresh = true;
public function testCreateAddsToDatabase(): void

Check warning on line 32 in tests/system/Database/Live/FabricatorLiveTest.php

GitHub Actions / DatabaseLive (8.1, OCI8, 8.0) / tests

Took 1.03s from 0.50s limit to run CodeIgniter\\Database\\Live\\FabricatorLiveTest::testCreateAddsToDatabase
{
$fabricator = new Fabricator(UserModel::class);