From 5fb20ae07a89b4e53d3afb80702bcf491aa97e0a Mon Sep 17 00:00:00 2001 From: michalsn Date: Thu, 2 Jan 2025 09:46:20 +0100 Subject: [PATCH 1/6] fix: gather affected rows after query call failed --- system/Database/BaseBuilder.php | 2 +- system/Database/Postgre/Connection.php | 4 ++++ system/Database/SQLSRV/Connection.php | 4 ++++ tests/system/Database/Live/InsertTest.php | 21 +++++++++++++++- .../system/Database/Live/TransactionTest.php | 24 +++++++++++++++++++ user_guide_src/source/changelogs/v4.5.8.rst | 2 ++ 6 files changed, 55 insertions(+), 2 deletions(-) diff --git a/system/Database/BaseBuilder.php b/system/Database/BaseBuilder.php index ca64800463a2..51ed022d131d 100644 --- a/system/Database/BaseBuilder.php +++ b/system/Database/BaseBuilder.php @@ -2198,7 +2198,7 @@ protected function formatValues(array $values): array * * @param array|object|null $set a dataset * - * @return false|int|list Number of rows inserted or FALSE on failure, SQL array when testMode + * @return false|int|list Number of rows inserted or FALSE on no data to perform an insert operation, SQL array when testMode */ public function insertBatch($set = null, ?bool $escape = null, int $batchSize = 100) { diff --git a/system/Database/Postgre/Connection.php b/system/Database/Postgre/Connection.php index ad278145afef..64b85eafb064 100644 --- a/system/Database/Postgre/Connection.php +++ b/system/Database/Postgre/Connection.php @@ -227,6 +227,10 @@ protected function getDriverFunctionPrefix(): string */ public function affectedRows(): int { + if ($this->resultID === false) { + return 0; + } + return pg_affected_rows($this->resultID); } diff --git a/system/Database/SQLSRV/Connection.php b/system/Database/SQLSRV/Connection.php index 6903f42eebac..26b4983dba0a 100644 --- a/system/Database/SQLSRV/Connection.php +++ b/system/Database/SQLSRV/Connection.php @@ -444,6 +444,10 @@ public function error(): array */ public function affectedRows(): int { + if ($this->resultID === false) { + return 0; + } + return sqlsrv_rows_affected($this->resultID); } diff --git a/tests/system/Database/Live/InsertTest.php b/tests/system/Database/Live/InsertTest.php index ca2117a06b35..01cc74e9c26b 100644 --- a/tests/system/Database/Live/InsertTest.php +++ b/tests/system/Database/Live/InsertTest.php @@ -13,6 +13,7 @@ namespace CodeIgniter\Database\Live; +use CodeIgniter\Database\Exceptions\DatabaseException; use CodeIgniter\Database\Forge; use CodeIgniter\Database\RawSql; use CodeIgniter\Test\CIUnitTestCase; @@ -79,13 +80,31 @@ public function testInsertBatch(): void ], ]; - $this->db->table($table)->insertBatch($data); + $count = $this->db->table($table)->insertBatch($data); + + $this->assertSame(2, $count); $expected = $data; $this->seeInDatabase($table, $expected[0]); $this->seeInDatabase($table, $expected[1]); } + public function testInsertBatchFailed(): void + { + $this->expectException(DatabaseException::class); + + $data = [ + [ + 'name' => 'Grocery Sales', + ], + [ + 'name' => null, + ], + ]; + + $this->db->table('job')->insertBatch($data); + } + public function testReplaceWithNoMatchingData(): void { $data = [ diff --git a/tests/system/Database/Live/TransactionTest.php b/tests/system/Database/Live/TransactionTest.php index 3414d2dfffdf..d3915c2217c8 100644 --- a/tests/system/Database/Live/TransactionTest.php +++ b/tests/system/Database/Live/TransactionTest.php @@ -239,4 +239,28 @@ public function testTransStrictFalseAndDBDebugFalse(): void $this->enableDBDebug(); } + + /** + * @see https://github.com/codeigniter4/CodeIgniter4/issues/9362 + */ + public function testTransInsertBatchFailed(): void + { + $data = [ + [ + 'name' => 'Grocery Sales', + ], + [ + 'name' => null, + ], + ]; + + $this->db->transBegin(); + $this->db->table('job')->insertBatch($data); + + $this->assertFalse($this->db->transStatus()); + + $this->db->transRollback(); + + $this->dontSeeInDatabase('job', ['name' => 'Grocery Sales']); + } } diff --git a/user_guide_src/source/changelogs/v4.5.8.rst b/user_guide_src/source/changelogs/v4.5.8.rst index cad7fec30248..ae29b59fb165 100644 --- a/user_guide_src/source/changelogs/v4.5.8.rst +++ b/user_guide_src/source/changelogs/v4.5.8.rst @@ -30,6 +30,8 @@ Deprecations Bugs Fixed ********** +- **Database:** Fixed a bug where ``Builder::affectedRows()`` threw an error when the previous query call failed in ``Postgre`` and ``SQLSRV`` drivers. + See the repo's `CHANGELOG.md `_ for a complete list of bugs fixed. From 9ecec565e750547111d4ae626591e6e631c7516d Mon Sep 17 00:00:00 2001 From: michalsn Date: Thu, 2 Jan 2025 09:54:34 +0100 Subject: [PATCH 2/6] update user guide --- user_guide_src/source/database/query_builder.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/user_guide_src/source/database/query_builder.rst b/user_guide_src/source/database/query_builder.rst index 454fc56c6bbf..6e4b1359c898 100644 --- a/user_guide_src/source/database/query_builder.rst +++ b/user_guide_src/source/database/query_builder.rst @@ -1870,7 +1870,7 @@ Class Reference :param array $set: Data to insert :param bool $escape: Whether to escape values :param int $batch_size: Count of rows to insert at once - :returns: Number of rows inserted or ``false`` on failure + :returns: Number of rows inserted or ``false`` on no data to perform an insert operation :rtype: int|false Compiles and executes batch ``INSERT`` statements. From e68b673d4dedd5a6a75720e76bb90d095f046c15 Mon Sep 17 00:00:00 2001 From: michalsn Date: Thu, 2 Jan 2025 10:11:42 +0100 Subject: [PATCH 3/6] fix test --- tests/system/Database/Live/TransactionTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/system/Database/Live/TransactionTest.php b/tests/system/Database/Live/TransactionTest.php index d3915c2217c8..e1c718058a77 100644 --- a/tests/system/Database/Live/TransactionTest.php +++ b/tests/system/Database/Live/TransactionTest.php @@ -259,7 +259,7 @@ public function testTransInsertBatchFailed(): void $this->assertFalse($this->db->transStatus()); - $this->db->transRollback(); + $this->db->transComplete(); $this->dontSeeInDatabase('job', ['name' => 'Grocery Sales']); } From bf91cad10f26023545144626902a0e049793d042 Mon Sep 17 00:00:00 2001 From: michalsn Date: Thu, 2 Jan 2025 10:30:28 +0100 Subject: [PATCH 4/6] fix test --- tests/system/Database/Live/TransactionTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/system/Database/Live/TransactionTest.php b/tests/system/Database/Live/TransactionTest.php index e1c718058a77..aced7767bcca 100644 --- a/tests/system/Database/Live/TransactionTest.php +++ b/tests/system/Database/Live/TransactionTest.php @@ -254,7 +254,7 @@ public function testTransInsertBatchFailed(): void ], ]; - $this->db->transBegin(); + $this->db->transStrict(false)->transBegin(); $this->db->table('job')->insertBatch($data); $this->assertFalse($this->db->transStatus()); From 20a2e91a07a35deb2444b71d29e22ac7c2a713b1 Mon Sep 17 00:00:00 2001 From: michalsn Date: Thu, 2 Jan 2025 11:25:24 +0100 Subject: [PATCH 5/6] fix failing tests for mysqli - enable strict mode --- tests/system/Database/Live/InsertTest.php | 12 ++++++++++- .../system/Database/Live/TransactionTest.php | 20 +++++++++++++++---- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/tests/system/Database/Live/InsertTest.php b/tests/system/Database/Live/InsertTest.php index 01cc74e9c26b..4db8e8868468 100644 --- a/tests/system/Database/Live/InsertTest.php +++ b/tests/system/Database/Live/InsertTest.php @@ -102,7 +102,17 @@ public function testInsertBatchFailed(): void ], ]; - $this->db->table('job')->insertBatch($data); + $db = $this->db; + + if ($this->db->DBDriver === 'MySQLi') { + // strict mode is required for MySQLi to throw an exception here + $config = config('Database'); + $config->tests['strictOn'] = true; + + $db = Database::connect($config->tests); + } + + $db->table('job')->insertBatch($data); } public function testReplaceWithNoMatchingData(): void diff --git a/tests/system/Database/Live/TransactionTest.php b/tests/system/Database/Live/TransactionTest.php index aced7767bcca..610306f2fd22 100644 --- a/tests/system/Database/Live/TransactionTest.php +++ b/tests/system/Database/Live/TransactionTest.php @@ -254,12 +254,24 @@ public function testTransInsertBatchFailed(): void ], ]; - $this->db->transStrict(false)->transBegin(); - $this->db->table('job')->insertBatch($data); + $db = $this->db; - $this->assertFalse($this->db->transStatus()); + if ($this->db->DBDriver === 'MySQLi') { + // strict mode is required for MySQLi to throw an exception here + $config = config('Database'); + $config->tests['strictOn'] = true; - $this->db->transComplete(); + $db = Database::connect($config->tests); + } + + $db->transStrict(false)->transBegin(); + $db->table('job')->insertBatch($data); + + $this->assertFalse($db->transStatus()); + + $db->transComplete(); + + $db->transStrict(); $this->dontSeeInDatabase('job', ['name' => 'Grocery Sales']); } From 817948b09c8e04fcd1f43f71be853d3245f9f20b Mon Sep 17 00:00:00 2001 From: michalsn Date: Thu, 2 Jan 2025 11:29:00 +0100 Subject: [PATCH 6/6] cs fix --- tests/system/Database/Live/InsertTest.php | 2 +- tests/system/Database/Live/TransactionTest.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/system/Database/Live/InsertTest.php b/tests/system/Database/Live/InsertTest.php index 4db8e8868468..79281a384e9a 100644 --- a/tests/system/Database/Live/InsertTest.php +++ b/tests/system/Database/Live/InsertTest.php @@ -106,7 +106,7 @@ public function testInsertBatchFailed(): void if ($this->db->DBDriver === 'MySQLi') { // strict mode is required for MySQLi to throw an exception here - $config = config('Database'); + $config = config('Database'); $config->tests['strictOn'] = true; $db = Database::connect($config->tests); diff --git a/tests/system/Database/Live/TransactionTest.php b/tests/system/Database/Live/TransactionTest.php index 610306f2fd22..74df4e3fd974 100644 --- a/tests/system/Database/Live/TransactionTest.php +++ b/tests/system/Database/Live/TransactionTest.php @@ -258,7 +258,7 @@ public function testTransInsertBatchFailed(): void if ($this->db->DBDriver === 'MySQLi') { // strict mode is required for MySQLi to throw an exception here - $config = config('Database'); + $config = config('Database'); $config->tests['strictOn'] = true; $db = Database::connect($config->tests);