Skip to content

Commit 86d42ac

Browse files
theodorejbclaude
andcommitted
Exclude trigger rows from SQL Server update and delete counts
The affected count from updateRows() and deleteFrom() was the sum of the row counts reported by the batch, which with SQL Server includes rows changed by triggers. The count is now selected from @@rowcount in the same batch, like for inserts, which only includes the rows changed by the statement itself (even with an INSTEAD OF trigger). MySQL and PostgreSQL row counts already exclude rows changed by triggers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 7ed6308 commit 86d42ac

3 files changed

Lines changed: 56 additions & 5 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@
1616
- With SQL Server, `insertRow()` and `insertRows()` returned the wrong ID when an insert trigger
1717
inserted into another table with an identity column. The IDs now come from `SCOPE_IDENTITY()`
1818
rather than `@@IDENTITY` (like before v7.0).
19-
- With SQL Server, the affected count from `insertRow()` and `insertRows()` no longer includes
20-
rows inserted by triggers.
19+
- With SQL Server, the affected count from `insertRow()`, `insertRows()`, `updateRows()`, and
20+
`deleteFrom()` no longer includes rows changed by triggers.
2121
- An error while fetching a row (e.g. a conversion error in a SQL Server result that's sent as it's
2222
computed) threw a `PDOException` rather than a `SqlException`. Without the exception error mode,
2323
the error was ignored and the results ended early.

‎src/PeachySql.php‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,7 @@ private function runInBatches(array $where, Closure $buildQuery, array $setColum
297297
$maxParams = $this->options->maxBoundParams;
298298

299299
if ($maxParams <= 0 || $paramCount <= $maxParams) {
300-
return $this->query($sqlParams->sql, $sqlParams->params)->getAffected();
300+
return $this->runAffectingRows($sqlParams);
301301
}
302302

303303
$batches = Query::batchWhere($where, $paramCount, $maxParams, $setColumns);
@@ -306,14 +306,28 @@ private function runInBatches(array $where, Closure $buildQuery, array $setColum
306306
$affected = 0;
307307

308308
foreach ($batches as $batchWhere) {
309-
$batchParams = $buildQuery($batchWhere);
310-
$affected += $this->query($batchParams->sql, $batchParams->params)->getAffected();
309+
$affected += $this->runAffectingRows($buildQuery($batchWhere));
311310
}
312311

313312
return $affected;
314313
});
315314
}
316315

316+
/**
317+
* Runs an update or delete query, and returns the number of affected rows. With SQL Server, the count is
318+
* selected from @@ROWCOUNT, since the query's row count would otherwise include rows changed by triggers.
319+
*/
320+
private function runAffectingRows(SqlParams $sqlParams): int
321+
{
322+
if ($this->options->driver === 'sqlsrv') {
323+
/** @var array{affected: int|string} $row */
324+
$row = $this->query($sqlParams->sql . '; SELECT @@ROWCOUNT AS affected', $sqlParams->params)->getFirst();
325+
return (int) $row['affected'];
326+
}
327+
328+
return $this->query($sqlParams->sql, $sqlParams->params)->getAffected();
329+
}
330+
317331
/**
318332
* Runs the function in a transaction (unless one has already been started), so that if it runs multiple
319333
* queries, a failure doesn't leave the changes from some of them committed.

‎tests/MssqlDbTest.php‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,43 @@ public function testExplicitIdentityValues(): void
9999
}
100100
}
101101

102+
/**
103+
* The affected count from updateRows() and deleteFrom() shouldn't include rows changed by triggers.
104+
*/
105+
public function testAffectedCountWithTrigger(): void
106+
{
107+
$db = static::dbProvider();
108+
$db->query('DROP TABLE IF EXISTS AffectedTest');
109+
$db->query('DROP TABLE IF EXISTS AffectedAudit');
110+
$db->query('CREATE TABLE AffectedAudit (id INT IDENTITY PRIMARY KEY, note INT NOT NULL)');
111+
$db->query('CREATE TABLE AffectedTest (id INT PRIMARY KEY, name NVARCHAR(50) NOT NULL)');
112+
$db->query('CREATE TRIGGER AffectedTestAudit ON AffectedTest AFTER UPDATE, DELETE AS
113+
INSERT INTO AffectedAudit (note) SELECT id FROM deleted;
114+
INSERT INTO AffectedAudit (note) SELECT id FROM deleted');
115+
116+
$options = $db->options;
117+
$maxBoundParams = $options->maxBoundParams;
118+
$ids = [1, 2, 3];
119+
120+
try {
121+
// with a max of 3 bound parameters, the updates and deletes are split into multiple queries
122+
foreach ([$maxBoundParams, 3] as $maxParams) {
123+
$options->maxBoundParams = $maxParams;
124+
$db->insertRows('AffectedTest', array_map(fn($id) => ['id' => $id, 'name' => 'a'], $ids));
125+
126+
$this->assertSame(3, $db->updateRows('AffectedTest', ['name' => 'b'], ['id' => $ids]));
127+
$this->assertSame(2, $db->updateRows('AffectedTest', ['name' => 'c'], ['id' => ['eq' => $ids, 'ne' => 1]]));
128+
$this->assertSame(3, $db->deleteFrom('AffectedTest', ['id' => $ids]));
129+
}
130+
} finally {
131+
$options->maxBoundParams = $maxBoundParams;
132+
}
133+
134+
// the trigger added 2 audit rows for each updated or deleted row
135+
$audit = $db->query('SELECT COUNT(*) AS audit_count FROM AffectedAudit')->getFirst();
136+
$this->assertSame(['audit_count' => 2 * (3 + 2 + 3) * 2], $audit);
137+
}
138+
102139
public function testFetchErrorThrowsSqlException(): void
103140
{
104141
// without ORDER BY, rows are sent as they're computed, so the error on the last row occurs while fetching

0 commit comments

Comments
 (0)