Skip to content

fix: gather affected rows after query call failed #9363

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

Merged
merged 6 commits into from
Jan 3, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion system/Database/BaseBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -2198,7 +2198,7 @@ protected function formatValues(array $values): array
*
* @param array|object|null $set a dataset
*
* @return false|int|list<string> Number of rows inserted or FALSE on failure, SQL array when testMode
* @return false|int|list<string> 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)
{
Expand Down
4 changes: 4 additions & 0 deletions system/Database/Postgre/Connection.php
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,10 @@ protected function getDriverFunctionPrefix(): string
*/
public function affectedRows(): int
{
if ($this->resultID === false) {
return 0;
}

return pg_affected_rows($this->resultID);
}

Expand Down
4 changes: 4 additions & 0 deletions system/Database/SQLSRV/Connection.php
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,10 @@ public function error(): array
*/
public function affectedRows(): int
{
if ($this->resultID === false) {
return 0;
}

return sqlsrv_rows_affected($this->resultID);
}

Expand Down
31 changes: 30 additions & 1 deletion tests/system/Database/Live/InsertTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -49,7 +50,7 @@
$this->seeInDatabase('job', ['name' => 'Grocery Sales']);
}

public function testInsertBatch(): void

Check warning on line 53 in tests/system/Database/Live/InsertTest.php

View workflow job for this annotation

GitHub Actions / DatabaseLive (8.2, OCI8, 8.0) / tests

Took 0.8623s from 0.5000s limit to run CodeIgniter\\Database\\Live\\InsertTest::testInsertBatch
{
$table = 'type_test';

Expand Down Expand Up @@ -79,13 +80,41 @@
],
];

$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,
],
];

$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
{
$data = [
Expand Down
36 changes: 36 additions & 0 deletions tests/system/Database/Live/TransactionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -239,4 +239,40 @@ public function testTransStrictFalseAndDBDebugFalse(): void

$this->enableDBDebug();
}

/**
* @see https://github.com/codeigniter4/CodeIgniter4/issues/9362
*/
public function testTransInsertBatchFailed(): void
{
$data = [
[
'name' => 'Grocery Sales',
],
[
'name' => null,
],
];

$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->transStrict(false)->transBegin();
$db->table('job')->insertBatch($data);

$this->assertFalse($db->transStatus());

$db->transComplete();

$db->transStrict();

$this->dontSeeInDatabase('job', ['name' => 'Grocery Sales']);
}
}
2 changes: 2 additions & 0 deletions user_guide_src/source/changelogs/v4.5.8.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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 <https://github.com/codeigniter4/CodeIgniter4/blob/develop/CHANGELOG.md>`_
for a complete list of bugs fixed.
2 changes: 1 addition & 1 deletion user_guide_src/source/database/query_builder.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Loading