Skip to content

Keep the user transaction active after a failed statement #536

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Nandan108 wants to merge 1 commit into
base: trunk
Choose a base branch
from
+140 −8
Open

Keep the user transaction active after a failed statement #536

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 51 additions & 8 deletions packages/mysql-on-sqlite/src/sqlite/class-wp-mysql-on-sqlite.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
Expand Down Expand Up @@ -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.
*/
Expand Down
16 changes: 16 additions & 0 deletions packages/mysql-on-sqlite/tests/WP_MySQL_On_SQLite_PDO_API_Tests.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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' );
Expand Down
73 changes: 73 additions & 0 deletions packages/mysql-on-sqlite/tests/WP_MySQL_On_SQLite_Tests.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)' );
Expand Down
Loading