From: Rafael Kitover Date: Wed, 9 Sep 2009 00:15:54 +0000 (+0000) Subject: remove unsafe_insert X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=322b7a6bb9b1c41935395b8fe809f2c36ac945dd;p=dbsrgits%2FDBIx-Class-Historic.git remove unsafe_insert --- diff --git a/lib/DBIx/Class/Storage/DBI/Sybase.pm b/lib/DBIx/Class/Storage/DBI/Sybase.pm index 68c2e20..43932fa 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 unsafe_insert _insert_dbh/ + qw/_identity _blob_log_on_update _insert_dbh _identity_method/ ); =head1 NAME @@ -33,11 +33,7 @@ also enable that driver explicitly, see the documentation for more details. With this driver there is unfortunately no way to get the C without doing a C when placeholders are enabled. - -When using C transactions are -disabled. - -To turn off transactions for inserts (for an application that doesn't need -concurrency, or a loader, for example) use this setting in -L, - - on_connect_call => ['unsafe_insert'] - -To manipulate this setting at runtime, use: - - $schema->storage->unsafe_insert(0|1); - -=cut - -sub connect_call_unsafe_insert { - my $self = shift; - $self->unsafe_insert(1); -} - sub _is_lob_type { my $self = shift; my $type = shift; @@ -292,8 +262,18 @@ sub insert { my $blob_cols = $self->_remove_blob_cols($source, $to_insert); -# insert+blob insert done atomically - my $guard = $self->txn_scope_guard if $blob_cols; +# insert+blob insert done atomically, on _insert_dbh + (my ($guard), local ($self->{_dbh})) = do { + $self->_insert_dbh($self->_connect(@{ $self->_dbi_connect_info })) + unless $self->_insert_dbh; + + my $new_guard = $self->txn_scope_guard; + +# _dbh_begin_work may reconnect, if so we need to update _insert_dbh + $self->_insert_dbh($self->_dbh); + + ($new_guard, $self->_insert_dbh) + } if $blob_cols; my $need_last_insert_id = 0; @@ -309,16 +289,24 @@ sub insert { # We have to do the insert in a transaction to avoid race conditions with the # SELECT MAX(COL) identity method used when placeholders are enabled. my $updated_cols = do { + no warnings 'uninitialized'; if ( - $need_last_insert_id && !$self->unsafe_insert && !$self->{transaction_depth} + $need_last_insert_id && + $self->_identity_method ne '@@IDENTITY' && + !$self->{transaction_depth} ) { $self->_insert_dbh($self->_connect(@{ $self->_dbi_connect_info })) unless $self->_insert_dbh; local $self->{_dbh} = $self->_insert_dbh; + my $guard = $self->txn_scope_guard; + +# _dbh_begin_work may reconnect, if so we need to update _insert_dbh + $self->_insert_dbh($self->_dbh); + my $upd_cols = $self->next::method (@_); $guard->commit; - $self->_insert_dbh($self->_dbh); + $upd_cols; } else { @@ -613,6 +601,17 @@ Open Client libraries. Inserts or updates of TEXT/IMAGE columns will B work with FreeTDS. +=head1 INSERTS WITH PLACEHOLDERS + +With placeholders enabled, inserts are done in a transaction so that there are +no concurrency issues with getting the inserted identity value using +C as it's a +session variable. + =head1 TRANSACTIONS Due to limitations of the TDS protocol, L, or both; you cannot @@ -621,6 +620,18 @@ example, a L that has been executed using C or C but has not been exhausted or L. +For example, this will not work: + + $schema->txn_do(sub { + my $rs = $schema->resultset('Book'); + while (my $row = $rs->next) { + $schema->resultset('MetaData')->create({ + book_id => $row->id, + ... + }); + } + }); + Transactions done for inserts in C mode when placeholders are in use are not affected, as they use an extra database handle to do the insert. @@ -634,8 +645,6 @@ Some workarounds: =item * load the data from your cursor with L -=item * enlarge the scope of the transaction - =back =head1 MAXIMUM CONNECTIONS diff --git a/lib/DBIx/Class/Storage/DBI/Sybase/Common.pm b/lib/DBIx/Class/Storage/DBI/Sybase/Common.pm index c183a9f..c83398f 100644 --- a/lib/DBIx/Class/Storage/DBI/Sybase/Common.pm +++ b/lib/DBIx/Class/Storage/DBI/Sybase/Common.pm @@ -28,7 +28,18 @@ sub _ping { my $dbh = $self->_dbh or return 0; local $dbh->{RaiseError} = 1; + local $dbh->{PrintError} = 0; + local $@; + + if ($dbh->{syb_no_child_con}) { +# ping is impossible with an active statement, we return false if so + my $ping = eval { $dbh->ping }; + return $@ ? 0 : $ping; + } + eval { +# XXX if the main connection goes stale, does opening another for this statement +# really determine anything? $dbh->do('select 1'); }; @@ -72,14 +83,16 @@ use this function instead. It does: $dbh->do("SET TEXTSIZE $bytes"); Takes the number of bytes, or uses the C value from your -L if omitted. +L if omitted, lastly falls back to the C<32768> which +is the L default. =cut sub set_textsize { my $self = shift; my $text_size = shift || - eval { $self->_dbi_connect_info->[-1]->{LongReadLen} }; + eval { $self->_dbi_connect_info->[-1]->{LongReadLen} } || + 32768; # the DBD::Sybase default return unless defined $text_size; diff --git a/lib/DBIx/Class/Storage/DBI/Sybase/Microsoft_SQL_Server.pm b/lib/DBIx/Class/Storage/DBI/Sybase/Microsoft_SQL_Server.pm index e7f0e51..22661a8 100644 --- a/lib/DBIx/Class/Storage/DBI/Sybase/Microsoft_SQL_Server.pm +++ b/lib/DBIx/Class/Storage/DBI/Sybase/Microsoft_SQL_Server.pm @@ -9,6 +9,12 @@ use base qw/ /; use mro 'c3'; +sub new { + my $self = shift->next::method(@_); + $self->_rebless; + return $self; +} + sub _rebless { my $self = shift; my $dbh = $self->_get_dbh; @@ -21,11 +27,8 @@ sub _rebless { # LongReadLen doesn't work with MSSQL through DBD::Sybase, and the default is # huge on some versions of SQL server and can cause memory problems, so we -# fix it up here. - my $text_size = eval { $self->_dbi_connect_info->[-1]->{LongReadLen} } || - 32768; # the DBD::Sybase default - - $dbh->do("set textsize $text_size"); +# fix it up here (see Sybase/Common.pm .) + $self->set_textsize; } 1; diff --git a/lib/DBIx/Class/Storage/DBI/Sybase/Microsoft_SQL_Server/NoBindVars.pm b/lib/DBIx/Class/Storage/DBI/Sybase/Microsoft_SQL_Server/NoBindVars.pm index 16db6d1..4fc6f6d 100644 --- a/lib/DBIx/Class/Storage/DBI/Sybase/Microsoft_SQL_Server/NoBindVars.pm +++ b/lib/DBIx/Class/Storage/DBI/Sybase/Microsoft_SQL_Server/NoBindVars.pm @@ -9,6 +9,12 @@ use base qw/ /; use mro 'c3'; +sub new { + my $self = shift->next::method(@_); + $self->_rebless; + return $self; +} + sub _rebless { my $self = shift; diff --git a/lib/DBIx/Class/Storage/DBI/Sybase/NoBindVars.pm b/lib/DBIx/Class/Storage/DBI/Sybase/NoBindVars.pm index d40d0a6..7737d95 100644 --- a/lib/DBIx/Class/Storage/DBI/Sybase/NoBindVars.pm +++ b/lib/DBIx/Class/Storage/DBI/Sybase/NoBindVars.pm @@ -1,23 +1,26 @@ package DBIx::Class::Storage::DBI::Sybase::NoBindVars; -use Class::C3; use base qw/ DBIx::Class::Storage::DBI::NoBindVars DBIx::Class::Storage::DBI::Sybase /; +use mro 'c3'; use List::Util (); use Scalar::Util (); +sub new { + my $self = shift->next::method(@_); + $self->_rebless; + return $self; +} + sub _rebless { my $self = shift; $self->disable_sth_caching(1); - $self->unsafe_insert(1); # there is nothing unsafe as the - # last_insert_id mechanism is different - # without bindvars + $self->_identity_method('@@IDENTITY'); } -# this works when NOT using placeholders -sub _fetch_identity_sql { 'SELECT @@IDENTITY' } +sub _fetch_identity_sql { 'SELECT ' . $_[0]->_identity_method } my $number = sub { Scalar::Util::looks_like_number($_[0]) }; diff --git a/t/746sybase.t b/t/746sybase.t index 7e5696a..b8a42e2 100644 --- a/t/746sybase.t +++ b/t/746sybase.t @@ -11,7 +11,7 @@ use DBIx::Class::Storage::DBI::Sybase::NoBindVars; my ($dsn, $user, $pass) = @ENV{map { "DBICTEST_SYBASE_${_}" } qw/DSN USER PASS/}; -my $TESTS = 39 + 2; +my $TESTS = 40 + 2; if (not ($dsn && $user)) { plan skip_all => @@ -308,21 +308,36 @@ SQL }); # test insert transaction when there's an active cursor - TODO: { -# local $TODO = 'not supported yet or possibly ever'; + SKIP: { + skip 'not testing insert with active cursor if using ::NoBindVars', 1 + if $storage_type =~ /NoBindVars/i; + + 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'; + $ping_count-- if $@; # dbh_do calls ->connected + } - SKIP: { - skip 'not testing insert with active cursor if using unsafe_insert', 1 - if $schema->storage->unsafe_insert; +# test insert in an outer transaction when there's an active cursor + TODO: { + local $TODO = 'this should work once we have eager cursors'; - my $artist_rs = $schema->resultset('Artist'); - $artist_rs->first; - lives_ok { +# clear state, or we get a deadlock on $row->delete +# XXX figure out why this happens + $schema->storage->disconnect; + + lives_ok { + $schema->txn_do(sub { + my $artist_rs = $schema->resultset('Artist'); + $artist_rs->first; my $row = $schema->resultset('Money')->create({ amount => 100 }); $row->delete; - } 'inserted a row with an active cursor'; - $ping_count-- if $@; # dbh_do calls ->connected - } + }); + } 'inserted a row with an active cursor in outer txn'; + $ping_count-- if $@; # dbh_do calls ->connected } # Now test money values.