Allow commit() without transaction?

php.internals

Nikita Popov

5 years ago
Hi internals, Since PHP 8.0, PDO fully supports MySQL transactions -- this means that inTransaction() will report the actual connection transaction state, as opposed to an emulated (incorrect) transaction state tracked by PDO itself. This has two primary effects: PDO is aware of transactions created without going through it's APIs (e.g. by manually issuing a START TRANSACTION query) and is aware of implicit commits (see https://dev.mysql.com/doc/refman/8.0/en/implicit-commit.html for more information). The latter means that code like $pdo->beginTransaction() $pdo->exec('DROP TABLE foobar'); // Implicitly commits transaction $pdo->exec('DROP TABLE barfoo'); // Transaction not active anymore $pdo->commit(); // Throws because no transaction active now throws an exception, because commit() is called without an active transaction. It's possible to use if ($pdo->inTransaction()) $pdo->commit(); if you don't care about the lack of active transaction. I believe this behavior is correct, but I could see an argument made in favor of always allowing commit() calls (but not rollBack() calls) even if there is no active transaction. That would change the meaning of commit() from "commit an active transaction" towards "commit if an active transaction exists". Any opinions on that? Regards, Nikita

Matteo Beccati

5 years ago
Hi Nikita, On 12/04/2021 14:07, Nikita Popov wrote:
> $pdo->beginTransaction() > $pdo->exec('DROP TABLE foobar'); // Implicitly commits transaction > $pdo->exec('DROP TABLE barfoo'); // Transaction not active anymore > $pdo->commit(); // Throws because no transaction active > > now throws an exception, because commit() is called without an active > transaction. It's possible to use if ($pdo->inTransaction()) > $pdo->commit(); if you don't care about the lack of active transaction. > > I believe this behavior is correct, but I could see an argument made in > favor of always allowing commit() calls (but not rollBack() calls) even if > there is no active transaction. That would change the meaning of commit() > from "commit an active transaction" towards "commit if an active > transaction exists". > > Any opinions on that?
I agree. As a user I'd prefer to be alerted that the transaction was not actually active rather than relying on seemingly working code that doesn't do what's on the tin. In fact I probably never knew about this particular MySQL quirk and most likely I had been fooled many times into thinking that the code above was a single transaction. Cheers
-- Matteo Beccati Development & Consulting - http://www.beccati.com/

Dik Takken

5 years ago
On 12-04-2021 14:07, Nikita Popov wrote:
> I believe this behavior is correct, but I could see an argument made in > favor of always allowing commit() calls (but not rollBack() calls) even if > there is no active transaction. That would change the meaning of commit() > from "commit an active transaction" towards "commit if an active > transaction exists". > > Any opinions on that? >
I think it is really nice that commit() throws for inactive transactions. Code paths that mess up your transactions will not go unnoticed that easily. My preference would be to have a separate commit method or a method parameter to explicitly allow committing without an active transaction. Regards, Dik Takken

Kamil Tekiela

5 years ago
What would be a benefit of a commit function that can be executed without an active transaction?

Benjamin Morel

5 years ago
> > > I think it is really nice that commit() throws for inactive > transactions. Code paths that mess up your transactions will not go > unnoticed that easily. >
+1. I've always found this behaviour in MySQL very surprising and error prone (BEGIN; BEGIN; COMMIT; COMMIT; => no errors), and I'm actually very happy to see this solved at PDO level. IMO if someone relies on commit() not throwing if there is no active transaction, they should be the one doing the extra check: if ($pdo->inTransaction()) $pdo->commit(). — Benjamin

David Gebler

5 years ago
Good example of why commit() should throw an exception, I think - it's just alerted the developer that executing their previous statement has caused an implicit commit. Imagine if they had a complex sequence of queries and didn't realise they'd added one which ended the transaction, thinking they could safely rollback later if necessary. The exception catches this early on. Regards, David Gebler On Mon, Apr 12, 2021 at 1:07 PM Nikita Popov <nikita.ppv@gmail.com> wrote: