Skip to content

fix: cast Postgre batch subquery values so updateBatch() and deleteBatch() accept mixed PHP types #10601

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.

Sign up for GitHub

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

Merged
paulbalandan merged 1 commit into from
Oct 9, 2026
+401 −11
Merged

fix: cast Postgre batch subquery values so updateBatch() and deleteBatch() accept mixed PHP types #10601

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
70 changes: 59 additions & 11 deletions system/Database/Postgre/Builder.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -395,14 +395,20 @@ static function ($key, $value) use ($table, $alias, $that): RawSql|string {
if (isset($this->QBOptions['setQueryAsData'])) {
$data = $this->QBOptions['setQueryAsData'];
} else {
$updateFields = $this->QBOptions['updateFields'] ?? [];

$data = implode(
" UNION ALL\n",
array_map(
static fn ($value): string => 'SELECT ' . implode(', ', array_map(
static fn ($key, $index): string => $index . ' ' . $key,
fn (array $value): string => sprintf('SELECT %s', implode(', ', array_map(
fn (string $key, float|int|string $index): string => sprintf(
'%s %s',
$this->castValue((string) $index, $this->getSourceType($table, $key, $updateFields)),
$key,
),
$keys,
$value,
)),
))),
$values,
),
) . "\n";
Expand All @@ -411,6 +417,38 @@ static function ($key, $value) use ($table, $alias, $that): RawSql|string {
return str_replace('{:_table_:}', $data, $sql);
}

/**
* Returns the type shared by every destination column fed from the source key, or null when they differ.
*
* @param array<array-key, RawSql|string> $map Destination column to source key.
*/
private function getSourceType(string $table, string $key, array $map): ?string
{
$columns = array_filter(array_keys($map, $key, true), is_string(...));

if ($columns === []) {
return $this->getFieldType($table, $key);
}

$types = array_unique(array_map(fn (string $column): ?string => $this->getFieldType($table, $column), $columns));

return count($types) === 1 ? reset($types) : null;
}

/**
* Returns the literal cast to the column type, except a numeric literal for a numeric column, which stays as is so a non-integral value never rounds into a match.
*/
private function castValue(string $value, ?string $type): string
{
$isNumericType = in_array(strtolower((string) $type), ['smallint', 'integer', 'bigint', 'numeric', 'real', 'double precision'], true);

if ($isNumericType && is_numeric($value)) {
return $value;
}

return $this->cast($value, $type);
}

/**
* Returns cast expression.
*
Expand All @@ -437,11 +475,15 @@ private function getFieldType(string $table, string $fieldName): ?string
foreach ($this->db->getFieldData($table) as $field) {
$type = $field->type;

// If `character` (or `char`) lacks a specifier, it is equivalent
// to `character(1)`.
// See https://www.postgresql.org/docs/current/datatype-character.html
if ($field->type === 'character') {
$type = $field->type . '(' . $field->max_length . ')';
// information_schema reports enum, domain and array columns as `USER-DEFINED` or `ARRAY`, which are not castable names.
if ($type === 'USER-DEFINED' || $type === 'ARRAY') {
continue;
}

// `character` without a length is `character(1)` and `character(n)` truncates on cast,
// while `bpchar` keeps the full value.
if ($type === 'character') {
$type = 'bpchar';
}

$this->QBOptions['fieldTypes'][$table][$field->name] = $type;
Expand Down Expand Up @@ -613,14 +655,20 @@ static function ($key, $value) use ($table, $alias, $that): RawSql|string {
if (isset($this->QBOptions['setQueryAsData'])) {
$data = $this->QBOptions['setQueryAsData'];
} else {
$constraints = $this->QBOptions['constraints'] ?? [];

$data = implode(
" UNION ALL\n",
array_map(
static fn ($value): string => 'SELECT ' . implode(', ', array_map(
static fn ($key, $index): string => $index . ' ' . $key,
fn (array $value): string => sprintf('SELECT %s', implode(', ', array_map(
fn (string $key, float|int|string $index): string => sprintf(
'%s %s',
$this->castValue((string) $index, $this->getSourceType($table, $key, $constraints)),
$key,
),
$keys,
$value,
)),
))),
$values,
),
) . "\n";
Expand Down
47 changes: 47 additions & 0 deletions tests/system/Database/Builder/DeleteTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
namespace CodeIgniter\Database\Builder;

use CodeIgniter\Database\BaseBuilder;
use CodeIgniter\Database\Postgre\Builder as PostgreBuilder;
use CodeIgniter\Test\CIUnitTestCase;
use CodeIgniter\Test\Mock\MockConnection;
use CodeIgniter\Test\Mock\MockQuery;
Expand Down Expand Up @@ -169,4 +170,50 @@ public function testDeleteBatchEscapesWhereInBinds(): void
$this->assertStringContainsString("IN ('anything'' OR ''1''=''1','Ahmadinejad')", $query->getQuery());
$this->assertStringNotContainsString("IN anything' OR '1'='1", $query->getQuery());
}

public function testDeleteBatchPostgreCastsPositionalAndMappedConstraints(): void
{
$db = new class (['DBDriver' => 'Postgre']) extends MockConnection {
protected function _fieldData(string $table): array
{
return [
(object) ['name' => 'id', 'type' => 'integer', 'max_length' => 32],
(object) ['name' => 'name', 'type' => 'text', 'max_length' => null],
];
}
};

$positional = (new PostgreBuilder('jobs', $db))->testMode()
->setData([['id' => 1], ['id' => '2'], ['id' => 1.4]], null, 'data')
->onConstraint('id')
->deleteBatch();

$expected = <<<'EOF'
DELETE FROM "jobs"
USING (
SELECT 1 "id" UNION ALL
SELECT CAST('2' AS INTEGER) "id" UNION ALL
SELECT 1.4 "id"
) "data"
WHERE "jobs"."id" = "data"."id"
EOF;

$this->assertSame($expected, rtrim($positional[0]));

$mapped = (new PostgreBuilder('jobs', $db))->testMode()
->setData([['value' => 'x'], ['value' => 5]], null, 'data')
->onConstraint(['name' => 'value'])
->deleteBatch();

$expected = <<<'EOF'
DELETE FROM "jobs"
USING (
SELECT CAST('x' AS TEXT) "value" UNION ALL
SELECT CAST(5 AS TEXT) "value"
) "data"
WHERE "jobs"."name" = CAST("data"."value" AS TEXT)
EOF;

$this->assertSame($expected, rtrim($mapped[0]));
}
}
69 changes: 69 additions & 0 deletions tests/system/Database/Builder/UpdateTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

use CodeIgniter\Database\BaseBuilder;
use CodeIgniter\Database\Exceptions\DatabaseException;
use CodeIgniter\Database\Postgre\Builder as PostgreBuilder;
use CodeIgniter\Test\CIUnitTestCase;
use CodeIgniter\Test\Mock\MockConnection;
use CodeIgniter\Test\Mock\MockQuery;
Expand Down Expand Up @@ -469,4 +470,72 @@ public function testSetWithAndWithoutEscape(): void
$this->assertSame($expectedSQL, str_replace("\n", ' ', $builder->getCompiledUpdate()));
$this->assertSame($expectedBinds, $builder->getBinds());
}

public function testUpdateBatchPostgreDoesNotCastEnumAndArrayColumns(): void
{
$db = new class (['DBDriver' => 'Postgre']) extends MockConnection {
protected function _fieldData(string $table): array
{
return [
(object) ['name' => 'id', 'type' => 'integer', 'max_length' => 32],
(object) ['name' => 'mood', 'type' => 'USER-DEFINED', 'max_length' => null],
(object) ['name' => 'tags', 'type' => 'ARRAY', 'max_length' => null],
];
}
};

$builder = new PostgreBuilder('jobs', $db);
$sql = $builder->testMode()->updateBatch([
['id' => 1, 'mood' => 'happy', 'tags' => '{a}'],
['id' => 2, 'mood' => 'sad', 'tags' => '{b}'],
], 'id');

$expected = <<<'EOF'
UPDATE "jobs"
SET
"mood" = _u."mood",
"tags" = _u."tags"
FROM (
SELECT 1 "id", 'happy' "mood", '{a}' "tags" UNION ALL
SELECT 2 "id", 'sad' "mood", '{b}' "tags"
) _u
WHERE "jobs"."id" = CAST(_u."id" AS INTEGER)
EOF;

$this->assertSame([$expected], $sql);
}

public function testUpdateBatchPostgreDoesNotCastSourceSharedByColumnsOfDifferentTypes(): void
{
$db = new class (['DBDriver' => 'Postgre']) extends MockConnection {
protected function _fieldData(string $table): array
{
return [
(object) ['name' => 'id', 'type' => 'integer', 'max_length' => 32],
(object) ['name' => 'number', 'type' => 'integer', 'max_length' => 32],
(object) ['name' => 'label', 'type' => 'text', 'max_length' => null],
];
}
};

$builder = new PostgreBuilder('jobs', $db);
$sql = $builder->testMode()
->setData([['id' => 1, 'value' => 1.4]], null, 'data')
->updateFields(['number' => 'value', 'label' => 'value'])
->onConstraint('id')
->updateBatch();

$expected = <<<'EOF'
UPDATE "jobs"
SET
"number" = CAST("data"."value" AS INTEGER),
"label" = CAST("data"."value" AS TEXT)
FROM (
SELECT 1 "id", 1.4 "value"
) "data"
WHERE "jobs"."id" = CAST("data"."id" AS INTEGER)
EOF;

$this->assertSame([$expected], $sql);
}
}
75 changes: 75 additions & 0 deletions tests/system/Database/Live/DeleteTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,81 @@ public function testDeleteBatchPreventsSQLInjectionInWhere(): void
$this->seeInDatabase('user', ['email' => 'derek@world.com', 'name' => 'Derek Jones']);
}

public function testDeleteBatchWithMixedConstraintValueTypesInTextColumn(): void
{
if ($this->db->DBDriver === 'SQLSRV') {
$this->markTestSkipped('SQL Server cannot compare `text` columns with `=`.');
}

if ($this->db->DBDriver === 'OCI8') {
$this->markTestSkipped('TODO: the OCI8 builder does not cast mixed `UNION ALL` values yet. Remove this skip once it does.');
}

$table = 'type_test';

$builder = $this->db->table($table);
$builder->truncate();

foreach (['Example', '587', 'kept'] as $i => $text) {
$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 + 1),
'type_datetime' => '2023-12-21 12:00:00',
]);
}

$this->db->table($table)
->setData([['text' => 'Example'], ['text' => 587]], null, 'data')
->onConstraint(['type_text' => 'text'])
->deleteBatch();

$this->dontSeeInDatabase($table, ['type_varchar' => 'test0']);
$this->dontSeeInDatabase($table, ['type_varchar' => 'test1']);
$this->seeInDatabase($table, ['type_varchar' => 'test2']);
}

public function testDeleteBatchDoesNotTruncateConstraintValueForCharColumn(): void
{
if ($this->db->DBDriver === 'OCI8') {
$this->markTestSkipped('TODO: Oracle resolves a `UNION ALL` of `CHAR` literals with different lengths to `VARCHAR2`, so the OCI8 builder must cast them. Remove this skip once it does.');
}

$table = 'type_test';

$builder = $this->db->table($table);
$builder->truncate();

for ($i = 0; $i < 2; $i++) {
$builder->insert([
'type_varchar' => 'test' . $i,
'type_char' => 'char' . $i,
'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 + 1),
'type_datetime' => '2023-12-21 12:00:00',
]);
}

$this->db->table($table)
->setData([['char' => 'char0 X'], ['char' => 'char1']], null, 'data')
->onConstraint(['type_char' => 'char'])
->deleteBatch();

$this->seeInDatabase($table, ['type_varchar' => 'test0']);
$this->dontSeeInDatabase($table, ['type_varchar' => 'test1']);
}

public function testDeleteBatchConstraintsDate(): void
{
$table = 'type_test';
Expand Down
Loading
Loading