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 57d72f70c..5ae0d26b5 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 @@ -1210,16 +1210,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(); - $this->table_lock_active = false; - } catch ( Throwable $rollback_exception ) { - // Ignore rollback errors. - } + /* + * 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(); + } catch ( Throwable $rollback_exception ) { + // Ignore rollback errors. + } + if ( ! $this->inTransaction() ) { + $this->table_lock_active = false; } if ( $e instanceof WP_SQLite_Information_Schema_Exception ) { $e = $this->convert_information_schema_exception( $e ); @@ -1396,7 +1398,12 @@ public function commit(): bool { throw $this->new_driver_exception( 'There is no active transaction' ); } $this->flush(); - $this->commit_user_transaction(); + + // Unlike a COMMIT statement, PDO::commit() doesn't roll back on failure, + // so the caller can still call PDO::rollBack(), as with PDO MySQL. + $this->connection->query( 'COMMIT' ); + $this->in_transaction = false; + $this->savepoint_names = array(); return true; } @@ -2105,6 +2112,33 @@ 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 ( 'transaction' === $this->wrapper_transaction_type ) { + $this->rollback_user_transaction(); + } elseif ( 'savepoint' === $this->wrapper_transaction_type ) { + $savepoint_name = $this->get_internal_savepoint_name( 'wrapper' ); + try { + $this->connection->query( sprintf( 'ROLLBACK TO SAVEPOINT %s', $savepoint_name ) ); + } catch ( PDOException $e ) { + /* + * Some SQLite errors roll back the whole transaction automatically, + * which also removes the savepoint. In that case, end the transaction. + * + * @see https://www.sqlite.org/lang_transaction.html#response_to_errors_within_a_transaction + */ + $this->rollback_user_transaction(); + return; + } + $this->connection->query( sprintf( 'RELEASE SAVEPOINT %s', $savepoint_name ) ); + } + } + /** * Execute the "BEGIN" or "START TRANSACTION" MySQL statement in SQLite. */ @@ -2137,7 +2171,17 @@ private function commit_user_transaction(): void { if ( ! $this->inTransaction() ) { return; } - $this->connection->query( 'COMMIT' ); + try { + $this->connection->query( 'COMMIT' ); + } catch ( PDOException $e ) { + // Like MySQL, roll back a transaction that fails to commit. + try { + $this->rollback_user_transaction(); + } catch ( PDOException $rollback_exception ) { + // SQLite has already rolled back the transaction. + } + throw $e; + } $this->in_transaction = false; $this->savepoint_names = array(); } @@ -2146,13 +2190,16 @@ private function commit_user_transaction(): void { * Execute the "ROLLBACK" MySQL statement in SQLite. */ private function rollback_user_transaction(): void { - // MySQL doesn't throw an error if there is no active transaction. - if ( ! $this->inTransaction() ) { - return; + try { + // MySQL doesn't throw an error if there is no active transaction. + if ( $this->inTransaction() ) { + $this->connection->query( 'ROLLBACK' ); + } + } finally { + // A ROLLBACK always ends the transaction, even when it fails. + $this->in_transaction = false; + $this->savepoint_names = array(); } - $this->connection->query( 'ROLLBACK' ); - $this->in_transaction = false; - $this->savepoint_names = array(); } /** 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 ceef0137c..4454b4754 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 @@ -1003,6 +1003,43 @@ public function test_rollback_method_as_first_transaction_on_connection(): void $this->assertSame( 0, $this->driver->exec( 'CREATE TABLE t (id INT)' ) ); } + 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_failed_commit_keeps_transaction_active(): void { + $this->driver->exec( 'CREATE TABLE p (id INT PRIMARY KEY)' ); + $this->driver->exec( 'CREATE TABLE c (id INT PRIMARY KEY, p_id INT, FOREIGN KEY (p_id) REFERENCES p (id))' ); + $this->driver->beginTransaction(); + + // Make the commit fail by deferring a foreign key check to it. + $this->driver->get_sqlite_pdo()->exec( 'PRAGMA defer_foreign_keys = ON' ); + $this->driver->exec( 'INSERT INTO c VALUES (1, 1)' ); + try { + $this->driver->commit(); + $this->fail( 'An exception was expected, but none was thrown.' ); + } catch ( PDOException $e ) { + $this->assertStringContainsString( 'FOREIGN KEY constraint failed', $e->getMessage() ); + } + + // Like PDO MySQL, the transaction is still active, so it can be rolled back. + $this->assertTrue( $this->driver->inTransaction() ); + $this->assertTrue( $this->driver->rollBack() ); + $this->assertSame( array(), $this->driver->query( 'SELECT id FROM c' )->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 d404a0b44..f29956d43 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 @@ -8335,6 +8335,183 @@ 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 t1 (id INT PRIMARY KEY)' ); + $this->assertQuery( 'CREATE TABLE t2 (id INT PRIMARY KEY)' ); + $this->assertQuery( 'CREATE TABLE t3 (id INT PRIMARY KEY, t2_id INT, FOREIGN KEY (t2_id) REFERENCES t2 (id))' ); + $this->assertQuery( 'INSERT INTO t1 VALUES (1)' ); + $this->assertQuery( 'INSERT INTO t2 VALUES (1)' ); + $this->assertQuery( 'INSERT INTO t3 VALUES (1, 1)' ); + $this->assertQuery( 'BEGIN' ); + $this->assertQuery( 'INSERT INTO t1 VALUES (2)' ); + + /* + * A multi-table DELETE runs a DELETE query for each table. Deleting + * from t1 succeeds, and then deleting from t2 fails on the foreign key. + */ + $this->assertQueryError( + 'DELETE t1, t2 FROM t1 JOIN t2 ON t2.id = t1.id', + 'SQLSTATE[23000]: Integrity constraint violation: 19 FOREIGN KEY constraint failed' + ); + $this->assertTrue( $this->engine->inTransaction() ); + $this->assertQuery( 'COMMIT' ); + + // The changes of the failed statement are undone, and the rest is kept. + $result = $this->assertQuery( 'SELECT id FROM t1 ORDER BY id' ); + $this->assertSame( array( '1', '2' ), array_column( $result, 'id' ) ); + $result = $this->assertQuery( 'SELECT id FROM t2' ); + $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' ) ); + } + + /** + * @dataProvider dataFailedCommits + */ + public function testFailedCommitRollsBackTransaction( string $begin_query, string $commit_query ): void { + $this->assertQuery( 'CREATE TABLE p (id INT PRIMARY KEY)' ); + $this->assertQuery( 'CREATE TABLE c (id INT PRIMARY KEY, p_id INT, FOREIGN KEY (p_id) REFERENCES p (id))' ); + $this->assertQuery( $begin_query ); + + // Make the commit fail by deferring a foreign key check to it. + $this->sqlite->exec( 'PRAGMA defer_foreign_keys = ON' ); + $this->assertQuery( 'INSERT INTO c VALUES (1, 1)' ); + $this->assertQueryError( + $commit_query, + 'SQLSTATE[23000]: Integrity constraint violation: 19 FOREIGN KEY constraint failed' + ); + + // Like MySQL, the failed commit rolls back the transaction. + $this->assertFalse( $this->engine->inTransaction() ); + $result = $this->assertQuery( 'SELECT id FROM c' ); + $this->assertSame( array(), array_column( $result, 'id' ) ); + + // The table lock is released, so UNLOCK TABLES doesn't commit a new transaction. + $this->assertQuery( 'BEGIN' ); + $this->assertQuery( 'UNLOCK TABLES' ); + $this->assertTrue( $this->engine->inTransaction() ); + } + + public static function dataFailedCommits(): array { + return array( + 'COMMIT' => array( 'BEGIN', 'COMMIT' ), + 'UNLOCK TABLES' => array( 'LOCK TABLES c WRITE', 'UNLOCK TABLES' ), + 'implicit START TRANSACTION' => array( 'BEGIN', 'START TRANSACTION' ), + 'implicit BEGIN' => array( 'BEGIN', 'BEGIN' ), + 'implicit LOCK TABLES' => array( 'BEGIN', 'LOCK TABLES p WRITE' ), + ); + } + + public function testFailedStatementRolledBackBySqliteEndsTransaction(): void { + $this->assertQuery( 'CREATE TABLE t (id INT PRIMARY KEY)' ); + + // Some SQLite errors roll back the whole transaction. Emulate them with a trigger. + $this->sqlite->exec( + "CREATE TRIGGER t_rollback BEFORE INSERT ON t WHEN NEW.id = 2 BEGIN SELECT RAISE(ROLLBACK, 'Rolled back'); END" + ); + + $this->assertQuery( 'BEGIN' ); + $this->assertQuery( 'INSERT INTO t VALUES (1)' ); + $this->assertQuery( 'SAVEPOINT sp1' ); + $this->assertQueryError( + 'INSERT INTO t VALUES (2)', + 'SQLSTATE[23000]: Integrity constraint violation: 19 Rolled back' + ); + + // The transaction and its savepoints are gone. + $this->assertFalse( $this->engine->inTransaction() ); + $this->assertQueryError( + 'ROLLBACK TO SAVEPOINT sp1', + 'SQLSTATE[42000]: Syntax error or access violation: 1305 SAVEPOINT sp1 does not exist' + ); + + // Without a transaction, the same error leaves the connection usable. + $this->assertQueryError( + 'INSERT INTO t VALUES (2)', + 'SQLSTATE[23000]: Integrity constraint violation: 19 Rolled back' + ); + $this->assertFalse( $this->engine->inTransaction() ); + $this->assertQuery( 'INSERT INTO t VALUES (3)' ); + $result = $this->assertQuery( 'SELECT id FROM t' ); + $this->assertSame( array( '3' ), array_column( $result, 'id' ) ); + } + + public function testFailedStatementRolledBackBySqliteReleasesTableLock(): void { + $this->assertQuery( 'CREATE TABLE t (id INT PRIMARY KEY)' ); + + // Some SQLite errors roll back the whole transaction. Emulate them with a trigger. + $this->sqlite->exec( + "CREATE TRIGGER t_rollback BEFORE INSERT ON t WHEN NEW.id = 2 BEGIN SELECT RAISE(ROLLBACK, 'Rolled back'); END" + ); + + $this->assertQuery( 'LOCK TABLES t WRITE' ); + $this->assertQueryError( + 'INSERT INTO t VALUES (2)', + 'SQLSTATE[23000]: Integrity constraint violation: 19 Rolled back' + ); + $this->assertFalse( $this->engine->inTransaction() ); + + // The table lock is released, so UNLOCK TABLES doesn't commit a new transaction. + $this->assertQuery( 'BEGIN' ); + $this->assertQuery( 'UNLOCK TABLES' ); + $this->assertTrue( $this->engine->inTransaction() ); + } + 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)' );