Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
57 changes: 54 additions & 3 deletions packages/mysql-on-sqlite/src/sqlite/class-wp-mysql-on-sqlite.php
Original file line number Diff line number Diff line change
Expand Up @@ -711,6 +711,20 @@ class WP_MySQL_On_SQLite extends PDO {
*/
private $in_transaction = false;

/**
* User savepoints in a transaction opened by a SAVEPOINT statement.
*
* On PHP < 8.4, PDO SQLite cannot detect transactions opened with raw SQL.
* Tracking the savepoint stack keeps the inTransaction() polyfill accurate
* when the outermost savepoint is released.
*
* Savepoints inside a transaction opened by BEGIN are not tracked because
* releasing them cannot end the outer transaction.
*
* @var string[]
*/
private $savepoint_transaction_stack = array();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we use a single property by tracking only savepoints in a transaction opened by SAVEPOINT? Savepoints inside a transaction opened by BEGIN do not need tracking because releasing them cannot end the outer transaction. This would remove the need for $transaction_started_by_savepoint.

Let’s also document why this state is needed. Something like:

	/**
	 * User savepoints in a transaction opened by a SAVEPOINT statement.
	 *
	 * On PHP < 8.4, PDO SQLite cannot detect transactions opened with raw SQL.
	 * Tracking the savepoint stack keeps the inTransaction() polyfill accurate
	 * when the outermost savepoint is released.
	 *
	 * Savepoints inside a transaction opened by BEGIN are not tracked because
	 * releasing them cannot end the outer transaction.
	 *
	 * @var string[]
	 */
	private $savepoint_transaction_stack = array();

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Thank you!

/**
* Whether a MySQL table lock is active.
*
Expand Down Expand Up @@ -2092,7 +2106,8 @@ private function begin_user_transaction(): void {
* @see self::begin_wrapper_transaction()
*/
$this->connection->query( 'BEGIN IMMEDIATE' );
$this->in_transaction = true;
$this->in_transaction = true;
$this->savepoint_transaction_stack = array();
}

/**
Expand All @@ -2104,7 +2119,8 @@ private function commit_user_transaction(): void {
return;
}
$this->connection->query( 'COMMIT' );
$this->in_transaction = false;
$this->in_transaction = false;
$this->savepoint_transaction_stack = array();
}

/**
Expand All @@ -2116,7 +2132,8 @@ private function rollback_user_transaction(): void {
return;
}
$this->connection->query( 'ROLLBACK' );
$this->in_transaction = false;
$this->in_transaction = false;
$this->savepoint_transaction_stack = array();
}

/**
Expand Down Expand Up @@ -2146,26 +2163,45 @@ private function execute_transaction_or_locking_statement( WP_Parser_Node $node
break;
case 'savepointStatement':
$savepoint_name = $this->translate( $subnode->get_first_child_node( 'identifier' ) );
$savepoint_key = null === $savepoint_name
? null
: strtolower( $this->unquote_sqlite_identifier( $savepoint_name ) );

// ROLLBACK/ROLLBACK TO SAVEPOINT <identifier>.
if ( WP_MySQL_Lexer::ROLLBACK_SYMBOL === $token->id ) {
if ( null === $savepoint_name ) {
$this->rollback_user_transaction();
} else {
$this->execute_sqlite_query( sprintf( 'ROLLBACK TO SAVEPOINT %s', $savepoint_name ) );
$savepoint_index = $this->find_transaction_savepoint_index( $savepoint_key );
if ( null !== $savepoint_index ) {
$this->savepoint_transaction_stack = array_slice( $this->savepoint_transaction_stack, 0, $savepoint_index + 1 );
}
}
return;
}

// SAVEPOINT.
if ( WP_MySQL_Lexer::SAVEPOINT_SYMBOL === $token->id ) {
$starts_transaction = ! $this->inTransaction();
$this->execute_sqlite_query( sprintf( 'SAVEPOINT %s', $savepoint_name ) );
if ( $starts_transaction || ! empty( $this->savepoint_transaction_stack ) ) {
$this->savepoint_transaction_stack[] = $savepoint_key;
}
$this->in_transaction = true;
return;
}

// RELEASE SAVEPOINT.
if ( WP_MySQL_Lexer::RELEASE_SYMBOL === $token->id ) {
$this->execute_sqlite_query( sprintf( 'RELEASE SAVEPOINT %s', $savepoint_name ) );
$savepoint_index = $this->find_transaction_savepoint_index( $savepoint_key );
if ( null !== $savepoint_index ) {
$this->savepoint_transaction_stack = array_slice( $this->savepoint_transaction_stack, 0, $savepoint_index );
}
if ( null !== $savepoint_index && empty( $this->savepoint_transaction_stack ) ) {
$this->in_transaction = false;
}
return;
}

Expand Down Expand Up @@ -2236,6 +2272,21 @@ private function execute_transaction_or_locking_statement( WP_Parser_Node $node
);
}

/**
* Find the innermost active user savepoint with the given name.
*
* @param string $savepoint_name Normalized savepoint name.
* @return int|null Savepoint index, or null when not tracked.
*/
private function find_transaction_savepoint_index( string $savepoint_name ): ?int {
for ( $index = count( $this->savepoint_transaction_stack ) - 1; $index >= 0; $index-- ) {
if ( $savepoint_name === $this->savepoint_transaction_stack[ $index ] ) {
return $index;
}
}
return null;
}

/**
* Translate and execute a MySQL SELECT statement in SQLite.
*
Expand Down
132 changes: 132 additions & 0 deletions packages/mysql-on-sqlite/tests/WP_MySQL_On_SQLite_PDO_API_Tests.php
Original file line number Diff line number Diff line change
Expand Up @@ -923,6 +923,138 @@ public function test_transaction_methods_flush_operation_state(): void {
$this->assertSame( array( 'ROLLBACK' ), array_column( $this->driver->get_last_sqlite_queries(), 'sql' ) );
}

public function test_standalone_write_uses_wrapper_transaction(): void {
$this->driver->query( 'CREATE TABLE t (id INT PRIMARY KEY, value INT)' );
$this->driver->query( 'INSERT INTO t VALUES (1, 1)' );

$statement = $this->driver->query( 'UPDATE t SET value = 2 WHERE id = 1' );
$queries = array_column( $this->driver->get_last_sqlite_queries(), 'sql' );

$this->assertSame( 1, $statement->rowCount() );
$this->assertSame( 'BEGIN IMMEDIATE', $queries[0] );
$this->assertSame( 'COMMIT', end( $queries ) );
}

public function test_write_inside_savepoint_commits_on_release(): void {
$this->driver->query( 'CREATE TABLE t (id INT PRIMARY KEY, value INT)' );
$this->driver->query( 'INSERT INTO t VALUES (1, 1)' );

$this->driver->query( 'SAVEPOINT outer_transaction' );
$statement = $this->driver->query( 'UPDATE t SET value = 2 WHERE id = 1' );
$queries = array_column( $this->driver->get_last_sqlite_queries(), 'sql' );

$this->assertSame( 1, $statement->rowCount() );
$this->assertNotContains( 'BEGIN IMMEDIATE', $queries );
$this->assertTrue( $this->driver->inTransaction() );

$this->driver->query( 'RELEASE SAVEPOINT outer_transaction' );

$this->assertFalse( $this->driver->inTransaction() );
$this->assertSame( '2', $this->driver->query( 'SELECT value FROM t' )->fetchColumn() );
}

public function test_releasing_savepoint_inside_explicit_transaction_keeps_transaction_active(): void {
$this->driver->query( 'CREATE TABLE t (id INT PRIMARY KEY, value INT)' );
$this->driver->query( 'INSERT INTO t VALUES (1, 1)' );

$this->driver->query( 'START TRANSACTION' );
$this->driver->query( 'SAVEPOINT nested_transaction' );
$this->driver->query( 'UPDATE t SET value = 2 WHERE id = 1' );
$this->driver->query( 'RELEASE SAVEPOINT nested_transaction' );

$this->assertTrue( $this->driver->inTransaction() );
$this->driver->query( 'ROLLBACK' );
$this->assertFalse( $this->driver->inTransaction() );
$this->assertSame( '1', $this->driver->query( 'SELECT value FROM t' )->fetchColumn() );
}

public function test_write_inside_savepoint_can_be_rolled_back(): void {
$this->driver->query( 'CREATE TABLE t (id INT PRIMARY KEY, value INT)' );
$this->driver->query( 'INSERT INTO t VALUES (1, 1)' );

$this->driver->query( 'SAVEPOINT outer_transaction' );
$this->driver->query( 'UPDATE t SET value = 2 WHERE id = 1' );
$this->driver->query( 'ROLLBACK TO SAVEPOINT outer_transaction' );
$this->driver->query( 'RELEASE SAVEPOINT outer_transaction' );

$this->assertFalse( $this->driver->inTransaction() );
$this->assertSame( '1', $this->driver->query( 'SELECT value FROM t' )->fetchColumn() );
}

public function test_writes_inside_nested_savepoints_preserve_outer_changes(): void {
$this->driver->query( 'CREATE TABLE t (id INT PRIMARY KEY, value INT)' );
$this->driver->query( 'INSERT INTO t VALUES (1, 1)' );

$this->driver->query( 'SAVEPOINT outer_transaction' );
$this->driver->query( 'UPDATE t SET value = 2 WHERE id = 1' );
$this->driver->query( 'SAVEPOINT inner_transaction' );
$this->driver->query( 'UPDATE t SET value = 3 WHERE id = 1' );
$this->driver->query( 'ROLLBACK TO SAVEPOINT inner_transaction' );
$this->driver->query( 'RELEASE SAVEPOINT inner_transaction' );

$this->assertTrue( $this->driver->inTransaction() );
$this->assertSame( '2', $this->driver->query( 'SELECT value FROM t' )->fetchColumn() );

$this->driver->query( 'RELEASE SAVEPOINT outer_transaction' );

$this->assertFalse( $this->driver->inTransaction() );
$this->assertSame( '2', $this->driver->query( 'SELECT value FROM t' )->fetchColumn() );
}

public function test_duplicate_savepoint_names_track_innermost_scope(): void {
$this->driver->query( 'CREATE TABLE t (id INT PRIMARY KEY, value INT)' );
$this->driver->query( 'INSERT INTO t VALUES (1, 1)' );

$this->driver->query( 'SAVEPOINT repeated' );
$this->driver->query( 'UPDATE t SET value = 2 WHERE id = 1' );
$this->driver->query( 'SAVEPOINT repeated' );
$this->driver->query( 'UPDATE t SET value = 3 WHERE id = 1' );
$this->driver->query( 'ROLLBACK TO SAVEPOINT repeated' );
$this->driver->query( 'RELEASE SAVEPOINT repeated' );

$this->assertTrue( $this->driver->inTransaction() );
$this->assertSame( '2', $this->driver->query( 'SELECT value FROM t' )->fetchColumn() );

$this->driver->query( 'RELEASE SAVEPOINT repeated' );

$this->assertFalse( $this->driver->inTransaction() );
$this->assertSame( '2', $this->driver->query( 'SELECT value FROM t' )->fetchColumn() );
}

public function test_quoted_savepoint_names_are_case_insensitive(): void {
$this->driver->query( 'CREATE TABLE t (id INT PRIMARY KEY, value INT)' );
$this->driver->query( 'INSERT INTO t VALUES (1, 1)' );

$this->driver->query( 'SAVEPOINT `MixedCase`' );
$this->driver->query( 'UPDATE t SET value = 2 WHERE id = 1' );
$this->driver->query( 'ROLLBACK TO SAVEPOINT `mixedcase`' );
$this->driver->query( 'RELEASE SAVEPOINT `MIXEDCASE`' );

$this->assertFalse( $this->driver->inTransaction() );
$this->assertSame( '1', $this->driver->query( 'SELECT value FROM t' )->fetchColumn() );
}

public function test_failed_write_cleans_up_savepoint_transaction_state(): void {
$this->driver->query( 'CREATE TABLE t (id INT PRIMARY KEY)' );
$this->driver->query( 'INSERT INTO t VALUES (1)' );
$this->driver->query( 'SAVEPOINT outer_transaction' );

try {
$this->driver->query( 'INSERT INTO t VALUES (1)' );
$this->fail( 'Expected the duplicate insert to fail.' );
} catch ( PDOException $e ) {
$this->assertStringContainsString( 'UNIQUE constraint failed', $e->getMessage() );
}

$this->assertFalse( $this->driver->inTransaction() );
$this->assertSame( '1', $this->driver->query( 'SELECT COUNT(*) FROM t' )->fetchColumn() );

$this->driver->query( 'INSERT INTO t VALUES (2)' );
$queries = array_column( $this->driver->get_last_sqlite_queries(), 'sql' );
$this->assertSame( 'BEGIN IMMEDIATE', $queries[0] );
$this->assertSame( 'COMMIT', end( $queries ) );
}

public function test_fetch_default(): void {
// Default fetch mode is PDO::FETCH_BOTH.
$result = $this->driver->query( "SELECT 1, 'abc', 2" );
Expand Down
44 changes: 44 additions & 0 deletions tests/phpunit/WP_SQLite_Database_Integration_Savepoint_Test.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
<?php

class WP_SQLite_Database_Integration_Savepoint_Test extends PHPUnit_Adapter_TestCase {

public function test_wpdb_update_inside_savepoint_does_not_open_nested_transaction() {
global $wpdb;

$option_name = 'sqlite_savepoint_write_test';
$wpdb->insert(
$wpdb->options,
array(
'option_name' => $option_name,
'option_value' => '1',
'autoload' => 'no',
)
);

try {
$this->assertFalse( $wpdb->get_driver()->inTransaction() );
$this->assertNotFalse( $wpdb->query( 'SAVEPOINT wpdb_update' ) );

$result = $wpdb->update( $wpdb->options, array( 'option_value' => '2' ), array( 'option_name' => $option_name ) );
$queries = array_column( $wpdb->get_driver()->get_last_sqlite_queries(), 'sql' );

$this->assertSame( 1, $result );
$this->assertSame( 1, $wpdb->rows_affected );
$this->assertSame( '', $wpdb->last_error );
$this->assertNotContains( 'BEGIN IMMEDIATE', $queries );
$this->assertNotFalse( $wpdb->query( 'RELEASE SAVEPOINT wpdb_update' ) );
$this->assertFalse( $wpdb->get_driver()->inTransaction() );
$this->assertSame(
'2',
$wpdb->get_var(
$wpdb->prepare( 'SELECT option_value FROM %i WHERE option_name = %s', $wpdb->options, $option_name )
)
);
} finally {
if ( $wpdb->get_driver()->inTransaction() ) {
$wpdb->query( 'ROLLBACK' );
}
$wpdb->delete( $wpdb->options, array( 'option_name' => $option_name ) );
}
}
}
Loading