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
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion rector.php
Original file line number Diff line number Diff line change
Expand Up @@ -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']);
Expand Down
38 changes: 29 additions & 9 deletions system/Database/BaseBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [];

Expand Down Expand Up @@ -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;
Expand All @@ -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
*
Expand Down Expand Up @@ -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
{
Expand Down
102 changes: 101 additions & 1 deletion system/Database/Postgre/Builder.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
use CodeIgniter\Database\BaseBuilder;
use CodeIgniter\Database\Exceptions\DatabaseException;
use CodeIgniter\Database\RawSql;
use CodeIgniter\Database\SqlValue;
use InvalidArgumentException;

/**
Expand Down Expand Up @@ -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.');
}
Expand Down Expand Up @@ -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
*
Expand Down
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 <[email protected]>
*
* 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
Expand Up @@ -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',
]);
}

Expand Down
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
Expand Up @@ -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
Expand Up @@ -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

Expand All @@ -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.

Expand Down
Loading
Loading