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

[Brandon Black]

On 10/19/07, Brandon Black <blblack at gmail.com> wrote:
> On 10/19/07, Jesper Krogh <jesper at krogh.cc> wrote:
> > > On 10/18/07, Jesper Krogh <jesper at krogh.cc> wrote:
> > >> There is a change in the behaviour of
> > >> $schema->txn_rollback from 0.07* to 0.08*
> > No rollback present for 0.08
> >
>
> I think I see the problem in the code, but I'm still flipping it
> around in my head to decide the best course of action.  The essentials
> of the problem is that DBIC supports fake nested transactions, and it
> tracks this with a transaction_depth attribute, so that it doesn't
> take real action until the outermost transaction is done.  However,
> the implementation is clearly flawed with AutoCommit off.
>
> To confirm, do your results improve if you comment out your
> ->txn_begin lines?  I believe this will make the depth counter work
> correctly, and txn_begin is really a functional no-op with autocommit
> off anyways.  I suspect the same bug affects commits too (assuming one
> started with txn_begin without autocommit), not just rollbacks, so I
> wouldn't expect this to be a very subtle problem.
>

I should add that, assuming I'm right about the nature of the problem,
the core of the issue is the confusion surrounding the
necessity/utility of txn_begin with autocommit off, and its
interaction with nested txns.  We really need to keep the fake nested
txns, as they're important for abstraction (and we can probably
upgrade them to support real nested txns for some vendors down the
road).

So, taking the necessity of abstracted nested txns as requirement, in
the autocommit => 0 world, we therefore should *require* txn_begin at
the start of a transaction, even though it isn't necessary at the
database level (so that we can keep accurate track of depth, and so
that your code doesn't need to know which level of txn abstraction it
is operating at).  Right now the code appears to be somewhat
wishy-washy about trying to also support the non-use of ->txn_begin,
and the two models can't co-exist without bugs.

I would propose that transaction_depth should start at zero with no
autocommit, just as it does with autocommit, and further any attempts
to actually do something should die if autocommit is off and we're at
depth zero.  This would fix your case, and is the only sane thing in
the long run.  It's going to break people who are currently using
AutoCommit => 0 with a single layer of transactions and not using
txn_begin though.

-- Brandon