Skip to content

Commit

Permalink
Merge pull request #8439 from kenjis/fix-postgre-updateBatch-2
Browse files Browse the repository at this point in the history 
fix: [Postgre] QueryBuilder::updateBatch() does not work (No API change)
  • Loading branch information
@kenjis
kenjis authored Jan 24, 2024
2 parents 3d8b04b + baca36e commit c7cfba4
Show file tree
Hide file tree
Showing 7 changed files with 311 additions and 43 deletions.
5 changes: 0 additions & 5 deletions phpstan-baseline.php
View file
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
7 changes: 6 additions & 1 deletion system/Database/BaseBuilder.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -169,8 +169,11 @@ class BaseBuilder
* constraints?: array,
* setQueryAsData?: string,
* sql?: string,
* alias?: string
* alias?: string,
* fieldTypes?: array<string, array<string, string>>
* }
*
* fieldTypes: [ProtectedTableName => [FieldName => Type]]
*/
protected $QBOptions;

Expand Down Expand Up @@ -1758,6 +1761,8 @@ public function getWhere($where = null, ?int $limit = null, ?int $offset = 0, bo
/**
* Compiles batch insert/update/upsert strings and runs the queries
*
* @param '_deleteBatch'|'_insertBatch'|'_updateBatch'|'_upsertBatch' $renderMethod
*
* @return false|int|string[] Number of rows inserted or FALSE on failure, SQL array when testMode
*
* @throws DatabaseException
Expand Down
128 changes: 127 additions & 1 deletion system/Database/Postgre/Builder.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.');
}
Expand Down Expand Up @@ -312,6 +312,132 @@ 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|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';

$sql = 'UPDATE ' . $this->compileIgnore('update') . $table . "\n";

$sql .= "SET\n";

$that = $this;
$sql .= implode(
",\n",
array_map(
static fn ($key, $value) => $key . ($value instanceof RawSql ?
' = ' . $value :
' = ' . $that->cast($alias . '.' . $value, $that->getFieldType($table, $key))),
array_keys($updateFields),
$updateFields
)
) . "\n";

$sql .= "FROM (\n{:_table_:}";

$sql .= ') ' . $alias . "\n";

$sql .= 'WHERE ' . implode(
' AND ',
array_map(
static function ($key, $value) use ($table, $alias, $that) {
if ($value instanceof RawSql && is_string($key)) {
return $table . '.' . $key . ' = ' . $value;
}

if ($value instanceof RawSql) {
return $value;
}

return $table . '.' . $value . ' = '
. $that->cast($alias . '.' . $value, $that->getFieldType($table, $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);
}

/**
* Returns cast expression.
*
* @TODO move this to BaseBuilder in 4.5.0
*
* @param float|int|string $expression
*/
private function cast($expression, ?string $type): string
{
return ($type === null) ? $expression : 'CAST(' . $expression . ' AS ' . strtoupper($type) . ')';
}

/**
* Returns the filed type from database meta data.
*
* @param string $table Protected table name.
* @param string $fieldName Field name. May be protected.
*/
private function getFieldType(string $table, string $fieldName): ?string
{
$fieldName = trim($fieldName, $this->db->escapeChar);

if (! isset($this->QBOptions['fieldTypes'][$table])) {
$this->QBOptions['fieldTypes'][$table] = [];

foreach ($this->db->getFieldData($table) as $field) {
$this->QBOptions['fieldTypes'][$table][$field->name] = $field->type;
}
}

return $this->QBOptions['fieldTypes'][$table][$fieldName] ?? null;
}

/**
* Generates a platform-specific upsertBatch string from the supplied data
*
Expand Down
25 changes: 16 additions & 9 deletions tests/_support/Database/Migrations/20160428212500_Create_test_tables.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,16 +46,23 @@ public function up(): void
])->addKey('id', true)->createTable('misc', true);

// Database Type test table
// missing types :
// TINYINT,MEDIUMINT,BIT,YEAR,BINARY , VARBINARY, TINYTEXT,LONGTEXT,YEAR,JSON,Spatial data types
// id must be interger else SQLite3 error on not null for autoinc field
// missing types:
// TINYINT,MEDIUMINT,BIT,YEAR,BINARY,VARBINARY,TINYTEXT,LONGTEXT,
// JSON,Spatial data types
// `id` must be INTEGER else SQLite3 error on not null for autoincrement field.
$data_type_fields = [
'id' => ['type' => 'INTEGER', 'constraint' => 20, 'auto_increment' => true],
'type_varchar' => ['type' => 'VARCHAR', 'constraint' => 40, 'null' => true],
'type_char' => ['type' => 'CHAR', 'constraint' => 10, 'null' => true],
'type_text' => ['type' => 'TEXT', 'null' => true],
'type_smallint' => ['type' => 'SMALLINT', 'null' => true],
'type_integer' => ['type' => 'INTEGER', 'null' => true],
'id' => ['type' => 'INTEGER', 'constraint' => 20, 'auto_increment' => true],
'type_varchar' => ['type' => 'VARCHAR', 'constraint' => 40, 'null' => true],
'type_char' => ['type' => 'CHAR', 'constraint' => 10, 'null' => true],
// TEXT should not be used on SQLSRV. It is deprecated.
'type_text' => ['type' => 'TEXT', 'null' => true],
'type_smallint' => ['type' => 'SMALLINT', 'null' => true],
'type_integer' => ['type' => 'INTEGER', 'null' => true],
// FLOAT should not be used on MySQL.
// CREATE TABLE t (f FLOAT, d DOUBLE);
// INSERT INTO t VALUES(99.9, 99.9);
// SELECT * FROM t WHERE f=99.9; // Empty set
// SELECT * FROM t WHERE d=99.9; // 1 row
'type_float' => ['type' => 'FLOAT', 'null' => true],
'type_numeric' => ['type' => 'NUMERIC', 'constraint' => '18,2', 'null' => true],
'type_date' => ['type' => 'DATE', 'null' => true],
Expand Down
168 changes: 149 additions & 19 deletions tests/system/Database/Live/UpdateTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,29 +111,159 @@ public function testUpdateWithWhereAndLimit(): void
}
}

public function testUpdateBatch(): void
/**
* @dataProvider provideUpdateBatch
*/
public function testUpdateBatch(string $constraints, array $data, array $expected): void
{
$data = [
[
'name' => 'Derek Jones',
'country' => 'Greece',
$table = 'type_test';

// Prepares test data.
$builder = $this->db->table($table);
$builder->truncate();

for ($i = 1; $i < 4; $i++) {
$builder->insert([
'type_varchar' => 'test' . $i,
'type_char' => 'char',
'type_text' => 'text',
'type_smallint' => 32767,
'type_integer' => 2_147_483_647,
'type_bigint' => 9_223_372_036_854_775_807,
'type_float' => 10.1,
'type_numeric' => 123.23,
'type_date' => '2023-12-0' . $i,
'type_datetime' => '2023-12-21 12:00:00',
]);
}

$this->db->table($table)->updateBatch($data, $constraints);

if ($this->db->DBDriver === 'SQLSRV') {
// We cannot compare `text` and `varchar` with `=`. It causes the error:
// [Microsoft][ODBC Driver 17 for SQL Server][SQL Server]The data types text and varchar are incompatible in the equal to operator.
// And data type `text`, `ntext`, `image` are deprecated in SQL Server 2016
// See https://github.com/codeigniter4/CodeIgniter4/pull/8439#issuecomment-1902535909
unset($expected[0]['type_text'], $expected[1]['type_text']);
}

$this->seeInDatabase($table, $expected[0]);
$this->seeInDatabase($table, $expected[1]);
}

public static function provideUpdateBatch(): iterable
{
yield from [
'constraints varchar' => [
'type_varchar',
[
[
'type_varchar' => 'test1', // Key
'type_text' => 'updated',
'type_smallint' => 9999,
'type_integer' => 9_999_999,
'type_bigint' => 9_999_999,
'type_float' => 99.9,
'type_numeric' => 999999.99,
'type_date' => '2024-01-01',
'type_datetime' => '2024-01-01 09:00:00',
],
[
'type_varchar' => 'test2', // Key
'type_text' => 'updated',
'type_smallint' => 9999,
'type_integer' => 9_999_999,
'type_bigint' => 9_999_999,
'type_float' => 99.9,
'type_numeric' => 999999.99,
'type_date' => '2024-01-01',
'type_datetime' => '2024-01-01 09:00:00',
],
],
[
[
'type_varchar' => 'test1',
'type_text' => 'updated',
'type_smallint' => 9999,
'type_integer' => 9_999_999,
'type_bigint' => 9_999_999,
'type_numeric' => 999999.99,
'type_date' => '2024-01-01',
'type_datetime' => '2024-01-01 09:00:00',
],
[
'type_varchar' => 'test2',
'type_text' => 'updated',
'type_smallint' => 9999,
'type_integer' => 9_999_999,
'type_bigint' => 9_999_999,
'type_numeric' => 999999.99,
'type_date' => '2024-01-01',
'type_datetime' => '2024-01-01 09:00:00',
],
],
],
[
'name' => 'Ahmadinejad',
'country' => 'Greece',
'constraints date' => [
'type_date',
[
[
'type_text' => 'updated',
'type_bigint' => 9_999_999,
'type_date' => '2023-12-01', // Key
'type_datetime' => '2024-01-01 09:00:00',
],
[
'type_text' => 'updated',
'type_bigint' => 9_999_999,
'type_date' => '2023-12-02', // Key
'type_datetime' => '2024-01-01 09:00:00',
],
],
[
[
'type_varchar' => 'test1',
'type_text' => 'updated',
'type_bigint' => 9_999_999,
'type_date' => '2023-12-01',
'type_datetime' => '2024-01-01 09:00:00',
],
[
'type_varchar' => 'test2',
'type_text' => 'updated',
'type_bigint' => 9_999_999,
'type_date' => '2023-12-02',
'type_datetime' => '2024-01-01 09:00:00',
],
],
],
'int as string' => [
'type_varchar',
[
[
'type_varchar' => 'test1', // Key
'type_integer' => '9999999', // PHP string
'type_bigint' => '2448114396435166946', // PHP string
],
[
'type_varchar' => 'test2', // Key
'type_integer' => '9999999', // PHP string
'type_bigint' => '2448114396435166946', // PHP string
],
],
[
[
'type_varchar' => 'test1',
'type_integer' => 9_999_999,
'type_bigint' => 2_448_114_396_435_166_946,
],
[
'type_varchar' => 'test2',
'type_integer' => 9_999_999,
'type_bigint' => 2_448_114_396_435_166_946,
],
],
],
];

$this->db->table('user')->updateBatch($data, 'name');

$this->seeInDatabase('user', [
'name' => 'Derek Jones',
'country' => 'Greece',
]);
$this->seeInDatabase('user', [
'name' => 'Ahmadinejad',
'country' => 'Greece',
]);
}

public function testUpdateWithWhereSameColumn(): void
Expand Down
3 changes: 3 additions & 0 deletions user_guide_src/source/changelogs/v4.4.5.rst
View file
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 repo's
`CHANGELOG.md <https://github.com/codeigniter4/CodeIgniter4/blob/develop/CHANGELOG.md>`_
for a complete list of bugs fixed.
Loading

0 comments on commit c7cfba4

Please sign in to comment.