From 1b992e6f6a1ce380579906c9cf252be4c276326b Mon Sep 17 00:00:00 2001 From: michalsn Date: Tue, 18 Feb 2025 21:56:30 +0100 Subject: [PATCH 1/4] fix: upsert with composite unique index --- system/Database/Postgre/Builder.php | 5 ++-- system/Database/SQLite3/Builder.php | 4 +-- .../20160428212500_Create_test_tables.php | 10 +++++++ .../_support/Database/Seeds/CITestSeeder.php | 12 ++++++++ tests/system/Database/Live/MetadataTest.php | 1 + tests/system/Database/Live/UpsertTest.php | 29 ++++++++++++++++++- user_guide_src/source/changelogs/v4.6.1.rst | 1 + 7 files changed, 56 insertions(+), 6 deletions(-) diff --git a/system/Database/Postgre/Builder.php b/system/Database/Postgre/Builder.php index 27a72b58c3..8d0d569d5e 100644 --- a/system/Database/Postgre/Builder.php +++ b/system/Database/Postgre/Builder.php @@ -469,9 +469,8 @@ class Builder extends BaseBuilder return ($index->type === 'UNIQUE' || $index->type === 'PRIMARY') && $hasAllFields; }); - foreach (array_map(static fn ($index) => $index->fields, $allIndexes) as $index) { - $constraints[] = current($index); - // only one index can be used? + foreach ($allIndexes as $index) { + $constraints = $index->fields; break; } diff --git a/system/Database/SQLite3/Builder.php b/system/Database/SQLite3/Builder.php index 6e62907ac1..3aa13edeb9 100644 --- a/system/Database/SQLite3/Builder.php +++ b/system/Database/SQLite3/Builder.php @@ -151,8 +151,8 @@ class Builder extends BaseBuilder return ($index->type === 'PRIMARY' || $index->type === 'UNIQUE') && $hasAllFields; }); - foreach (array_map(static fn ($index) => $index->fields, $allIndexes) as $index) { - $constraints[] = current($index); + foreach ($allIndexes as $index) { + $constraints = $index->fields; break; } diff --git a/tests/_support/Database/Migrations/20160428212500_Create_test_tables.php b/tests/_support/Database/Migrations/20160428212500_Create_test_tables.php index 8a7cb5b7d1..506ba7c41b 100644 --- a/tests/_support/Database/Migrations/20160428212500_Create_test_tables.php +++ b/tests/_support/Database/Migrations/20160428212500_Create_test_tables.php @@ -47,6 +47,16 @@ class Migration_Create_test_tables extends Migration 'value' => ['type' => 'VARCHAR', 'constraint' => 400, 'null' => true], ])->addKey('id', true)->createTable('misc', true); + $this->forge->addField([ + 'id' => ['type' => 'INTEGER', 'constraint' => 3, 'auto_increment' => true], + 'city' => ['type' => 'VARCHAR', 'constraint' => 40], + 'country' => ['type' => 'VARCHAR', 'constraint' => 40], + 'population' => ['type' => 'INTEGER', 'constraint' => 3, 'unsigned' => true, 'null' => true], + 'created_at' => ['type' => 'DATETIME', 'null' => true], + 'updated_at' => ['type' => 'DATETIME', 'null' => true], + 'deleted_at' => ['type' => 'DATETIME', 'null' => true], + ])->addKey('id', true)->addUniqueKey(['city', 'country'])->createTable('cities', true); + // Database Type test table // missing types: // TINYINT,MEDIUMINT,BIT,YEAR,BINARY,VARBINARY,TINYTEXT,LONGTEXT, diff --git a/tests/_support/Database/Seeds/CITestSeeder.php b/tests/_support/Database/Seeds/CITestSeeder.php index 9ce0bce4f1..9a42bdeec1 100644 --- a/tests/_support/Database/Seeds/CITestSeeder.php +++ b/tests/_support/Database/Seeds/CITestSeeder.php @@ -107,6 +107,18 @@ class CITestSeeder extends Seeder 'value' => 'ടൈപ്പ്', ], ], + 'cities' => [ + [ + 'city' => 'Tokyo', + 'country' => 'Japan', + 'population' => 37_115_035, + ], + [ + 'city' => 'Delhi', + 'country' => 'India', + 'population' => 33_807_403, + ], + ], 'type_test' => [ [ 'type_varchar' => 'test', diff --git a/tests/system/Database/Live/MetadataTest.php b/tests/system/Database/Live/MetadataTest.php index 0488dd2f4a..a0921988ef 100644 --- a/tests/system/Database/Live/MetadataTest.php +++ b/tests/system/Database/Live/MetadataTest.php @@ -44,6 +44,7 @@ final class MetadataTest extends CIUnitTestCase $tables = [ $prefix . 'migrations', + $prefix . 'cities', $prefix . 'user', $prefix . 'job', $prefix . 'misc', diff --git a/tests/system/Database/Live/UpsertTest.php b/tests/system/Database/Live/UpsertTest.php index a702ff5e1b..8fe7277e39 100644 --- a/tests/system/Database/Live/UpsertTest.php +++ b/tests/system/Database/Live/UpsertTest.php @@ -448,7 +448,7 @@ final class UpsertTest extends CIUnitTestCase $this->assertSame('El Salvador', $data[4]->country); } - public function testUpsertWithMatchingDataOnUniqueIndexandPrimaryKey(): void + public function testUpsertWithMatchingDataOnUniqueIndexAndPrimaryKey(): void { $data = [ 'id' => 6, @@ -607,6 +607,33 @@ final class UpsertTest extends CIUnitTestCase } } + /** + * @see https://github.com/codeigniter4/CodeIgniter4/issues/9450 + */ + public function testUpsertBatchCompositeUniqueIndex(): void + { + $data = [ + [ + 'id' => 1, + 'city' => 'Tokyo', + 'country' => 'Japan', + 'population' => 222 + ], + [ + 'id' => 2, + 'city' => 'Delhi', + 'country' => 'India', + 'population' => 111 + ], + ]; + + // uses city_country (city,country) - composite unique index + $this->db->table('cities')->upsertBatch($data); + + $this->seeInDatabase('cities', ['id' => 1, 'population' => 222]); + $this->seeInDatabase('cities', ['id' => 2, 'population' => 111]); + } + public function testSetBatchOneRow(): void { $data = [ diff --git a/user_guide_src/source/changelogs/v4.6.1.rst b/user_guide_src/source/changelogs/v4.6.1.rst index 9a3ccfe764..9a5ea391d4 100644 --- a/user_guide_src/source/changelogs/v4.6.1.rst +++ b/user_guide_src/source/changelogs/v4.6.1.rst @@ -32,6 +32,7 @@ Bugs Fixed - **CURLRequest:** Fixed an issue where multiple header sections appeared in the CURL response body during multiple redirects from the target server. - **Cors:** Fixed a bug in the Cors filter that caused the appropriate headers to not be added when another filter returned a response object in the ``before`` filter. +- **Database:** Fixed a bug in ``Postgre`` and ``SQLite3`` handlers where composite unique keys were not fully taken into account for ``upsert`` type of queries. See the repo's `CHANGELOG.md `_ From 1625eaf16025cd6ef5aaec717340058578bd65af Mon Sep 17 00:00:00 2001 From: michalsn Date: Tue, 18 Feb 2025 22:01:54 +0100 Subject: [PATCH 2/4] cs fix --- tests/_support/Database/Seeds/CITestSeeder.php | 8 ++++---- tests/system/Database/Live/UpsertTest.php | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/_support/Database/Seeds/CITestSeeder.php b/tests/_support/Database/Seeds/CITestSeeder.php index 9a42bdeec1..be695e536c 100644 --- a/tests/_support/Database/Seeds/CITestSeeder.php +++ b/tests/_support/Database/Seeds/CITestSeeder.php @@ -109,13 +109,13 @@ class CITestSeeder extends Seeder ], 'cities' => [ [ - 'city' => 'Tokyo', - 'country' => 'Japan', + 'city' => 'Tokyo', + 'country' => 'Japan', 'population' => 37_115_035, ], [ - 'city' => 'Delhi', - 'country' => 'India', + 'city' => 'Delhi', + 'country' => 'India', 'population' => 33_807_403, ], ], diff --git a/tests/system/Database/Live/UpsertTest.php b/tests/system/Database/Live/UpsertTest.php index 8fe7277e39..5f3c73508a 100644 --- a/tests/system/Database/Live/UpsertTest.php +++ b/tests/system/Database/Live/UpsertTest.php @@ -617,13 +617,13 @@ final class UpsertTest extends CIUnitTestCase 'id' => 1, 'city' => 'Tokyo', 'country' => 'Japan', - 'population' => 222 + 'population' => 222, ], [ 'id' => 2, 'city' => 'Delhi', 'country' => 'India', - 'population' => 111 + 'population' => 111, ], ]; From f67babcec0480b778aba5065366f5ee29002c508 Mon Sep 17 00:00:00 2001 From: michalsn Date: Wed, 19 Feb 2025 08:32:03 +0100 Subject: [PATCH 3/4] update tests --- .../20160428212500_Create_test_tables.php | 13 +++++---- .../_support/Database/Seeds/CITestSeeder.php | 16 ++++++----- tests/system/Database/Live/MetadataTest.php | 2 +- tests/system/Database/Live/UpsertTest.php | 27 ++++++++++--------- 4 files changed, 31 insertions(+), 27 deletions(-) diff --git a/tests/_support/Database/Migrations/20160428212500_Create_test_tables.php b/tests/_support/Database/Migrations/20160428212500_Create_test_tables.php index 506ba7c41b..6af55fa7c9 100644 --- a/tests/_support/Database/Migrations/20160428212500_Create_test_tables.php +++ b/tests/_support/Database/Migrations/20160428212500_Create_test_tables.php @@ -47,15 +47,14 @@ class Migration_Create_test_tables extends Migration 'value' => ['type' => 'VARCHAR', 'constraint' => 400, 'null' => true], ])->addKey('id', true)->createTable('misc', true); + // Team members Table (composite key) $this->forge->addField([ - 'id' => ['type' => 'INTEGER', 'constraint' => 3, 'auto_increment' => true], - 'city' => ['type' => 'VARCHAR', 'constraint' => 40], - 'country' => ['type' => 'VARCHAR', 'constraint' => 40], - 'population' => ['type' => 'INTEGER', 'constraint' => 3, 'unsigned' => true, 'null' => true], + 'team_id' => ['type' => 'INTEGER', 'constraint' => 3], + 'person_id' => ['type' => 'INTEGER', 'constraint' => 3], + 'role' => ['type' => 'VARCHAR', 'constraint' => 40], + 'status' => ['type' => 'VARCHAR', 'constraint' => 40], 'created_at' => ['type' => 'DATETIME', 'null' => true], - 'updated_at' => ['type' => 'DATETIME', 'null' => true], - 'deleted_at' => ['type' => 'DATETIME', 'null' => true], - ])->addKey('id', true)->addUniqueKey(['city', 'country'])->createTable('cities', true); + ])->addUniqueKey(['team_id', 'person_id'])->createTable('team_members', true); // Database Type test table // missing types: diff --git a/tests/_support/Database/Seeds/CITestSeeder.php b/tests/_support/Database/Seeds/CITestSeeder.php index be695e536c..09dba65f15 100644 --- a/tests/_support/Database/Seeds/CITestSeeder.php +++ b/tests/_support/Database/Seeds/CITestSeeder.php @@ -107,16 +107,18 @@ class CITestSeeder extends Seeder 'value' => 'ടൈപ്പ്', ], ], - 'cities' => [ + 'team_members' => [ [ - 'city' => 'Tokyo', - 'country' => 'Japan', - 'population' => 37_115_035, + 'team_id' => 1, + 'person_id' => 22, + 'role' => 'member', + 'status' => 'active', ], [ - 'city' => 'Delhi', - 'country' => 'India', - 'population' => 33_807_403, + 'team_id' => 1, + 'person_id' => 33, + 'role' => 'mentor', + 'status' => 'active', ], ], 'type_test' => [ diff --git a/tests/system/Database/Live/MetadataTest.php b/tests/system/Database/Live/MetadataTest.php index a0921988ef..643d0689fd 100644 --- a/tests/system/Database/Live/MetadataTest.php +++ b/tests/system/Database/Live/MetadataTest.php @@ -44,10 +44,10 @@ final class MetadataTest extends CIUnitTestCase $tables = [ $prefix . 'migrations', - $prefix . 'cities', $prefix . 'user', $prefix . 'job', $prefix . 'misc', + $prefix . 'team_members', $prefix . 'type_test', $prefix . 'empty', $prefix . 'secondary', diff --git a/tests/system/Database/Live/UpsertTest.php b/tests/system/Database/Live/UpsertTest.php index 5f3c73508a..000fa6fec7 100644 --- a/tests/system/Database/Live/UpsertTest.php +++ b/tests/system/Database/Live/UpsertTest.php @@ -614,24 +614,27 @@ final class UpsertTest extends CIUnitTestCase { $data = [ [ - 'id' => 1, - 'city' => 'Tokyo', - 'country' => 'Japan', - 'population' => 222, + 'team_id' => 1, + 'person_id' => 22, + 'role' => 'leader', + 'status' => 'active', ], [ - 'id' => 2, - 'city' => 'Delhi', - 'country' => 'India', - 'population' => 111, + 'team_id' => 1, + 'person_id' => 33, + 'role' => 'member', + 'status' => 'active', ], ]; - // uses city_country (city,country) - composite unique index - $this->db->table('cities')->upsertBatch($data); + // uses (team_id, person_id) - composite unique index + $this->db->table('team_members')->upsertBatch($data); - $this->seeInDatabase('cities', ['id' => 1, 'population' => 222]); - $this->seeInDatabase('cities', ['id' => 2, 'population' => 111]); + $this->seeInDatabase('team_members', ['team_id' => 1, 'person_id' => 22, 'role' => 'leader']); + $this->dontSeeInDatabase('team_members', ['team_id' => 1, 'person_id' => 22, 'role' => 'member']); + + $this->seeInDatabase('team_members', ['team_id' => 1, 'person_id' => 33, 'role' => 'member']); + $this->dontSeeInDatabase('team_members', ['team_id' => 1, 'person_id' => 33, 'role' => 'mentor']); } public function testSetBatchOneRow(): void From 332a78fdc0f6d04308341ed1dcebb03e98ab5222 Mon Sep 17 00:00:00 2001 From: michalsn Date: Wed, 19 Feb 2025 21:48:15 +0100 Subject: [PATCH 4/4] docs: handling constraints for databases other than MySQL --- user_guide_src/source/database/query_builder.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/user_guide_src/source/database/query_builder.rst b/user_guide_src/source/database/query_builder.rst index 6e4b1359c8..31515768d5 100644 --- a/user_guide_src/source/database/query_builder.rst +++ b/user_guide_src/source/database/query_builder.rst @@ -937,6 +937,10 @@ constraint by default. Here is an example using an array: .. literalinclude:: query_builder/112.php +.. note:: For databases other than MySQL, if a table has multiple keys (primary or unique), + the primary key will be prioritized by default when handling constraints. If you prefer + to use a different unique key instead of the primary key, use the ``onConstraint()`` method. + The first parameter is an associative array of values. Here is an example using an object: