Repository navigation
TransactionScope issues a bare SAVEPOINT on SQLite without an active transaction #3436
Description
Summary
TransactionScope::begin() unconditionally takes the savepoint path on SQLite, even when no transaction is active:
if ( $in_transaction || BaseRepository::is_sqlite() ) {
$name = 'datamachine_transaction_' . ( ++self::$savepoint_sequence );
if ( false === $wpdb->query( "SAVEPOINT {$name}" ) ) {The || BaseRepository::is_sqlite() clause bypasses the autocommit/in_transaction check that guards the MySQL path. As a result, on SQLite we emit a bare SAVEPOINT with autocommit enabled and then rely on RELEASE SAVEPOINT to commit it.
That sequence is invalid in MySQL, and it is about to become invalid on SQLite as well.
Real MySQL behavior
Verified directly against MariaDB 10.11.14 with autocommit enabled:
SELECT @@autocommit, @@in_transaction; -- 1, 0
SAVEPOINT datamachine_transaction_1;
SELECT @@in_transaction; -- 0 (no transaction was opened)
A bare savepoint does not open a transaction. Referencing it afterwards fails:
SAVEPOINT datamachine_transaction_1;
UPDATE ... ;
RELEASE SAVEPOINT datamachine_transaction_1;
-- ERROR 1305 (42000): SAVEPOINT datamachine_transaction_1 does not exist
A savepoint inside a real START TRANSACTION works correctly and releases cleanly, so only the bare-savepoint path is affected.
Why this has not broken MySQL in practice
On MySQL the savepoint branch is only reached when @@autocommit = 0 or @@in_transaction = 1, so a transaction genuinely exists. The bare savepoint is emitted only on SQLite, via the is_sqlite() shortcut.
Impact
Upstream WordPress/sqlite-database-integration PR #496 aligns SQLite savepoint handling with MySQL semantics: a bare SAVEPOINT no longer opens a transaction, and referencing it later raises MySQL error 1305.
Running our exact TransactionScope sequence against that PR head:
{
"savepoint": "ok",
"in_transaction_after_savepoint": "false",
"update": "ok",
"rows_affected": 1,
"release": "ERROR: SQLSTATE[42000]: ... 1305 SAVEPOINT datamachine_transaction_1 does not exist"
}The rollback path is worse: both ROLLBACK TO SAVEPOINT and RELEASE SAVEPOINT fail with 1305, and the write remains committed.
Consequences once that lands:
TransactionScope::commit()seesfalsefromRELEASE SAVEPOINTand reports a failed commit.TransactionScope::rollback()returns early on the failedROLLBACK TO SAVEPOINTand does not undo the write, while leaving the scope marked active.
This affects every TransactionScope::begin() caller on SQLite, including the job status compare-and-swap in Jobs.php.
Expected
On SQLite, when no transaction is active, TransactionScope::begin() should open a real transaction (START TRANSACTION) rather than emitting a bare SAVEPOINT, matching the existing MySQL behavior. The savepoint path should remain only for genuinely nested scopes.
Constraints
- Do not reintroduce a driver-specific shortcut that assumes a bare savepoint opens a transaction.
- Preserve nested-scope semantics: an inner scope inside an active transaction must still use a savepoint.
- Preserve existing commit/rollback ownership and retry behavior.
- Verify against both MySQL and SQLite.
Notes
The original SQLite failure tracked in #2578 was a genuine adapter defect (a write inside a savepoint attempted a nested BEGIN IMMEDIATE). Upstream chose to fix it by matching MySQL semantics instead of tracking the phantom transaction, which is the correct resolution and exposes this latent assumption on our side.
Refs #2578
Refs WordPress/sqlite-database-integration#496
AI assistance
- AI assistance: Yes
- Tool(s): Claude Code
- Used for: Verifying MySQL savepoint semantics against MariaDB, reproducing the sequence against the upstream PR head, and drafting this report.
Activity
Align savepoint handling with MySQL semantics (#496)
chubes4 commentedon Sep 2, 2026
Correction: the original diagnosis was incomplete
Implementing this surfaced three facts that make the fix described above insufficient. Recording them because the "Expected" section is wrong as written.
1. There are two SQLite special-cases, not one
The report named line 38 (
$in_transaction || BaseRepository::is_sqlite()). The seed on line 25 matters more:On SQLite that short-circuits the probe entirely, so line 38's
|| is_sqlite()is already redundant. Removing only line 38 would have changed nothing.2. Removing both is still not enough —
@@autocommitis NULL on SQLiteThe adapter does not implement
@@autocommitand returns NULL. Probed directly againstWP_SQLite_Driver:{ "select_autocommit": null, "show_var_in_transaction": [], "select_in_transaction": null }(int) null === 0, and the existing check is0 === (int) $wpdb->get_var( 'SELECT @@autocommit' ). So with both special-cases deleted, the probe still concludes "autocommit disabled, caller owns a transaction" and still takes the bare-savepoint path. The NULL case has to be excluded explicitly.3. Naive
START TRANSACTIONintroduces a worse nesting bugA plain
START TRANSACTIONis invisible to every server variable on SQLite:{ "autocommit_after_plain_begin": null, "in_transaction_after_plain_begin": null, "show_var": null }So an inner scope cannot detect the outer one, and on SQLite a second
START TRANSACTIONimplicitly commits the first. Measured: outer writeouter, inner writeinner, inner rollback, final valueouter— the outer write was committed early and survived a rollback it should have been part of. Trading a bad savepoint for a silent early commit is worse.Actual fix
is_sqlite()special-cases.@@autocommitonly.Evidence
Patched
TransactionScope, four scenarios, identical results on MariaDB 10.11.14 and on SQLite at PR #496 head3cc53c7and at48c457e:committedBaseline code against
3cc53c7fails all four:case1_commit: false,case2: rolled_back,case3: inner,case4: inner2.WordPress test isolation is unaffected — the framework issues
SET autocommit = 0, which the adapter does persist, so tests still take the savepoint path.PR: #3437
chubes4 commentedon Sep 2, 2026
Correction: the PR for this is #3438, not #3437 as stated above.
fix: open real transactions when no boundary exists (#3438)