Transaction abstraction will allow use of nested transactions without risk of accidentally running an implicit COMMIT on an existing transaction.
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | transactions_1.patch | 3.24 KB | david strauss |
| #6 | transactions_0.patch | 3.23 KB | david strauss |
| #2 | transactions.patch | 1.48 KB | david strauss |
Comments
Comment #1
david straussHow would people feel about adopting the semantics of PressFlow Transaction as the abstraction layer? For PHP 4, it could be a dummy object that doesn't run transactions. So, functional transactions would requires PHP 5 + MySQL with InnoDB or PHP 5 + PostgreSQL. Anything less would still run, just without transactions.
The semantics of PressFlow transaction are particularly elegant because destructors handle COMMITs and ROLLBACKs. It prevents stray transactions from lingering.
Comment #2
david straussHere's a rough patch versus HEAD. I'm trying to be conservatively compatible with PHP 4, at least ot avoid errors.
Comment #3
david straussBy PHP 4 "compatible," I mean that functions using this object shouldn't cause errors in PHP 4.
Comment #4
killes@www.drop.org commentedCould you use a more Drupal-like syntax? Would mke it easier to evaluate its usefullness.
Comment #5
robertdouglass commented@killes: by "Drupal-like" do you mean "no class, please"?
@David: sorry to not address your main question, but you might want to clean up your code style so that people (like me!) don't harp on it. This construct, in particular, always hurts my eyes:
Comment #6
david straussHere's an updated patch. If this gets committed, I'll create a new issue that adds support to the rest of Drupal core for using these transactions. The ugliness of the code is largely a result of complying with PHP 4 parser rules so this class doesn't create an error.
Running a function's operations (and all encapsulated operations) in a transaction simply requires adding the following to the top of the function:
When $txn leaves scope, PHP takes care of the ending the transaction.
Using classes isn't typical Drupal style. Here's why we should make an exception:
* A class provides nice encapsulation for detecting nested transactions. Once Drupal drops PHP 4 support, the current global variables will become static class members. Using a class today prevents unnecessary API changes in the future.
* Exploiting object scope to end transactions prevents runaway transactions from happening. If we require a call to db_transaction_commit() or something similar, some code, somewhere will forget to call it and cause the most confusing issues you've ever debugged. (Changes will appear to take effect while the page runs, but nothing will permanently get saved to the database.)
* Using object scope as the transaction controller encourages good programming style. Instead of starting and ending transactions all over a function, developers will create new functions that encapsulate the transactional operations. This is how critical sections are isolated in Java, and it's very easy to read and debug.
Comment #7
david straussFixed some comments.
Comment #8
david straussComment #9
david straussWhy we should accept degraded support for PHP 4:
* The support degrades gracefully. Drupal doesn't use transactions right now, and the degraded PHP 4 behavior of this patch just continues using no transactions.
* We can't use object scope as the transaction model and support PHP 4.
* We're trying to encourage users to move to PHP 5. Transaction support provides a good reason for user to consider the switch.
To further add to the list in #6:
* Using object scope allows functions to return values more easily. If we require a call to something like db_transaction_commit(), functions will have to store a DB result to a temporary variable, end the transaction, and then return the temporary variable.
Comment #10
moshe weitzman commentedcould we include one instance of this feature in the patch, and some instructions for testing. thanks.
Comment #11
catchIt's a small patch and still applies with offset, but marking to needs work per Moshe's comments that it needs a use-case included.
Comment #12
Crell commented@DavidStrauss: The problem I see is that MySQL can be a mix of InnoDB and MyISAM tables, so in order to be transaction-capable a given set of query would have to touch only InnoDB tables. Is there an easy and fast way to detect that, or would we need to have users just flag their database connection transaction safe?
I can see getting this into the Drupal 7 database API overhaul, but we need to make sure it's done in a way that it will fail gracefully. PDO, unfortunately, throws a warning if you try to start a transaction when you are unable to do so. (An exception would make it easier to handle gracefully, but c'est la vie.) We'd need some simple value we could if-check, even if it's an explicit flag as part of the database settings.
(I'm OK with doing it using a self-destructing class, but we would need to be able to check in the constructor if it's even possible, and simply return if it's not.)
Comment #13
Crell commentedA self-destructing transaction object has been folded into the Database TNG patch: http://drupal.org/node/225450