Merge 'trunk' into 'DBIx-Class-current'
[dbsrgits/DBIx-Class.git] / lib / DBIx / Class / Storage / DBI.pm
index d0289c9..14a8f61 100644 (file)
@@ -12,10 +12,10 @@ use DBIx::Class::Storage::Statistics;
 use IO::File;
 use Scalar::Util 'blessed';
 
-__PACKAGE__->mk_group_accessors(
-  'simple' =>
-    qw/_connect_info _dbh _sql_maker _sql_maker_opts _conn_pid _conn_tid
-       disable_sth_caching cursor on_connect_do transaction_depth/
+__PACKAGE__->mk_group_accessors('simple' =>
+    qw/_connect_info _dbi_connect_info _dbh _sql_maker _sql_maker_opts
+       _conn_pid _conn_tid disable_sth_caching cursor on_connect_do
+       transaction_depth/
 );
 
 BEGIN {
@@ -331,7 +331,13 @@ C<connect_info> here.
 
 The arrayref can either contain the same set of arguments one would
 normally pass to L<DBI/connect>, or a lone code reference which returns
-a connected database handle.
+a connected database handle.  Please note that the L<DBI> docs
+recommend that you always explicitly set C<AutoCommit> to either
+C<0> or C<1>.   L<DBIx::Class> further recommends that it be set
+to C<1>, and that you perform transactions via our L</txn_do>
+method.  L<DBIx::Class> will set it to C<1> if you do not do explicitly
+set it to zero.  This is the default for most DBDs.  See below for more
+details.
 
 In either case, if the final argument in your connect_info happens
 to be a hashref, C<connect_info> will look there for several
@@ -390,6 +396,21 @@ might not work very well, YMMV.  If you don't use a subref, DBIC will
 force this setting for you anyways.  Setting HandleError to anything
 other than simple exception object wrapper might cause problems too.
 
+Another Important Note:
+
+DBIC can do some wonderful magic with handling exceptions,
+disconnections, and transactions when you use C<AutoCommit =&gt; 1>
+combined with C<txn_do> for transaction support.
+
+If you set C<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 C<AutoCommit =&gt 0> path, just as you would
+be with raw DBI.
+
 Examples:
 
   # Simple SQLite connection
@@ -404,7 +425,7 @@ Examples:
       'dbi:Pg:dbname=foo',
       'postgres',
       'my_pg_password',
-      { AutoCommit => 0 },
+      { AutoCommit => 1 },
       { quote_char => q{"}, name_sep => q{.} },
     ]
   );
@@ -415,7 +436,7 @@ Examples:
       'dbi:Pg:dbname=foo',
       'postgres',
       'my_pg_password',
-      { AutoCommit => 0, quote_char => q{"}, name_sep => q{.} },
+      { AutoCommit => 1, quote_char => q{"}, name_sep => q{.} },
     ]
   );
 
@@ -443,9 +464,11 @@ sub connect_info {
   #  the new set of options
   $self->_sql_maker(undef);
   $self->_sql_maker_opts({});
+  $self->_connect_info([@$info_arg]); # copy for _connect_info
 
-  my $info = [ @$info_arg ]; # copy because we can alter it
-  my $last_info = $info->[-1];
+  my $dbi_info = [@$info_arg]; # copy for _dbi_connect_info
+
+  my $last_info = $dbi_info->[-1];
   if(ref $last_info eq 'HASH') {
     for my $storage_opt (qw/on_connect_do disable_sth_caching/) {
       if(my $value = delete $last_info->{$storage_opt}) {
@@ -459,10 +482,11 @@ sub connect_info {
     }
 
     # Get rid of any trailing empty hashref
-    pop(@$info) if !keys %$last_info;
+    pop(@$dbi_info) if !keys %$last_info;
   }
+  $self->_dbi_connect_info($dbi_info);
 
-  $self->_connect_info($info);
+  $self->_connect_info;
 }
 
 =head2 on_connect_do
@@ -506,7 +530,9 @@ sub dbh_do {
   ref $coderef eq 'CODE' or $self->throw_exception
     ('$coderef must be a CODE reference');
 
-  return $coderef->($self, $self->_dbh, @_) if $self->{_in_dbh_do};
+  return $coderef->($self, $self->_dbh, @_) if $self->{_in_dbh_do}
+      || $self->{transaction_depth};
+
   local $self->{_in_dbh_do} = 1;
 
   my @result;
@@ -683,9 +709,13 @@ sub sql_maker {
 
 sub _populate_dbh {
   my ($self) = @_;
-  my @info = @{$self->_connect_info || []};
+  my @info = @{$self->_dbi_connect_info || []};
   $self->_dbh($self->_connect(@info));
 
+  # Always set the transaction depth on connect, since
+  #  there is no transaction in progress by definition
+  $self->{transaction_depth} = $self->_dbh->{AutoCommit} ? 0 : 1;
+
   if(ref $self eq 'DBIx::Class::Storage::DBI') {
     my $driver = $self->_dbh->{Driver}->{Name};
     if ($self->load_optional_class("DBIx::Class::Storage::DBI::${driver}")) {
@@ -739,75 +769,61 @@ sub _connect {
   $dbh;
 }
 
-sub _dbh_txn_begin {
-  my ($self, $dbh) = @_;
-  if ($dbh->{AutoCommit}) {
-    $self->debugobj->txn_begin()
-      if ($self->debug);
-    $dbh->begin_work;
-  }
-}
 
 sub txn_begin {
   my $self = shift;
-  $self->dbh_do($self->can('_dbh_txn_begin'))
-    if $self->{transaction_depth}++ == 0;
-}
-
-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;
-    }
+  if($self->{transaction_depth}++ == 0) {
+    $self->debugobj->txn_begin()
+      if $self->debug;
+    # this isn't ->_dbh-> because
+    #  we should reconnect on begin_work
+    #  for AutoCommit users
+    $self->dbh->begin_work;
   }
 }
 
 sub txn_commit {
   my $self = shift;
-  $self->dbh_do($self->can('_dbh_txn_commit'));
+  if ($self->{transaction_depth} == 1) {
+    my $dbh = $self->_dbh;
+    $self->debugobj->txn_commit()
+      if ($self->debug);
+    $dbh->commit;
+    $self->{transaction_depth} = 0
+      if $dbh->{AutoCommit};
+  }
+  elsif($self->{transaction_depth} > 1) {
+    $self->{transaction_depth}--
+  }
 }
 
-sub _dbh_txn_rollback {
-  my ($self, $dbh) = @_;
-  if ($self->{transaction_depth} == 0) {
-    unless ($dbh->{AutoCommit}) {
+sub txn_rollback {
+  my $self = shift;
+  my $dbh = $self->_dbh;
+  my $autocommit;
+  eval {
+    $autocommit = $dbh->{AutoCommit};
+    if ($self->{transaction_depth} == 1) {
       $self->debugobj->txn_rollback()
         if ($self->debug);
       $dbh->rollback;
+      $self->{transaction_depth} = 0
+        if $autocommit;
     }
-  }
-  else {
-    if (--$self->{transaction_depth} == 0) {
-      $self->debugobj->txn_rollback()
-        if ($self->debug);
-      $dbh->rollback;
+    elsif($self->{transaction_depth} > 1) {
+      $self->{transaction_depth}--;
     }
     else {
       die DBIx::Class::Storage::NESTED_ROLLBACK_EXCEPTION->new;
     }
-  }
-}
-
-sub txn_rollback {
-  my $self = shift;
-
-  eval { $self->dbh_do($self->can('_dbh_txn_rollback')) };
+  };
   if ($@) {
     my $error = $@;
     my $exception_class = "DBIx::Class::Storage::NESTED_ROLLBACK_EXCEPTION";
     $error =~ /$exception_class/ and $self->throw_exception($error);
-    $self->{transaction_depth} = 0;          # ensure that a failed rollback
-    $self->throw_exception($error);          # resets the transaction depth
+    # ensure that a failed rollback resets the transaction depth
+    $self->{transaction_depth} = $autocommit ? 0 : 1;
+    $self->throw_exception($error);
   }
 }
 
@@ -1425,7 +1441,7 @@ sub deploy {
       next if($_ =~ /^COMMIT/m);
       next if $_ =~ /^\s+$/; # skip whitespace only
       $self->debugobj->query_start($_) if $self->debug;
-      $self->dbh->do($_) or warn "SQL was:\n $_"; # XXX exceptions?
+      $self->dbh->do($_) or warn $self->dbh->errstr, "\nSQL was:\n $_"; # XXX throw_exception?
       $self->debugobj->query_end($_) if $self->debug;
     }
   }