Skip to content

Conversation

@ndossche
Copy link
Member

Instead of tracking the transaction state internally, implement the internal PDO getter to check with the sqlite library itself.
Fixes https://bugs.php.net/bug.php?id=81227

Applying to master only because this bugfix is a behaviour change.

Copy link
Member

@SakiTakamachi SakiTakamachi left a comment

Choose a reason for hiding this comment

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

LGTM, there's only one nits, but it makes sense to me :)

Comment on lines 285 to 293
static bool pdo_sqlite_in_transaction(pdo_dbh_t *dbh)
{
pdo_sqlite_db_handle* H = (pdo_sqlite_db_handle*) dbh->driver_data;
return H->db && sqlite3_get_autocommit(H->db) == 0;
}

Copy link
Member

@SakiTakamachi SakiTakamachi May 19, 2024

Choose a reason for hiding this comment

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

  • SQLite3 does not allow to explicitly turn off autocommit mode other than manually starting a transaction
  • Manual transactions always exist when autocommit mode is off

Since this kind of behavior is specific to SQLite, it might be better to leave a comment.

Copy link
Member

@Girgias Girgias left a comment

Choose a reason for hiding this comment

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

This makes sense to me

@ndossche ndossche force-pushed the pdo-sqlite-transaction branch 2 times, most recently from 868e69c to 492aa9e Compare May 20, 2024 11:15
@ndossche ndossche merged commit b7bf846 into php:master May 20, 2024
adamziel added a commit to WordPress/sqlite-database-integration that referenced this pull request Dec 18, 2025
Extracting smaller chunks from
#291 so we
can review and merge them independently.

This PR includes the following changes:
- Removed support for nested transactions. MySQL doesn't support them.
- Implemented PDO-like APIs for start transaction, commit, and rollback.
- The PDO-like APIs differ from an equivalent SQL command in that they
always throw an exception in scenarios when a transaction was already
started or not yet started (depending on the method), irrespective of
the `ATTR_ERRMODE` setting.
- Polyfill `PDO::inTransaction()` on PHP < 8.4, where it is not reliable
([issue](https://bugs.php.net/bug.php?id=81227),
[PR](php/php-src#14268)).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants