From: Peter Rabbitson Date: Tue, 20 Mar 2012 05:49:33 +0000 (+0100) Subject: Multiple cleanups to accommodate broken FreeTDS driver X-Git-Tag: v0.08197~70 X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?p=dbsrgits%2FDBIx-Class.git;a=commitdiff_plain;h=25d3127deaaa381fdaa35b8b9d09e0483ba9e532 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 --- 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 --- 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 --- 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 --- 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 column. B on FreeTDS, changes made in one statement (e.g. an insert) may not be visible from a following statement (e.g. a select.) +B 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 --- 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