[Dbix-class] Re: Transactions and AutoCommit

[Josef Karthauser]

On Sat, Jan 27, 2007 at 09:28:34AM +0000, Josef Karthauser wrote:
> 
> What I propose is removing the unnecessary complexity.
> 

The code change I propose is something like this.  I've moved all the
nested level checking into the txn_{begin,commit,rollback} functions.
They then decide to call _dbh_txn_{begin,commit,rollback} or new
functions _dbh_txn_nested_{begin,commit,rollback}, depending upon
what commit level they are operating at.  By default the functions
_dbh_txn_nested_{begin,commit} do nothing and are just stubs, and the
nested rollback causes an exception.

The _dbh_txn_nested_* functions are candidates for over-riding in
the .../DBI/database.pm files so that each driver can implement a
level of nesting if they want.  For example the mysql driver can support
a level of nested commits using the SQL syntax,

    SAVEPOINT identifier
    ROLLBACK [WORK] TO SAVEPOINT identifier
    RELEASE SAVEPOINT identifier,

and so with these changes to .../Storage/DBI.pm it is a relatively
simple matter to implement this.

The second thing this patch does is remove the dependancy upon the
AutoCommit, so that it trusts DBI's judgement as to whether a
transaction command is valid or not, and propagates any errors back
to the caller.

Joe


=== lib/DBIx/Class/Storage/DBI.pm
==================================================================
--- lib/DBIx/Class/Storage/DBI.pm	(revision 3063)
+++ lib/DBIx/Class/Storage/DBI.pm	(local)
@@ -741,67 +741,66 @@
 
 sub _dbh_txn_begin {
   my ($self, $dbh) = @_;
-  if ($dbh->{AutoCommit}) {
-    $self->debugobj->txn_begin()
-      if ($self->debug);
-    $dbh->begin_work;
-  }
+  $self->debugobj->txn_begin() if ($self->debug);
+  $dbh->begin_work;
 }
 
+sub _dbh_txn_nested_begin {
+  my ($self, $dbh) = @_;
+  # Do nothing; by default databases don't support nested transactions.
+}
+
 sub txn_begin {
   my $self = shift;
-  $self->dbh_do($self->can('_dbh_txn_begin'))
-    if $self->{transaction_depth}++ == 0;
+  $self->ensure_connected();
+  if ($self->{transaction_depth}++ == 0) {
+    $self->dbh_do($self->can('_dbh_txn_begin'));
+  } else {
+    $self->dbh_do($self->can('_dbh_txn_nested_begin'));
+  }
 }
 
 sub _dbh_txn_commit {
   my ($self, $dbh) = @_;
-  if ($self->{transaction_depth} == 0) {
-    unless ($dbh->{AutoCommit}) {
-      $self->debugobj->txn_commit()
-        if ($self->debug);
-      $dbh->commit;
-    }
-  }
-  else {
-    if (--$self->{transaction_depth} == 0) {
-      $self->debugobj->txn_commit()
-        if ($self->debug);
-      $dbh->commit;
-    }
-  }
+  $self->debugobj->txn_commit() if ($self->debug);
+  $dbh->commit;
 }
 
+sub _dbh_txn_nested_commit {
+  my ($self, $dbh) = @_;
+  # Do nothing; by default databases don't support nested transactions.
+}
+
 sub txn_commit {
   my $self = shift;
-  $self->dbh_do($self->can('_dbh_txn_commit'));
+  if (--$self->{transaction_depth} <= 0) {
+    $self->{transaction_depth} = 0;
+    $self->dbh_do($self->can('_dbh_txn_commit'));
+  } else {
+    $self->dbh_do($self->can('_dbh_txn_nested_commit'));
+  }
 }
 
 sub _dbh_txn_rollback {
   my ($self, $dbh) = @_;
-  if ($self->{transaction_depth} == 0) {
-    unless ($dbh->{AutoCommit}) {
-      $self->debugobj->txn_rollback()
-        if ($self->debug);
-      $dbh->rollback;
-    }
-  }
-  else {
-    if (--$self->{transaction_depth} == 0) {
-      $self->debugobj->txn_rollback()
-        if ($self->debug);
-      $dbh->rollback;
-    }
-    else {
-      die DBIx::Class::Storage::NESTED_ROLLBACK_EXCEPTION->new;
-    }
-  }
+  $self->debugobj->txn_rollback() if ($self->debug);
+  $dbh->rollback;
 }
 
+sub _dbh_txn_nested_rollback {
+  my ($self, $dbh) = @_;
+  die DBIx::Class::Storage::NESTED_ROLLBACK_EXCEPTION->new;
+}
+
 sub txn_rollback {
   my $self = shift;
 
-  eval { $self->dbh_do($self->can('_dbh_txn_rollback')) };
+  if (--$self->{transaction_depth} <= 0) {
+    $self->{transaction_depth} = 0;
+    eval { $self->dbh_do($self->can('_dbh_txn_rollback')) };
+  } else {
+    eval { $self->dbh_do($self->can('_dbh_txn_nested_rollback')) };
+  }
   if ($@) {
     my $error = $@;
     my $exception_class = "DBIx::Class::Storage::NESTED_ROLLBACK_EXCEPTION";
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
Url : http://lists.scsys.co.uk/pipermail/dbix-class/attachments/20070127/36a2fa73/attachment.pgp