From: Rafael Kitover <rkitover@cpan.org>
Date: Sat, 29 Aug 2009 19:08:51 +0000 (+0000)
Subject: fix inserts with active cursors
X-Git-Tag: v0.08112~14^2~42
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=166c656193b56e08b472c675e92b9076aac03a53;p=dbsrgits%2FDBIx-Class.git

fix inserts with active cursors
---

diff --git a/lib/DBIx/Class/Storage/DBI.pm b/lib/DBIx/Class/Storage/DBI.pm
index 1c1ddfe..eb923d1 100644
--- a/lib/DBIx/Class/Storage/DBI.pm
+++ b/lib/DBIx/Class/Storage/DBI.pm
@@ -1078,12 +1078,11 @@ sub txn_begin {
 
 sub _dbh_begin_work {
   my $self = shift;
-  # being here implies we have AutoCommit => 1
-  # if the user is utilizing txn_do - good for
-  # him, otherwise we need to ensure that the
-  # $dbh is healthy on BEGIN
-  my $dbh_method = $self->{_in_dbh_do} ? '_dbh' : 'dbh';
-  $self->$dbh_method->begin_work;
+  if ($self->{_in_dbh_do}) {
+    $self->_dbh->begin_work;
+  } else {
+    $self->dbh_do(sub { $_[1]->begin_work });
+  }
 }
 
 sub txn_commit {
diff --git a/lib/DBIx/Class/Storage/DBI/NoBindVars.pm b/lib/DBIx/Class/Storage/DBI/NoBindVars.pm
index 030ad9f..ada367b 100644
--- a/lib/DBIx/Class/Storage/DBI/NoBindVars.pm
+++ b/lib/DBIx/Class/Storage/DBI/NoBindVars.pm
@@ -59,7 +59,7 @@ sub _prep_for_execute {
     foreach my $data (@$bound) {
       $data = ''.$data if ref $data;
 
-      $data = $self->_prep_bind_value($datatype, $data)
+      $data = $self->_prep_interpolated_value($datatype, $data)
         if $datatype;
 
       $data = $self->_dbh->quote($data)
@@ -95,14 +95,14 @@ sub should_quote_value {
   return 1;
 }
 
-=head2 _prep_bind_value
+=head2 _prep_interpolated_value
 
 Given a datatype and the value to be inserted directly into a SQL query, returns
 the necessary string to represent that value (by e.g. adding a '$' sign)
 
 =cut
 
-sub _prep_bind_value {
+sub _prep_interpolated_value {
   #my ($self, $datatype, $value) = @_;
   return $_[2];
 }
diff --git a/lib/DBIx/Class/Storage/DBI/Sybase.pm b/lib/DBIx/Class/Storage/DBI/Sybase.pm
index 886308d..6be2943 100644
--- a/lib/DBIx/Class/Storage/DBI/Sybase.pm
+++ b/lib/DBIx/Class/Storage/DBI/Sybase.pm
@@ -12,7 +12,7 @@ use Carp::Clan qw/^DBIx::Class/;
 use List::Util ();
 
 __PACKAGE__->mk_group_accessors('simple' =>
-    qw/_identity _blob_log_on_update insert_txn/
+    qw/_identity _blob_log_on_update insert_txn _extra_dbh/
 );
 
 =head1 NAME
@@ -34,7 +34,8 @@ also enable that driver explicitly, see the documentation for more details.
 With this driver there is unfortunately no way to get the C<last_insert_id>
 without doing a C<SELECT MAX(col)>. This is done safely in a transaction
 (locking the table.) The transaction can be turned off if concurrency is not an
-issue, see L<DBIx::Class::Storage::DBI::Sybase/connect_call_unsafe_insert>.
+issue, or you don't need the C<IDENTITY> value, see
+L<DBIx::Class::Storage::DBI::Sybase/connect_call_unsafe_insert>.
 
 But your queries will be cached.
 
@@ -132,6 +133,10 @@ sub _populate_dbh {
       $self->_dbh->do('SET CHAINED ON');
     }
   }
+
+# for insert transactions
+  $self->_extra_dbh($self->_connect(@{ $self->_dbi_connect_info }));
+  $self->_extra_dbh->{AutoCommit} = 1;
 }
 
 =head2 connect_call_blob_setup
@@ -311,6 +316,7 @@ sub insert {
   my $updated_cols = do {
     if ($need_last_insert_id && $self->insert_txn &&
         (not $self->{transaction_depth})) {
+      local $self->{_dbh} = $self->_extra_dbh;
       my $guard = $self->txn_scope_guard;
       my $upd_cols = $self->next::method (@_);
       $guard->commit;
@@ -601,6 +607,21 @@ Open Client libraries.
 
 Inserts or updates of TEXT/IMAGE columns will B<NOT> work with FreeTDS.
 
+=head1 TRANSACTIONS
+
+Due to limitations of the TDS protocol, L<DBD::Sybase>, or both; you cannot
+begin a transaction while there are active cursors. An active cursor is, for
+example, a L<ResultSet|DBIx::Class::ResultSet> that has been executed using
+C<next> or C<first> but has not been exhausted or
+L<DBIx::Class::ResultSet/reset>.
+
+To get around this problem, use L<DBIx::Class::ResultSet/all> for smaller
+ResultSets, and/or put the active cursors you will need in the scope of the
+transaction.
+
+Transactions done for inserts in C<AutoCommit> mode when placeholders are in use
+are not affected, as they are executed on a separate connection.
+
 =head1 MAXIMUM CONNECTIONS
 
 The TDS protocol makes separate connections to the server for active statements
diff --git a/lib/DBIx/Class/Storage/DBI/Sybase/NoBindVars.pm b/lib/DBIx/Class/Storage/DBI/Sybase/NoBindVars.pm
index 43837e1..7b59d0a 100644
--- a/lib/DBIx/Class/Storage/DBI/Sybase/NoBindVars.pm
+++ b/lib/DBIx/Class/Storage/DBI/Sybase/NoBindVars.pm
@@ -52,7 +52,7 @@ sub should_quote_value {
   return $self->next::method(@_);
 }
 
-sub _prep_bind_value {
+sub _prep_interpolated_value {
   my ($self, $type, $value) = @_;
 
   if ($type =~ /money/i && defined $value) {
diff --git a/t/746sybase.t b/t/746sybase.t
index 29593ab..260fd07 100644
--- a/t/746sybase.t
+++ b/t/746sybase.t
@@ -6,10 +6,12 @@ use Test::More;
 use Test::Exception;
 use lib qw(t/lib);
 use DBICTest;
+use DBIx::Class::Storage::DBI::Sybase;
+use DBIx::Class::Storage::DBI::Sybase::NoBindVars;
 
 my ($dsn, $user, $pass) = @ENV{map { "DBICTEST_SYBASE_${_}" } qw/DSN USER PASS/};
 
-my $TESTS = 37 + 2;
+my $TESTS = 38 + 2;
 
 if (not ($dsn && $user)) {
   plan skip_all =>
@@ -35,6 +37,15 @@ sub get_schema {
   });
 }
 
+my $ping_count = 0;
+{
+  my $ping = DBIx::Class::Storage::DBI::Sybase->can('_ping');
+  *DBIx::Class::Storage::DBI::Sybase::_ping = sub {
+    $ping_count++;
+    goto $ping;
+  };
+}
+
 for my $storage_type (@storage_types) {
   $storage_idx++;
 
@@ -287,6 +298,21 @@ CREATE TABLE money_test (
 SQL
   });
 
+# First, we'll open a cursor to test insert transactions when there's an active
+# cursor.
+  SKIP: {
+    skip 'not testing insert with active cursor unless using insert_txn', 1
+      unless $schema->storage->insert_txn;
+
+    my $artist_rs = $schema->resultset('Artist');
+    $artist_rs->first;
+    lives_ok {
+      my $row = $schema->resultset('Money')->create({ amount => 100 });
+      $row->delete;
+    } 'inserted a row with an active cursor';
+  }
+
+# Now test money values.
   my $rs = $schema->resultset('Money');
 
   my $row;
@@ -321,4 +347,5 @@ END {
     eval { $dbh->do("DROP TABLE $_") }
       for qw/artist bindtype_test money_test/;
   }
+  diag "ping count was $ping_count" unless $ping_count == 0;
 }