From: Rafael Kitover Date: Thu, 17 Sep 2009 01:03:34 +0000 (+0000) Subject: I'll rewrite this bit tomorrow to be less retarded X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=7ef97d266d92f1239f075f079cb94f20e31fa3d6;p=dbsrgits%2FDBIx-Class-Historic.git I'll rewrite this bit tomorrow to be less retarded --- diff --git a/lib/DBIx/Class/Storage/DBI/Sybase.pm b/lib/DBIx/Class/Storage/DBI/Sybase.pm index 1bb8956..a11c24f 100644 --- a/lib/DBIx/Class/Storage/DBI/Sybase.pm +++ b/lib/DBIx/Class/Storage/DBI/Sybase.pm @@ -355,17 +355,33 @@ sub update { my ($source, $fields, $where) = @_; my $wantarray = wantarray; + my $blob_cols = $self->_remove_blob_cols($source, $fields); + my $table = $source->name; + + my $identity_col = List::Util::first + { $source->column_info($_)->{is_auto_increment} } + $source->columns; + + my $is_identity_update = $identity_col && defined $fields->{$identity_col}; + if (not $blob_cols) { + $self->_set_identity_insert($table, 'update') if $is_identity_update; return $self->next::method(@_); + $self->_unset_identity_insert($table, 'update') if $is_identity_update; } +# check if condition and fields allow for a 2-step update + $self->_assert_blob_update_possible($source, $fields, $where); + # update+blob update(s) done atomically on separate connection $self = $self->_writer_storage; my $guard = $self->txn_scope_guard; + $self->_set_identity_insert($table, 'update') if $is_identity_update; + my @res; if ($wantarray) { @res = $self->next::method(@_); @@ -377,26 +393,72 @@ sub update { $self->next::method(@_); } - $self->_update_blobs($source, $blob_cols, $where); + $self->_unset_identity_insert($table, 'update') if $is_identity_update; + + my %new_where = map { $_ => ($fields->{$_} || $where->{$_}) } keys %$where; + + $self->_update_blobs($source, $blob_cols, \%new_where); $guard->commit; return $wantarray ? @res : $res[0]; } +sub _assert_blob_update_possible { + my ($self, $source, $fields, $where) = @_; + + my $table = $source->name; + +# If $where condition is mutually exclusive from $fields (what gets updated) +# then update is safe. + my %count; + $count{$_}++ foreach keys %$where, keys %$fields; + return 1 unless List::Util::first { $_ == 2 } values %count; + +# Otherwise check that what is updated includes either a primary or unique key. + my (@primary_cols) = $source->primary_columns; + return 1 if (grep exists $fields->{$_}, @primary_cols) == @primary_cols; + + my %unique_constraints = $source->unique_constraints; + for my $uniq_constr (values %unique_constraints) { + return 1 if (grep exists $fields->{$_}, @$uniq_constr) == @$uniq_constr; + } + +# otherwise throw exception + require Data::Dumper; + local $Data::Dumper::Terse = 1; + local $Data::Dumper::Indent = 1; + local $Data::Dumper::Useqq = 1; + local $Data::Dumper::Quotekeys = 0; + local $Data::Dumper::Sortkeys = 1; + + croak sprintf +"2-step TEXT/IMAGE update on table '$table' impossible for condition: \n%s\n". +"Setting columns: \n%s\n", + Data::Dumper::Dumper($where), + Data::Dumper::Dumper($fields); +} + ### the insert_bulk stuff stolen from DBI/MSSQL.pm sub _set_identity_insert { - my ($self, $table) = @_; + my ($self, $table, $op) = @_; my $sql = sprintf ( - 'SET IDENTITY_INSERT %s ON', + 'SET IDENTITY_%s %s ON', + (uc($op) || 'INSERT'), $self->sql_maker->_quote ($table), ); + $self->_query_start($sql); + my $dbh = $self->_get_dbh; eval { $dbh->do ($sql) }; - if ($@) { + my $exception = $@; + + $self->_query_end($sql); + + if ($exception) { $self->throw_exception (sprintf "Error executing '%s': %s", $sql, $dbh->errstr, @@ -405,15 +467,20 @@ sub _set_identity_insert { } sub _unset_identity_insert { - my ($self, $table) = @_; + my ($self, $table, $op) = @_; my $sql = sprintf ( - 'SET IDENTITY_INSERT %s OFF', + 'SET IDENTITY_%s %s OFF', + (uc($op) || 'INSERT'), $self->sql_maker->_quote ($table), ); + $self->_query_start($sql); + my $dbh = $self->_get_dbh; $dbh->do ($sql); + + $self->_query_end($sql); } # XXX this should use the DBD::Sybase bulk API, where possible @@ -491,7 +558,7 @@ sub _insert_blobs { my ($self, $source, $blob_cols, $row) = @_; my $dbh = $self->_get_dbh; - my $table = $source->from; + my $table = $source->name; my %row = %$row; my (@primary_cols) = $source->primary_columns; @@ -512,6 +579,18 @@ sub _insert_blobs { $cursor->next; my $sth = $cursor->sth; + if (not $sth) { + require Data::Dumper; + local $Data::Dumper::Terse = 1; + local $Data::Dumper::Indent = 1; + local $Data::Dumper::Useqq = 1; + local $Data::Dumper::Quotekeys = 0; + local $Data::Dumper::Sortkeys = 1; + + croak "\nCould not find row in table '$table' for blob update:\n". + Data::Dumper::Dumper(\%where)."\n"; + } + eval { do { $sth->func('CS_GET', 1, 'ct_data_info') or die $sth->errstr; diff --git a/t/746sybase.t b/t/746sybase.t index 9e0caae..1e5eeaa 100644 --- a/t/746sybase.t +++ b/t/746sybase.t @@ -5,13 +5,22 @@ no warnings 'uninitialized'; use Test::More; use Test::Exception; use lib qw(t/lib); + +BEGIN { + require DBICTest::Schema::BindType; + DBICTest::Schema::BindType->add_column( + anint => { data_type => 'integer' } + ); +} + use DBICTest; -use DBIx::Class::Storage::DBI::Sybase; -use DBIx::Class::Storage::DBI::Sybase::NoBindVars; + +require DBIx::Class::Storage::DBI::Sybase; +require DBIx::Class::Storage::DBI::Sybase::NoBindVars; my ($dsn, $user, $pass) = @ENV{map { "DBICTEST_SYBASE_${_}" } qw/DSN USER PASS/}; -my $TESTS = 48 + 2; +my $TESTS = 51 + 2; if (not ($dsn && $user)) { plan skip_all => @@ -157,8 +166,7 @@ SQL is( $it->count, 7, 'COUNT of GROUP_BY ok' ); -# do an identity insert (which should happen with no txn when using -# placeholders.) +# do an IDENTITY_INSERT { no warnings 'redefine'; @@ -178,7 +186,7 @@ SQL $schema->resultset('Artist') ->create({ artistid => 999, name => 'mtfnpy' }); - ok((grep /IDENTITY_INSERT/i, @debug_out), 'IDENTITY_INSERT'); + ok((grep /IDENTITY_INSERT/i, @debug_out), 'IDENTITY_INSERT used'); SKIP: { skip 'not testing lack of txn on IDENTITY_INSERT with NoBindVars', 1 @@ -188,6 +196,23 @@ SQL } } +# do an IDENTITY_UPDATE + { + my @debug_out; + local $schema->storage->{debug} = 1; + local $schema->storage->debugobj->{callback} = sub { + push @debug_out, $_[1]; + }; + + lives_and { + $schema->resultset('Artist') + ->find(999)->update({ artistid => 555 }); + ok((grep /IDENTITY_UPDATE/i, @debug_out)); + } 'IDENTITY_UPDATE used'; + $ping_count-- if $@; + } + + # test insert_bulk using populate, this should always pass whether or not it # does anything Sybase specific or not. Just here to aid debugging. lives_ok { @@ -264,7 +289,7 @@ SQL # mostly stolen from the blob stuff Nniuq wrote for t/73oracle.t SKIP: { - skip 'TEXT/IMAGE support does not work with FreeTDS', 13 + skip 'TEXT/IMAGE support does not work with FreeTDS', 16 if $schema->storage->using_freetds; my $dbh = $schema->storage->_dbh; @@ -278,7 +303,8 @@ SQL id INT IDENTITY PRIMARY KEY, bytea INT NULL, blob IMAGE NULL, - clob TEXT NULL + clob TEXT NULL, + anint INT NULL ) ],{ RaiseError => 1, PrintError => 0 }); } @@ -317,20 +343,9 @@ SQL # blob insert with explicit PK # also a good opportunity to test IDENTITY_INSERT - { - local $SIG{__WARN__} = sub {}; - eval { $dbh->do('DROP TABLE bindtype_test') }; - $dbh->do(qq[ - CREATE TABLE bindtype_test - ( - id INT IDENTITY PRIMARY KEY, - bytea INT NULL, - blob IMAGE NULL, - clob TEXT NULL - ) - ],{ RaiseError => 1, PrintError => 0 }); - } + $rs->delete; + my $created = eval { $rs->create( { id => 1, blob => $binstr{large} } ) }; ok(!$@, "inserted large blob without dying with manual PK"); diag $@ if $@; @@ -359,13 +374,27 @@ SQL diag $@ if $@; ok($got eq $new_str, "verified updated blob"); + # try a blob update with IDENTITY_UPDATE + lives_and { + $new_str = $binstr{large} . 'hlagh'; + $rs->find(1)->update({ id => 999, blob => $new_str }); + ok($rs->find(999)->blob eq $new_str); + } 'verified updated blob with IDENTITY_UPDATE'; + ## try multi-row blob update # first insert some blobs - $rs->find(1)->delete; + $rs->delete; $rs->create({ blob => $binstr{large} }) for (1..3); $new_str = $binstr{large} . 'foo'; $rs->update({ blob => $new_str }); is((grep $_->blob eq $new_str, $rs->all), 3, 'multi-row blob update'); + + # make sure impossible blob update throws + throws_ok { + $rs->update({ anint => 5 }); + $rs->create({ anint => 6 }); + $rs->search({ anint => 5 })->update({ blob => $new_str, anint => 6 }); + } qr/impossible/, 'impossible blob update throws'; } # test MONEY column support