[Dbix-class] ROLLBACK seems to be skipped on 0.08

[Brandon Black]

On 10/20/07, Jesper Krogh <jesper at krogh.cc> wrote:
> Matt S Trout wrote:
> > On Fri, Oct 19, 2007 at 10:28:38AM +0200, Jesper Krogh wrote:
> >> AutoCommit is 0 .. you can see it above, thats by intention.
> >
> > AutoCommit 0 has been considered undefined for DBIC for some time.
>
> I've failed to notice that.
>

>From the Storage::DBI docs, these are the two relevant stanzas:

-------------------

Please note that the DBI docs recommend that you always explicitly set
AutoCommit to either 0 or 1. DBIx::Class further recommends that it be
set to 1, and that you perform transactions via our "txn_do" method.
DBIx::Class will set it to 1 if you do not do explicitly set it to
zero. This is the default for most DBDs. See below for more details.

---- later in the pod ----

Another Important Note:

DBIC can do some wonderful magic with handling exceptions,
disconnections, and transactions when you use AutoCommit =&gt; 1
combined with txn_do for transaction support.

If you set AutoCommit =&gt; 0 in your connect info, then you are
always in an assumed transaction between commits, and you're telling
us you'd like to manage that manually. A lot of DBIC's magic
protections go away. We can't protect you from exceptions due to
database disconnects because we don't know anything about how to
restart your transactions. You're on your own for handling all sorts
of exceptional cases if you choose the AutoCommit =&gt 0 path, just as
you would be with raw DBI.

----------------
> > DBIC provides safe transaction primitives. Use them please.
>
> The $schema->txn_do ?
>

Yes, see the txn_do marketing above.

> > (note: I'd love if you can help Brandon find the problem but you should
> > only ever use AutoCommit 0 with DBIx::Class if the $dbh needs to be shared
> > with legacy code that requires it; there is no reason whatsoever to do so
> > otherwise, DBIC's transaction primitives are both more effective and more
> > robust)
>
> It was my impression that passing AutoCommit => 1 goes directly to the
> $dbh (correct me if I'm wrong?)
>
> And that AutoCommit => 1 makes every UPDATE/INSERT/DELETE in the
> database, to work as a single entity (correct me if I'm wrong here?)
>
> The case is that we "serialise" some objects into several relations in
> the database and want to encapsulate the complete
> "object-write"/"object-update" procedure as a complete transaction to
> make sure that we dont get any "half" objects in the database if
> comething goes wrong. If I'm not wrong in one of the above cases, then I
> cant see how setting AutoCommit => 1 doesn't open up the situation where
> I might get an inconsistent database?
>

What AutoCommit => 1 does is make it so that if you aren't in an
explicit transaction at the DBI (or DBIC) level, every statement is
committed as an individual transaction.  It doesn't prevent you from
using multi-statement transactions.  AutoCommit => 1 is generally
better all around, regardless of DBIC (but especially with DBIC), for
a lot of obvious (and a few non-obvious) reasons.

> (I'll test Brandons suggestion tonight)

The suggestion to comment out begin_work in your example is just to
test a theory about what our AutoCommit => 0 bug is.  I wouldn't
recommend leaving the code that way, as a future update will probably
break you when we fix this.  Even given all of the above, we haven't
actually made the leap of just forcing everyone to use AutoCommit =>
1, so we'd still like to fix the bug :)

I'm leaning towards what I said earlier: requiring AutoCommit => 0
people to explicitly start their transactions before doing any work
(txn_begin or txn_do), and basically dropping support for AutoCommit
=> 0 and implicit txn starts.  Otherwise the nesting will never work
right for them, and you know they'll expect to use the same DBIC-based
CPAN modules everyone else does, which might have methods with
transactions in them (which is the point of abstracting it and
allowing arbitrary nesting in the first place).

-- Brandon