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 (No API change) #8439

Merged
merged 23 commits into from
Jan 24, 2024
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
02de356
test: update comment
kenjis Jan 21, 2024
b796f0b
test: add test for type date and datetime
kenjis Jan 21, 2024
a71aa3c
refactor: copy _updateBatch() to override
kenjis Jan 17, 2024
f1b4a6c
refactor: make if condition more strict
kenjis Jan 17, 2024
8832601
fix: updateBatch() Postgre type error
kenjis Jan 21, 2024
1d96753
docs: add sub section titles and tweaks
kenjis Jan 18, 2024
9ae66a2
docs: add changelog
kenjis Jan 21, 2024
560e745
refactory: by rector
kenjis Jan 21, 2024
1bffa76
test: fix assertion for SQLSRV
kenjis Jan 21, 2024
38fb5b2
test: add test for updateBatch() with constrants DATE
kenjis Jan 22, 2024
3336010
fix: Postgre updateBatch() SQL type error in WHERE part
kenjis Jan 22, 2024
f86c6d2
test: add test for more types
kenjis Jan 22, 2024
fc0a858
refactor: add castValue() and use it
kenjis Jan 22, 2024
ec11284
test: refactor with dataProvider
kenjis Jan 22, 2024
d4f3e92
refactor: make type uppercase in SQL string for readablitiy
kenjis Jan 22, 2024
5ca6dfa
test: add test for int as string
kenjis Jan 22, 2024
64cd791
test: FLOAT on MySQL cannot be used as WHERE = key
kenjis Jan 22, 2024
6d4ce25
refactor: change castValue() API
kenjis Jan 23, 2024
cd644df
docs: add @param
kenjis Jan 23, 2024
92cc84e
refactor: change castValue(),getFieldTypes() to cast(),getFieldType()
kenjis Jan 23, 2024
fa2e073
refactor: by rector
kenjis Jan 23, 2024
fbacb30
refactor: use `CAST(expression AS type)` instead of `::type`
kenjis Jan 23, 2024
baca36e
docs: update PHPDoc
kenjis Jan 24, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
5 changes: 4 additions & 1 deletion system/Database/BaseBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,8 @@ class BaseBuilder
* constraints?: array,
* setQueryAsData?: string,
* sql?: string,
* alias?: string
* alias?: string,
* fieldTypes?: array<string, string>
* }
*/
protected $QBOptions;
Expand Down Expand Up @@ -1758,6 +1759,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
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
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
Copy link
Member

Choose a reason for hiding this comment

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

Float is an approximation of a number. The following would work:

SELECT * FROM t WHERE f > 99.89 AND f < 99.91;

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, and we cannot use seeInDatabase().

// 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
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
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
Loading