ap / DBIx-Connector

Fast, safe DBI connection and transaction management
https://metacpan.org/release/DBIx-Connector
40 stars 14 forks source link

transaction as subroutine appears to be unworkable in my case #24

Open xenoterracide opened 11 years ago

xenoterracide commented 11 years ago

I'm trying to write a Unit of Work object http://martinfowler.com/eaaCatalog/unitOfWork.html with DBIx::Connector as the underlying connector, unfortunately as I look at it I'm unable to resolve any way that I can use Connectors transaction support using this pattern.

The reason for this is that my begin needs to start in a different method from commit, and the only way I have seen to resolve this is to move it to a higher level, but I don't like that because that puts database specific code at a layer higher than I want.

would you add something like DBIC's txn guard? expose support for DBI's native transaction support? or maybe add callbacks that are more like AnyEvent::DBI's (considering switching to that)

theory commented 11 years ago

I wouldn't turn down a patch that offered a lightweight implementation of such a thing. I don't think it would need to be very complicated. I'd like to see what it looked like, though.

xenoterracide commented 11 years ago

well my thought would be just to use DBI's functionality, but glancing over your code you are using the underlying DBD's directly and I'm not sure why. Why is DBI's own transaction interface insufficient?

the more I'm looking over it, I'm thinking the simple solution of exposing DBI's methods would work.

$rc  = $dbh->begin_work;
$rc  = $dbh->commit;
$rc  = $dbh->rollback;
theory commented 11 years ago

FWIW, setting AutoCommit is the DBI's interface. The transaction methods you mention were added later.

But to answer your question, the direct hash stuff is used so that things can be locally scoped.

xenoterracide commented 11 years ago

they may have been added later but they're still useful, the thing is I'll have to turn autocommit off and manually handle the transactions. I think begin_work may be optional, but it seems like a good idea to use.

theory commented 11 years ago

It is but sometimes it's not the right choice. If you were to switch DBIx::Connector over to them, I'm pretty sure the tests will fail. The ability to use local is crucial to allowing nested calls to txn and friends.

xenoterracide commented 11 years ago

I'm getting the feeling that allowing an interface that allows me to have an interface similar to ->begin ->commit isn't going to work. I don't really care if it passes through or works in a completely different way... it's just the problem of having readers operate outside of writers but needing them to happen within a single (or nested) transaction to have actual ACID-ity. I'll look into seeing if it can be made to work this weekend (if tuits), since DBIC has it, and your code I belive copies theirs it should be possible.

theory commented 11 years ago

It can be done. DBIc implements txn_do() with its transaction guard stuff. The thing to do would be to add the guard interface to Connector without regard to txn(), at first. Then modify txn() to use the guard interface.

xenoterracide commented 11 years ago

out of curiousity? because I've never really turned autocommit off... but it appears by DBI's doc if it's off you have to call commit. How does ::Connector handle autocommit off now?

theory commented 11 years ago

Like this -- yes, using begin_work, commit, and rollback. Looks like I forgot that DBIx::Connector does not set AutoCommit. It just checks it to see if it's in a transaction or not (begin_work turns it off, commit and rollback turn it on).