ActiveRecord should use savepoints for nested transactions

Jeremy, the plugin by Jonathan Viney actually does make transaction{} only create a nested transaction if an explicit ":force => true" option is passed to it. The only exception being transactional tests where one additional level of non-explicit transactions can be created (because the test class itself uses the first level).

So these changes should not change rails behavior outside tests.

This plugin however does not include the ability to configure rails behavior for implicit transactions (those automatically created by save/destroy). Which means that you will not be able to make some activerecord model always create an implicit transaction for saving itself, even if a higher level transaction already exists. But this is a capability not strictly neccessary for the nested transactions feature to be useful, as you can always create an explicit "transaction :force => true" block around a save if you need it.

I've attached a patch for master.

The handling of increment/decrement_open_transactions has been moved to the connection object rather than handling it in AR::Base.transaction, seemed better to me.

I've added a transactional_fixtures accessor to the connection which is set by the fixtures. Not the nicest, but I wasn't sure what else to do.

There are some failures for databases that don't support savepoints (eg. sqlite). These are a bit hard to avoid unless we add a supports_savepoints? to the connection.

Anyway, this should get the ball rolling.

I think ':force => true' is a bit confusing. How about ':nested => true'?

savepoints-3.diff doesn't apply cleanly om master. Here's an updated patch, with conflicts resolved.

It doesn't pass the unit tests however. More investigation will be required.

Jonathan, is there a reason why you changed

def transaction(start_db_transaction = false)

to

def transaction(start_db_transaction = false)

?

Never mind my last message, I already figured it out. :)

Here's an updated patch which passes all unit tests, for MySQL, PostgreSQL and SQLite3. Changes are:

• Much improved documentation, including documentation about potential caveats on MySQL.
• :force option renamed to :nest, which is probably clearer.
• I had to revert commit 045713ee240fff815edb5962b25d668512649478 by Jeremy Kemper, "PostgreSQL: introduce transaction_active? rather than tracking activity ourselves". It conflicts with the savepoint code.

After speaking with Koz I've reimplemented Jeremy's transaction state introspection feature. The old implementation was incompatible with the old 'postgres' driver, which I fixed. I also added unit tests for this feature.

Koz said that it's probably not feasible to include this feature in 2.2, seeing that we're in a feature freeze. If anybody wants to help, I'm maintaining this savepoint stuff in the following git repository:

git://git.phusion.nl/rails.git branch 'savepoints'

Is git://git.phusion.nl/rails.git still available?

Yes it is. I've just merged the patches against latest edge, and all unit tests pass.

$ git remote add phusion git://git.phusion.nl/rails.git
$ git fetch phusion
fatal: The remote end hung up unexpectedly

Oops, a little git-daemon configuration problem, sorry. It should be up now.

     def transaction(start_db_transaction = false)
       start_db_transaction ||= open_transactions == 0 || (open_transactions == 1 && transactional_fixtures)

This is an unexpected change of behavior: your transactions are nested in tests by default, but not in development or production without :nest => true.

There should be a clearer distinction between wanting to participate in a transaction and requiring a new transaction boundary.

To keep Base#transaction compatible with existing apps, we should :nest => true by default but disable for save/destroy callback transactions.

(from [ab0ce052ba23a4cce7a84ecade0d00d9cc518ebd]) Introduce transaction_joinable flag to mark that the fixtures transaction can't joined, a new savepoint is required even if :requires_new is not set. Use :requires_new option instead of :nest. Update changelog.

[#383 ActiveRecord should use savepoints for nested transactions state:committed] http://github.com/rails/rails/co...

Can we ensure there's a way to easily debug issues one has when using the feature. I'm thinking of setting out the development database logs with the SQL trace in a fashion where it's easy to see what: (a) transaction it's part of (b) what save point section it's part of (if that makes sense)