Multiple cleanups to accommodate broken FreeTDS driver

- Disable batch operations entirely (can get as bad as silent insert failures)
- Disable SELECT SCOPE_IDENTITY generation when used with dynamic cursors
- Clean some warnings while we're at it

diff --git a/Changes b/Changes
index 6a26965..44a5264 100644 (file)
--- a/Changes
+++ b/Changes
@@ -24,6 +24,12 @@ Revision history for DBIx::Class
           handle
         - Improve identity/autoinc retrieval code in MSSQL and Sybase -
           should reduce weird side-effects especially with populate()
+        - Explicitly disable DBD::ODBC batch operations (as of DBD::ODBC 1.35)
+          when using freetds - the freetds driver is just too buggy to handle
+          the optimized path
+        - Explicitly disable DBD::ODBC dynamic_cursors when using freetds 0.83
+          or later - they made enough ODBC incompatible making it impossible
+          to support
         - Fix leakage of $schema on in-memory new_related() calls
         - Fix more cases of $schema leakage in SQLT::Parser::DBIC
         - Fix leakage of $storage in ::Storage::DBI::Oracle

diff --git a/lib/DBIx/Class/Storage/DBI/Cursor.pm b/lib/DBIx/Class/Storage/DBI/Cursor.pm
index 92e8702..bf17e90 100644 (file)
--- a/lib/DBIx/Class/Storage/DBI/Cursor.pm
+++ b/lib/DBIx/Class/Storage/DBI/Cursor.pm
@@ -179,6 +179,7 @@ sub _check_dbh_gen {
 sub DESTROY {
   # None of the reasons this would die matter if we're in DESTROY anyways
   if (my $sth = $_[0]->sth) {
+    local $SIG{__WARN__} = sub {};
     try { $sth->finish } if $sth->FETCH('Active');
   }
 }

diff --git a/lib/DBIx/Class/Storage/DBI/MSSQL.pm b/lib/DBIx/Class/Storage/DBI/MSSQL.pm
index 5474e36..b20db9f 100644 (file)
--- a/lib/DBIx/Class/Storage/DBI/MSSQL.pm
+++ b/lib/DBIx/Class/Storage/DBI/MSSQL.pm
@@ -14,7 +14,7 @@ use List::Util 'first';
 use namespace::clean;
 
 __PACKAGE__->mk_group_accessors(simple => qw/
-  _identity _identity_method
+  _identity _identity_method _no_scope_identity_query
 /);
 
 __PACKAGE__->sql_maker_class('DBIx::Class::SQLMaker::MSSQL');
@@ -60,7 +60,7 @@ sub _prep_for_execute {
   # point we don't have many guarantees we will get what we expected.
   # http://msdn.microsoft.com/en-us/library/ms190315.aspx
   # http://davidhayden.com/blog/dave/archive/2006/01/17/2736.aspx
-  if ($self->_perform_autoinc_retrieval) {
+  if ($self->_perform_autoinc_retrieval and not $self->_no_scope_identity_query) {
     $sql .= "\nSELECT SCOPE_IDENTITY()";
   }
 
@@ -76,9 +76,15 @@ sub _execute {
 
   if ($self->_perform_autoinc_retrieval) {
 
-    # this should bring back the result of SELECT SCOPE_IDENTITY() we tacked
+    # attempt to bring back the result of SELECT SCOPE_IDENTITY() we tacked
     # on in _prep_for_execute above
-    my ($identity) = try { $sth->fetchrow_array };
+    my $identity;
+
+    # we didn't even try on ftds
+    unless ($self->_no_scope_identity_query) {
+      ($identity) = try { $sth->fetchrow_array };
+      $sth->finish;
+    }
 
     # SCOPE_IDENTITY failed, but we can do something else
     if ( (! $identity) && $self->_identity_method) {
@@ -88,7 +94,6 @@ sub _execute {
     }
 
     $self->_identity($identity);
-    $sth->finish;
   }
 
   return wantarray ? ($rv, $sth, @bind) : $rv;

diff --git a/lib/DBIx/Class/Storage/DBI/ODBC/Microsoft_SQL_Server.pm b/lib/DBIx/Class/Storage/DBI/ODBC/Microsoft_SQL_Server.pm
index e94c938..afd51f3 100644 (file)
--- a/lib/DBIx/Class/Storage/DBI/ODBC/Microsoft_SQL_Server.pm
+++ b/lib/DBIx/Class/Storage/DBI/ODBC/Microsoft_SQL_Server.pm
@@ -177,6 +177,11 @@ insert trigger that inserts into another table with an C<IDENTITY> column.
 B<WARNING:> on FreeTDS, changes made in one statement (e.g. an insert) may not
 be visible from a following statement (e.g. a select.)
 
+B<WARNING:> FreeTDS versions > 0.82 seem to have completely broken the ODBC
+protocol. DBIC will not allow dynamic cursor support with such versions to
+protect your data. Please hassle the authors of FreeTDS to act on the bugs that
+make their driver not overly usable with DBD::ODBC.
+
 =cut
 
 sub connect_call_use_dynamic_cursors {
@@ -230,14 +235,34 @@ sub _run_connection_actions {
 
       $self->_using_dynamic_cursors(1);
       $self->_identity_method('@@identity');
+      $self->_no_scope_identity_query($self->using_freetds);
     }
     else {
       $self->_using_dynamic_cursors(0);
       $self->_identity_method(undef);
+      $self->_no_scope_identity_query(undef);
     }
   }
 
   $self->next::method (@_);
+
+  # freetds is too damn broken, some fixups
+  if ($self->using_freetds) {
+
+    # no dynamic cursors starting from 0.83
+    if ($self->_using_dynamic_cursors) {
+      my $fv = $self->_dbh_get_info('SQL_DRIVER_VER');
+      $self->throw_exception(
+        'Dynamic cursors (odbc_cursortype => 2) are not supported with FreeTDS > 0.82 '
+      . "(you have $fv). Please hassle FreeTDS authors to fix the outstanding bugs in "
+      . 'their driver.'
+      ) if $fv > 0.82
+    }
+
+    # FreeTDS is too broken wrt execute_for_fetch batching
+    # just disable it outright until things quiet down
+    $self->_get_dbh->{odbc_disable_array_operations} = 1;
+  }
 }
 
 =head2 connect_call_use_server_cursors

diff --git a/t/746mssql.t b/t/746mssql.t
index bafbef2..403d75c 100644 (file)
--- a/t/746mssql.t
+++ b/t/746mssql.t
@@ -57,7 +57,9 @@ my %opts = (
   use_mars =>
     { opts => { on_connect_call => 'use_mars' } },
   use_dynamic_cursors =>
-    { opts => { on_connect_call => 'use_dynamic_cursors' }, required => 1 },
+    { opts => { on_connect_call => 'use_dynamic_cursors' },
+      required => $schema->storage->using_freetds ? 0 : 1,
+    },
   use_server_cursors =>
     { opts => { on_connect_call => 'use_server_cursors' } },
   NO_OPTION =>
@@ -133,6 +135,8 @@ SQL
       is_deeply \@result, \@map, "multiple active statements in $opts_name";
 
       $artist_rs->delete;
+
+      is($artist_rs->count, 0, '$dbh still viable');
     }
 
 # Test populate