From 32216127f22890878fbbdc632b61db9c6be685e7 Mon Sep 17 00:00:00 2001 From: Samuel de Rougemont Date: Mon, 12 Oct 2026 20:13:57 +0200 Subject: [PATCH] Keep the user transaction active after a failed statement When a statement failed, the driver rolled back the whole user transaction, including the work done before the failed statement, and ended it. MySQL undoes only the failed statement and keeps the transaction active, so the caller can decide whether to commit or roll back the rest. A later COMMIT kept only the work done after the error, and a savepoint created before the error could no longer be used. Roll back only the wrapper transaction of the failed statement. Inside a user transaction, the wrapper is a savepoint, so the transaction stays active. This also keeps an active LOCK TABLES transaction, and replaces the special case for a missing savepoint, since savepoint statements are not wrapped. Part of https://github.com/WordPress/sqlite-database-integration/issues/473. Co-Authored-By: Claude Opus 5.5 --- .../src/sqlite/class-wp-mysql-on-sqlite.php | 59 +++++++++++++-- .../WP_MySQL_On_SQLite_PDO_API_Tests.php | 16 ++++ .../tests/WP_MySQL_On_SQLite_Tests.php | 73 +++++++++++++++++++ 3 files changed, 140 insertions(+), 8 deletions(-) diff --git a/packages/mysql-on-sqlite/src/sqlite/class-wp-mysql-on-sqlite.php b/packages/mysql-on-sqlite/src/sqlite/class-wp-mysql-on-sqlite.php index 37d4a4c78..206a25b44 100644 --- a/packages/mysql-on-sqlite/src/sqlite/class-wp-mysql-on-sqlite.php +++ b/packages/mysql-on-sqlite/src/sqlite/class-wp-mysql-on-sqlite.php @@ -1193,16 +1193,18 @@ public function query( string $query, ?int $fetch_mode = null, ...$fetch_mode_ar $this->error_info = array( '00000', null, null ); return $stmt; } catch ( Throwable $e ) { - // MySQL error 1305 reports a missing savepoint without ending the transaction. - $preserves_transaction = $e instanceof WP_MySQL_On_SQLite_Exception - && 1305 === ( $e->errorInfo[1] ?? null ); - if ( ! $preserves_transaction ) { - try { - $this->rollback_user_transaction(); + /* + * Undo the failed statement. Like MySQL, keep an active user + * transaction open, so the caller can decide whether to commit + * or roll back the rest of it. + */ + try { + $this->rollback_wrapper_transaction(); + if ( ! $this->inTransaction() ) { $this->table_lock_active = false; - } catch ( Throwable $rollback_exception ) { - // Ignore rollback errors. } + } catch ( Throwable $rollback_exception ) { + // Ignore rollback errors. } if ( $e instanceof WP_SQLite_Information_Schema_Exception ) { $e = $this->convert_information_schema_exception( $e ); @@ -2088,6 +2090,47 @@ private function commit_wrapper_transaction(): void { $this->in_transaction = $in_transaction; } + /** + * Roll back a wrapper transaction. + * + * This undoes all changes of a failed statement. When the wrapper is + * a savepoint, the user transaction around it stays active. + */ + private function rollback_wrapper_transaction(): void { + if ( null === $this->wrapper_transaction_type ) { + return; + } + + try { + if ( 'savepoint' === $this->wrapper_transaction_type ) { + $savepoint_name = $this->get_internal_savepoint_name( 'wrapper' ); + $this->connection->query( sprintf( 'ROLLBACK TO SAVEPOINT %s', $savepoint_name ) ); + $this->connection->query( sprintf( 'RELEASE SAVEPOINT %s', $savepoint_name ) ); + } else { + $this->connection->query( 'ROLLBACK' ); + $this->in_transaction = false; + $this->savepoint_names = array(); + } + } catch ( PDOException $e ) { + /* + * Some SQLite errors roll back the whole transaction automatically, + * which also removes the savepoint. In that case, or if the rollback + * fails for any other reason, end the transaction. + * + * @see https://www.sqlite.org/lang_transaction.html#response_to_errors_within_a_transaction + */ + try { + $this->connection->query( 'ROLLBACK' ); + } catch ( PDOException $rollback_exception ) { + // The transaction was already rolled back. + } + $this->in_transaction = false; + $this->savepoint_names = array(); + } finally { + $this->wrapper_transaction_type = null; + } + } + /** * Execute the "BEGIN" or "START TRANSACTION" MySQL statement in SQLite. */ diff --git a/packages/mysql-on-sqlite/tests/WP_MySQL_On_SQLite_PDO_API_Tests.php b/packages/mysql-on-sqlite/tests/WP_MySQL_On_SQLite_PDO_API_Tests.php index 65a25f22c..ab6e05492 100644 --- a/packages/mysql-on-sqlite/tests/WP_MySQL_On_SQLite_PDO_API_Tests.php +++ b/packages/mysql-on-sqlite/tests/WP_MySQL_On_SQLite_PDO_API_Tests.php @@ -888,6 +888,22 @@ public function test_rollback(): void { $this->assertTrue( $result ); } + public function test_failed_query_keeps_transaction_active(): void { + $this->driver->exec( 'CREATE TABLE t (id INT PRIMARY KEY)' ); + $this->driver->beginTransaction(); + $this->driver->exec( 'INSERT INTO t VALUES (1)' ); + try { + $this->driver->exec( 'INSERT INTO t VALUES (1)' ); + $this->fail( 'An exception was expected, but none was thrown.' ); + } catch ( PDOException $e ) { + $this->assertSame( '23000', $e->getCode() ); + } + + $this->assertTrue( $this->driver->inTransaction() ); + $this->assertTrue( $this->driver->commit() ); + $this->assertSame( array( '1' ), $this->driver->query( 'SELECT id FROM t' )->fetchAll( PDO::FETCH_COLUMN ) ); + } + public function test_rollback_no_active_transaction(): void { $this->expectException( PDOException::class ); $this->expectExceptionMessage( 'There is no active transaction' ); diff --git a/packages/mysql-on-sqlite/tests/WP_MySQL_On_SQLite_Tests.php b/packages/mysql-on-sqlite/tests/WP_MySQL_On_SQLite_Tests.php index a5c927824..31134adcb 100644 --- a/packages/mysql-on-sqlite/tests/WP_MySQL_On_SQLite_Tests.php +++ b/packages/mysql-on-sqlite/tests/WP_MySQL_On_SQLite_Tests.php @@ -7920,6 +7920,79 @@ public function testTransactionSavepoints(): void { $this->assertSame( array(), (array) array_column( $result, 'id' ) ); } + public function testFailedStatementKeepsTransactionActive(): void { + $this->assertQuery( 'CREATE TABLE t (id INT PRIMARY KEY)' ); + $this->assertQuery( 'BEGIN' ); + $this->assertQuery( 'INSERT INTO t VALUES (1)' ); + + // A failed write and a failed read keep the transaction active. + $this->assertQueryError( + 'INSERT INTO t VALUES (1)', + 'SQLSTATE[23000]: Integrity constraint violation: 19 UNIQUE constraint failed: t.id' + ); + $this->assertTrue( $this->engine->inTransaction() ); + $this->assertQueryError( + 'SELECT * FROM missing_table', + "SQLSTATE[42S02]: Base table or view not found: 1146 Table 'missing_table' doesn't exist" + ); + $this->assertTrue( $this->engine->inTransaction() ); + + $this->assertQuery( 'INSERT INTO t VALUES (2)' ); + $this->assertQuery( 'COMMIT' ); + $result = $this->assertQuery( 'SELECT id FROM t ORDER BY id' ); + $this->assertSame( array( '1', '2' ), array_column( $result, 'id' ) ); + } + + public function testFailedStatementIsRolledBackInTransaction(): void { + $this->assertQuery( 'CREATE TABLE t (id INT PRIMARY KEY)' ); + $this->assertQuery( 'BEGIN' ); + $this->assertQuery( 'INSERT INTO t VALUES (1)' ); + + // The first row of the failed statement is not kept. + $this->assertQueryError( + 'INSERT INTO t VALUES (2), (1)', + 'SQLSTATE[23000]: Integrity constraint violation: 19 UNIQUE constraint failed: t.id' + ); + + $this->assertQuery( 'COMMIT' ); + $result = $this->assertQuery( 'SELECT id FROM t ORDER BY id' ); + $this->assertSame( array( '1' ), array_column( $result, 'id' ) ); + } + + public function testRollbackToSavepointAfterFailedStatement(): void { + $this->assertQuery( 'CREATE TABLE t (id INT PRIMARY KEY)' ); + $this->assertQuery( 'BEGIN' ); + $this->assertQuery( 'INSERT INTO t VALUES (1)' ); + $this->assertQuery( 'SAVEPOINT sp1' ); + $this->assertQuery( 'INSERT INTO t VALUES (2)' ); + $this->assertQueryError( + 'INSERT INTO t VALUES (1)', + 'SQLSTATE[23000]: Integrity constraint violation: 19 UNIQUE constraint failed: t.id' + ); + + // The savepoint still exists after the failed statement. + $this->assertQuery( 'ROLLBACK TO SAVEPOINT sp1' ); + $this->assertQuery( 'COMMIT' ); + $result = $this->assertQuery( 'SELECT id FROM t ORDER BY id' ); + $this->assertSame( array( '1' ), array_column( $result, 'id' ) ); + } + + public function testFailedStatementKeepsTableLock(): void { + $this->assertQuery( 'CREATE TABLE t (id INT PRIMARY KEY)' ); + $this->assertQuery( 'LOCK TABLES t WRITE' ); + $this->assertQuery( 'INSERT INTO t VALUES (1)' ); + $this->assertQueryError( + 'INSERT INTO t VALUES (1)', + 'SQLSTATE[23000]: Integrity constraint violation: 19 UNIQUE constraint failed: t.id' + ); + + // UNLOCK TABLES keeps the rows written before the failed statement. + $this->assertQuery( 'UNLOCK TABLES' ); + $this->assertFalse( $this->engine->inTransaction() ); + $result = $this->assertQuery( 'SELECT id FROM t' ); + $this->assertSame( array( '1' ), array_column( $result, 'id' ) ); + } + public function testSavepointWithoutTransactionDoesNotStartTransaction(): void { $this->assertQuery( 'CREATE TABLE t (id INT PRIMARY KEY, v INT)' ); $this->assertQuery( 'INSERT INTO t (id, v) VALUES (1, 1)' );