summary:
http://drawohara.tumblr.com/post/12411960
i am the only one thinking along these terms?
is there some rationale i'm missing behind the current behaviour?
kind regards.
Im not able to reproduce your first example:
ActiveRecord::Base.connection.transaction do Model.create! raise end # in this case the Model creation is NOT rolled back
I can't remember ever running into a situation where that would get committed. Even if a new transaction isn't opened for that block because one was already open, the exception still raises to the outer transaction which should cause the rollback. Maybe I'm wrong.
But anyway, I have attempted to simplify working with transactions myself. I started with this:
http://code.stat.im/repos/plugins/super_transaction/trunk/lib/super_transaction.rb (I recommend against using it/dont use it)
But I thought it was overkill and ended up running into some issues with it. Im now using this code (haven't pluginized it yet):
Its very straight forward and doesn't hack anything up in core. The basic idea is to simplify exception handling so it doesn't need to be duplicated everywhere throughout the application. It also allows for calling #save instead of #save! if you dont want to die on the first failure (validation errors get populated on all attempted #save's).
I think you've got some cool stuff there and are addressing some different issues than I am, but the first example you gave did get me worried. Either way I agree transactions and exception handling in transactions could definitely be improved.
Joe
summary:
http://drawohara.tumblr.com/post/12411960
i am the only one thinking along these terms?
is there some rationale i'm missing behind the current behaviour?
ActiveRecord::Base.connection.transaction do Model.create! raise end # in this case the Model creation is NOT rolled back
What do you mean by 'rolling back' model creation. The transaction
should definitely be aborted. The database should be left as it was
before. If not, you've found a bug for which test cases would be
greatly appreciated 
However the particular model instance probably still thinks that its been saved successfully, @model.new_record? probably returns false, etc. Is that what you mean?
hey koz-
note that i *am* abusing the api a *little* (see commented line)
here are two samples:
sqlite.sh (818 Bytes)
postgresql.sh (1.03 KB)
I think you are correct in assuming this is your problem. bits of create are wrapped in a transaction (eg so that the callbacks either run or don't etc... sqlite doesn't allow nested transaction, by using ActiveRecord::Base.connection.transaction (ie fiddling at the adapter level), you're starting the transaction, but not letting rails know about it. If you use ActiveRecord::Base.transaction then rails counts the transactions opened (so when it's about to do a begin inside create it can see it's already in a transaction and thus omit the actual begin)
The exception you get isn't your raise - it's the sqlite3 library complaining about nested transactions. I don't know sqlite3 much, but say in mysql then 'BEGIN' is one of those magic things that automatically commits the current transaction, which would explain what you see.
In postgres land the path which gets you there is different: the first begin executes, rails doesn't know about it. Then you get the second begin (from create), which (as per postgresql docs) is a no-op (but note the warning you get): you don't actually get the nested transaction. rails reaches the end of the transaction it was trying to handle in create, since it's not aware of anything else it sends commit back to the database: your raise causes no actual rollback (you can see rails tried from the warning about there being no transaction in progress) because it's too late: the creation of Foo has already committed.
Anyway, what I was wondering is why not use ActiveRecord::Base.transaction ?
Fred
I think you are correct in assuming this is your problem. bits of create are wrapped in a transaction (eg so that the callbacks either run or don't etc... sqlite doesn't allow nested transaction, by using ActiveRecord::Base.connection.transaction (ie fiddling at the adapter level), you're starting the transaction, but not letting rails know about it.
then that method should not be exposed. it should be private. we always have
execute 'BEGIN'
after all. the fact that it's exposed and takes a block means it should work for us.
If you use ActiveRecord::Base.transaction then rails counts the transactions opened (so when it's about to do a begin inside create it can see it's already in a transaction and thus omit the actual begin)
The exception you get isn't your raise - it's the sqlite3 library complaining about nested transactions. I don't know sqlite3 much, but say in mysql then 'BEGIN' is one of those magic things that automatically commits the current transaction, which would explain what you see.
right. the fact is that it's brain dead *easy* for rails to avoid this issue with all dbs at once.
In postgres land the path which gets you there is different: the first begin executes, rails doesn't know about it.
but it should *unless* i did something sneaky 'like execute BEGIN'
Then you get the second begin (from create), which (as per postgresql docs) is a no-op (but note the warning you get): you don't actually get the nested transaction. rails reaches the end of the transaction it was trying to handle in create, since it's not aware of anything else it sends commit back to the database:
yeah, but again, it *should* be aware of it since it's so easy to do.
your raise causes no actual rollback (you can see rails tried from the warning about there being no transaction in progress) because it's too late: the creation of Foo has already committed.
yup. the fact is that the multiple routes to get at transactions do not play well together.
Anyway, what I was wondering is why not use ActiveRecord::Base.transaction ?
nested transactions are only *one* issue sane_transactions.rb addresses, the others are
- a throw/catch based mechanism for commit/rollback. in the current rails there is no way, that i know of, of committing a transaction other than falling through to the end of the block. this means all logic must be contained in a single block, rather than split out into methods, to commit early if logic dictates. you cannot raise because this rolls back the exception.
- the current mechanism is using raise/exceptions to rollback doesn't seem to be good form: as using exceptions for control flow never is. a throw/catch mechanism allows the user to rollback a pending transaction without ALSO have to deal with catching the error and avoids all the risks of accidentally swallowing a real error (not one used for control flow)
- transactions and savepoints need to be integrated. sane_transactions.rb does this do in a way that connection adapters can utilize, ignore, or extend.
this, and the nested transacitons work across the board in a POLS way regardless of the entry point. the main benefit i see, however, is that the implementation is also vastly simpler with far less state tracking - this is mostly due to the use of throw/catch.
anyhow, i hope it's food for thought.
kind regards.
I think you are correct in assuming this is your problem. bits of create are wrapped in a transaction (eg so that the callbacks either run or don't etc... sqlite doesn't allow nested transaction, by using ActiveRecord::Base.connection.transaction (ie fiddling at the adapter level), you're starting the transaction, but not letting rails know about it.
then that method should not be exposed. it should be private. we always have
execute 'BEGIN'
after all. the fact that it's exposed and takes a block means it should work for us.
Hmm. If the adaptor methods were private the ActiveRecord itself couldn't call them. In general, adaptor methods are sort of down their and grungy. If you don't use the adaptor methods then the nesting and so on just works
yup. the fact is that the multiple routes to get at transactions do not play well together.
definitely the underlying issue.
Anyway, what I was wondering is why not use ActiveRecord::Base.transaction ?
nested transactions are only *one* issue sane_transactions.rb addresses, the others are
- a throw/catch based mechanism for commit/rollback. in the current rails there is no way, that i know of, of committing a transaction other than falling through to the end of the block. this means all logic must be contained in a single block, rather than split out into methods, to commit early if logic dictates. you cannot raise because this rolls back the exception.
well if you use break then you exit the block and the transaction is committed. that's not very nice though 'break' hardly sounds like it should mean commit.
- the current mechanism is using raise/exceptions to rollback doesn't seem to be good form: as using exceptions for control flow never is.
I think it depends on how you view it: if the code inside the block raises exceptions to say 'help it's all screwed' then the current stuff is fine: it's not using exceptions to do control flow, it's using exceptions to deal with the fact that something horrible has happened.
I've tended to use transactions so that should some random thing go wrong then the database isn't inconsistent: the only case in which I rollback is if an exception is thrown. However if you are going to be saying commit! and rollback! as part of your actual logic then it makes perfect sense to make them proper things (sorry can't think of a better word)
- transactions and savepoints need to be integrated. sane_transactions.rb does this do in a way that connection adapters can utilize, ignore, or extend.
Coolio!
this, and the nested transacitons work across the board in a POLS way regardless of the entry point. the main benefit i see, however, is that the implementation is also vastly simpler with far less state tracking - this is mostly due to the use of throw/catch.
Apart from ActiveRecord::Base.connection.begin_db_transaction for some adapters (eg the mysql/postgres adapter implementation just calls execute 'BEGIN') (and of course execute 'BEGIN'). That's sort of adaptor dependant I suppose.
Fred
Hmm. If the adaptor methods were private the ActiveRecord itself couldn't call them. In general, adaptor methods are sort of down their and grungy. If you don't use the adaptor methods then the nesting and so on just works
that is a good point. maybe a doc issue? we end up using the connection a fair bit since we're dealing with postgresql and some tricky sql and - obivously - that's what initially bit me.
it's still hard to say what this is good for:
ActiveRecord::Base.connection.transaction &block
since very little will work in block. ceratinly begin_db_transaction and end_db_transaction are required. but of what value is transaction with a block on the connection object?
well if you use break then you exit the block and the transaction is committed. that's not very nice though 'break' hardly sounds like it should mean commit.
exactly. break != commit and raise != rollback semantically.
I think it depends on how you view it: if the code inside the block raises exceptions to say 'help it's all screwed' then the current stuff is fine: it's not using exceptions to do control flow, it's using exceptions to deal with the fact that something horrible has happened.
oh right. there are two cases
- something went wrong. rollback. this is fine now.
- nothing went wrong in the code, but has logically, rollback with no errors or warnings. that is not supported at all now and, imho, is a rather common scenario.
I've tended to use transactions so that should some random thing go wrong then the database isn't inconsistent: the only case in which I rollback is if an exception is thrown. However if you are going to be saying commit! and rollback! as part of your actual logic then it makes perfect sense to make them proper things (sorry can't think of a better word)
yeah that's my point alright. there just isn't satisfying hooks for it now.
- transactions and savepoints need to be integrated. sane_transactions.rb does this do in a way that connection adapters can utilize, ignore, or extend.
Coolio!
yeah it's handy - we're using it now. i'm not the first one to patch this in either - there are some things on trac that do the same.
Apart from ActiveRecord::Base.connection.begin_db_transaction for some adapters (eg the mysql/postgres adapter implementation just calls execute 'BEGIN') (and of course execute 'BEGIN'). That's sort of adaptor dependant I suppose.
yeah - it's certainly sufficient though. maybe that's where it belongs? don't offer the user ruby ways to hose themselves - if they are writing sql they deserve it!
cheers.
summary:
http://drawohara.tumblr.com/post/12411960
i am the only one thinking along these terms?
Please see http://dev.rubyonrails.org/ticket/5457 and search Trac for 'nested transaction' or 'savepoint' for others.
is there some rationale i'm missing behind the current behaviour?
Yes, though you've blanketed the possibility with "the standard activerecord transactions are not sane."
The current behavior -- flattening successive 'I require a transaction' declarations in the app into a single database transaction -- is precisely what most developers writing most web apps need.
Explicitly nesting transactions or emulating them with a proliferation of savepoints are sexy features, but do they make sense at the application level? Does their exposure increase programmer happiness or their blood pressure?
I hope you contribute your work to #5457. Incremental, fully-tested
changes to the existing API are most welcome. The underlying features
your propose are solid and needn't be roaming the countryside tilting
at sanitoriums
jeremy