So I realized earlier tonight that while the documentation for transactions says to call $txn->rollback() to kill the active transaction, that method does not in fact exist. You need to call ->rollBack() (different case?) on the connection object, which you generally don't get access to anyway. How we let that through I do not know. Quite embarrassing.
The attached patch adds the missing method, which just passes through to the connection object, and standardizes on rollback() everywhere. That is, after this patch the code now does what the documentation says it should.
Comment | File | Size | Author |
---|---|---|---|
#6 | rollback.patch | 652 bytes | aaaristo |
#2 | rollback.patch | 2.78 KB | Crell |
rollback.patch | 1.96 KB | Crell | |
Comments
Comment #1
David StrausswillRollBack should also become willRollback.
Comment #2
Crell CreditAttribution: Crell commentedYes it should. It should also get its own wrapping method, just like rollback().
Comment #3
David StraussRTBC
Comment #4
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD. Thanks.
Comment #5
aaaristo CreditAttribution: aaaristo commentedThere is a problem with the renaming of willRollBack to willRollback:
the popTransaction still use willRollBack not willRollback so it does not call parent::rollBack() if supportsTransactions() is TRUE....
i'am not sure why you renamed it..
Comment #6
aaaristo CreditAttribution: aaaristo commentedI'm attaching a patch tested with Database -> Transaction tests on oracle with supportTransactions() == true.
Comment #7
Damien Tournoud CreditAttribution: Damien Tournoud commentedSince when are PHP methods (EDIT: and variables) case sensitive? ;)
Not nice, but not critical.
Comment #8
aaaristo CreditAttribution: aaaristo commentedOk, posted the patch, only needs review..
I'am a newbie (be patient) but transactions are not rolled back actually without this fix (on databases that supports transactions), is it non critical?
Comment #9
Crell CreditAttribution: Crell commentedEither way, we should be consistent. +1 to #6. Thanks, Andrea.
Comment #10
David StraussRTBC for #6
Comment #11
Crell CreditAttribution: Crell commentedTagging to get more attention.
Comment #12
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD. Thanks.