[Nandan108]

Summary

Part of #473 (the "Explicit transactions" section).

When a statement fails inside a transaction, the driver rolls back the whole transaction and ends it. MySQL undoes only the failed statement and keeps the transaction active:

CREATE TABLE t (id INT PRIMARY KEY);
START TRANSACTION;
INSERT INTO t VALUES (1);
INSERT INTO t VALUES (1);  -- fails with a duplicate key error
INSERT INTO t VALUES (2);
COMMIT;
SELECT id FROM t;
-- Driver: 2     MySQL: 1, 2

The work before the error is lost, and the work after it runs in autocommit mode. A savepoint created before the error is gone too, so ROLLBACK TO SAVEPOINT fails with error 1305.

Fix

Each statement already runs in a wrapper: a transaction when none is active, or a savepoint inside a user transaction. On failure, only that wrapper is now rolled back. This:

• keeps the user transaction and its savepoints active, and still undoes all changes of the failed statement, including the earlier rows of a multi-row INSERT;
• keeps the transaction opened by LOCK TABLES, so UNLOCK TABLES commits the rows written before the error, as in MySQL;
• replaces the special case for error 1305 (missing savepoint), since savepoint statements are not wrapped and have nothing to roll back.

Without a user transaction, the behavior is unchanged: the wrapper transaction is rolled back.

Some SQLite errors roll back the whole transaction automatically (SQLite docs). Then rolling back to the savepoint fails, and the driver ends the transaction as before. This path has no test, because those errors (such as SQLITE_FULL) are hard to trigger in a unit test.

The rest of #473 (error modes and diagnostics) is not part of this PR.

Tests

• testFailedStatementKeepsTransactionActive checks that a failed write and a failed read keep the transaction active, and that COMMIT keeps the work before and after them.
• testFailedStatementIsRolledBackInTransaction checks that a failed multi-row INSERT leaves none of its rows.
• testRollbackToSavepointAfterFailedStatement checks that a savepoint survives a failed statement.
• testFailedStatementKeepsTableLock checks that UNLOCK TABLES keeps the rows written before a failed statement.
• test_failed_query_keeps_transaction_active checks the same through PDO::beginTransaction() and PDO::commit().

All five fail on trunk. The existing testMissingSavepointDoesNotRollbackTransaction still passes.

packages/mysql-on-sqlite: 1048 tests pass (PHP 8.3, SQLite 3.45.1); PHPCS is clean.

Use of AI

This change was prepared with Claude Code (Claude Opus 5.5). I reviewed the code, the tests, and this description, and I take responsibility for the change.

Note: this PR and #535 both add tests at the same spot of WP_MySQL_On_SQLite_PDO_API_Tests.php, so whichever merges second needs a trivial rebase.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Failed statements no longer unexpectedly end an active transaction. You can continue working in the transaction and commit it afterward.
  • Changes made by a failed statement are rolled back without discarding earlier work in the transaction.
  • Savepoints remain usable after a statement fails, and table locks stay active until explicitly released.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: ee80fa0e-12f5-4b99-b6be-0018f709b538

📥 Commits

Reviewing files that changed from the base of the PR and between 503958e and 4ae6d34.

📒 Files selected for processing: 3

• packages/mysql-on-sqlite/src/sqlite/class-wp-mysql-on-sqlite.php
• packages/mysql-on-sqlite/tests/WP_MySQL_On_SQLite_PDO_API_Tests.php
• packages/mysql-on-sqlite/tests/WP_MySQL_On_SQLite_Tests.php

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.

📝 Walkthrough

Query failures now roll back the active wrapper transaction. When the wrapper uses a savepoint, the surrounding user transaction remains active. Regression tests cover transaction state, failed multi-row writes, savepoints, and table locks.

Changes: Query failure rollback

Layer / File(s)	Summary
Wrapper rollback behavior and regression coverage packages/mysql-on-sqlite/src/sqlite/class-wp-mysql-on-sqlite.php, packages/mysql-on-sqlite/tests/WP_MySQL_On_SQLite_PDO_API_Tests.php, packages/mysql-on-sqlite/tests/WP_MySQL_On_SQLite_Tests.php	The query failure handler rolls back the active wrapper transaction. If the wrapper uses a savepoint, the user transaction remains active. Tests cover duplicate-key and missing-table errors, failed multi-row inserts, savepoint rollback, and table locks.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Suggested reviewers: janjakes, chubes4

Merge Risk: ⚪ Minimal · up to 4ae6d

No actionable merge-blocking issue is established; the change is ready for normal merge checks.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 3 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the primary change: preserving the user transaction after a failed statement.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Warning

Some tools did not complete. Review the errors below.

🔧 PHPStan (2.3.0)

PHPStan reported 2 non-file analysis error(s); analysis is incomplete

🔧 PHPMD (3.0.0)

packages/mysql-on-sqlite/tests/WP_MySQL_On_SQLite_Tests.php

PHPMD could not process this file (exit code 255): 0/1 [░░░░░░░░░░░░░░░░░░░░░░░░░░░░] 0%
1/1 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%
PHP Fatal error: Allowed memory size of 134217728 bytes exhausted (tried to allocate 20971520 bytes) in phar:///usr/bin/phpmd/vendor/pdepend/pdepend/src/Util/Cache/Driver/FileCacheDriver.php on line 183

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}