Hi all
I think it would be extremely useful to allow nested database transactions - or simulate them using a counter that would only to the actual commit after commit() has been called as many times as begin() was called before.
This actually used to be the case, according to the comment on Database::trxLevel:
* Historically, transactions were allowed to be "nested". This is no * longer supported, so this function really only returns a boolean.
This means that currently, if you call begin() while a transaction is already in progress, the previous transaction is inadvertently committed, possibly causing inconsistencies (at least on MySQL).
Why was this feature removed? Not counting transaction levels is causing a world of pain for us on the Wikidata project, and I'm sure the same problem arises elsewhere. Here's the problem:
* Before saving a change using WikiPage::doEdit(), i want to perform some checks on the database, enforcing some global consistency constraints. * The check should be in the same transaction, so the DB can't change after the check but before the save. * I can't open a transaction before my check, because WikiPage::doEdit()already opens a transaction which would in turn abort mine, causing the save to be performed in a separate transaction after all. * I could try to inject my check into WikiPage::doEdit() using some hook. That may work but it cumbersome and annoying, and I have to hope that the hook is never moved outside the transaction (hooks generally don't guarantee whther they are called in a transaction or not).
Basically, any code that needs transactional logic needs to first check whether a transaction is already ongoing, open a transaction if not, remember whether it owns the transaction, and commit the transaction in the end only if it's owned locally. This essentially implements the is-in-transaction-counter based on the call stack.
Looking at the code, this kind of check is hardly ever done. So the code just *assumes* that it is, or is not, called within a transaction. This is bad.
So... why was the nice and simple trxLevel counting removed? What would break if we put it back? Is there some other magic method to do this safely and nicely?
Thanks, Daniel
PS: btw, if the non-counting behavior is to be kept, Database::begin() should really fail if a transaction is already in progress. Silently committing the previous transaction is very likely to cause creeping, low volume and hard to track down database corruption.