From: Rafael Kitover Date: Thu, 24 Sep 2009 12:45:04 +0000 (+0000) Subject: remove some duplicate code X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=587daa97726631a2a5edc95ea6bfe62bf340bd8d;p=dbsrgits%2FDBIx-Class-Historic.git remove some duplicate code --- diff --git a/lib/DBIx/Class/Storage/DBI.pm b/lib/DBIx/Class/Storage/DBI.pm index 938f4d5..bf7bdf2 100644 --- a/lib/DBIx/Class/Storage/DBI.pm +++ b/lib/DBIx/Class/Storage/DBI.pm @@ -1346,7 +1346,7 @@ sub insert_bulk { my %colvalues; @colvalues{@$cols} = (0..$#$cols); - # bind literal sql if it's the same in all slices + # pass scalarref to SQLA for literal sql if it's the same in all slices for my $i (0..$#$cols) { my $first_val = $data->[0][$i]; next unless (Scalar::Util::reftype($first_val)||'') eq 'SCALAR'; @@ -1374,23 +1374,24 @@ sub insert_bulk { $self->_query_start( $sql, @bind ); my $sth = $self->sth($sql, 'insert', $sth_attr); - if ($empty_bind) { - # bind_param_array doesn't work if there are no binds - eval { - local $self->_get_dbh->{RaiseError} = 1; - local $self->_get_dbh->{PrintError} = 0; - foreach (0..$#$data) { - $sth->execute; - $sth->fetchall_arrayref; - } - }; - my $exception = $@; - $sth->finish; - $self->throw_exception($exception) if $exception; - return; - } + my $rv = do { + if ($empty_bind) { + # bind_param_array doesn't work if there are no binds + $self->_execute_array_empty( $sth, scalar @$data ); + } + else { +# @bind = map { ref $_ ? ''.$_ : $_ } @bind; # stringify args + $self->_execute_array( $source, $sth, \@bind, $cols, $data ); + } + }; + + $self->_query_end( $sql, @bind ); -# @bind = map { ref $_ ? ''.$_ : $_ } @bind; # stringify args + return (wantarray ? ($rv, $sth, @bind) : $rv); +} + +sub _execute_array { + my ($self, $source, $sth, $bind, $cols, $data, $guard) = @_; ## This must be an arrayref, else nothing works! my $tuple_status = []; @@ -1401,7 +1402,7 @@ sub insert_bulk { ## Bind the values and execute my $placeholder_index = 1; - foreach my $bound (@bind) { + foreach my $bound (@$bind) { my $attributes = {}; my ($column_name, $data_index) = @$bound; @@ -1418,7 +1419,11 @@ sub insert_bulk { } my $rv = eval { $sth->execute_array({ArrayTupleStatus => $tuple_status}) }; + + $guard->commit if $guard; # probably only needed for Sybase + $sth->finish; + if (my $err = $@ || $sth->errstr) { my $i = 0; ++$i while $i <= $#$tuple_status && !ref $tuple_status->[$i]; @@ -1434,8 +1439,27 @@ sub insert_bulk { ); } - $self->_query_end( $sql, @bind ); - return (wantarray ? ($rv, $sth, @bind) : $rv); + return $rv; +} + +sub _execute_array_empty { + my ($self, $sth, $count) = @_; + eval { + my $dbh = $self->_get_dbh; + local $dbh->{RaiseError} = 1; + local $dbh->{PrintError} = 0; + foreach (1..$count) { + $sth->execute; +# In case of a multi-statement with a select, some DBDs (namely Sybase) require +# the cursor to be exhausted. + $sth->fetchall_arrayref; + } + }; + my $exception = $@; + $sth->finish; + $self->throw_exception($exception) if $exception; + + return $count; } sub update { diff --git a/lib/DBIx/Class/Storage/DBI/Sybase.pm b/lib/DBIx/Class/Storage/DBI/Sybase.pm index 319b703..5747cb9 100644 --- a/lib/DBIx/Class/Storage/DBI/Sybase.pm +++ b/lib/DBIx/Class/Storage/DBI/Sybase.pm @@ -510,7 +510,7 @@ sub insert_bulk { $source->columns; my $is_identity_insert = (List::Util::first - { $source->column_info ($_)->{is_auto_increment} } + { $_ eq $identity_col } @{$cols} ) ? 1 : 0; @@ -525,7 +525,7 @@ sub insert_bulk { (not $self->_bulk_disabled_due_to_coderef_connect_info_warned)) { carp <<'EOF'; Bulk API support disabled due to use of a CODEREF connect_info. Reverting to -array inserts. +regular array inserts. EOF $self->_bulk_disabled_due_to_coderef_connect_info_warned(1); } @@ -647,42 +647,14 @@ EOF } ); - my $bind_attributes = $self->source_bind_attributes($source); - - foreach my $slice_idx (0..$#source_columns) { - my $col = $source_columns[$slice_idx]; - - my $attributes = $bind_attributes->{$col} - if $bind_attributes && defined $bind_attributes->{$col}; - - my @slice = map $_->[$slice_idx], @new_data; - - $sth->bind_param_array(($slice_idx + 1), \@slice, $attributes); - } - - $bulk->_query_start($sql); - -# this is stolen from DBI::insert_bulk - my $tuple_status = []; - my $rv = eval { $sth->execute_array({ArrayTupleStatus => $tuple_status}) }; - - if (my $err = $@ || $sth->errstr) { - my $i = 0; - ++$i while $i <= $#$tuple_status && !ref $tuple_status->[$i]; - - $self->throw_exception("Unexpected populate error: $err") - if ($i > $#$tuple_status); - - $self->throw_exception(sprintf "%s for populate slice:\n%s", - ($tuple_status->[$i][1] || $err), - $self->_pretty_print ({ - map { $source_columns[$_] => $new_data[$i][$_] } (0 .. $#$cols) - }), - ); - } + my @bind = do { + my $idx = 0; + map [ $_, $idx++ ], @source_columns; + }; - $guard->commit; - $sth->finish; + $self->_execute_array( + $source, $sth, \@bind, \@source_columns, \@new_data, $guard + ); $bulk->_query_end($sql); }; @@ -720,7 +692,7 @@ sub _remove_blob_cols { my %blob_cols; for my $col (keys %$fields) { - if ($self->_is_lob_type($source->column_info($col)->{data_type})) { + if ($self->_is_lob_column($source, $col)) { my $blob_val = delete $fields->{$col}; if (not defined $blob_val) { $fields->{$col} = \'NULL'; @@ -744,7 +716,7 @@ sub _remove_blob_cols_array { for my $i (0..$#$cols) { my $col = $cols->[$i]; - if ($self->_is_lob_type($source->column_info($col)->{data_type})) { + if ($self->_is_lob_column($source, $col)) { for my $j (0..$#$data) { my $blob_val = delete $data->[$j][$i]; if (not defined $blob_val) { diff --git a/t/746sybase.t b/t/746sybase.t index c05b09d..81982af 100644 --- a/t/746sybase.t +++ b/t/746sybase.t @@ -12,7 +12,7 @@ require DBIx::Class::Storage::DBI::Sybase::NoBindVars; my ($dsn, $user, $pass) = @ENV{map { "DBICTEST_SYBASE_${_}" } qw/DSN USER PASS/}; -my $TESTS = 58 + 2; +my $TESTS = 62 + 2; if (not ($dsn && $user)) { plan skip_all => @@ -336,7 +336,7 @@ SQL # mostly stolen from the blob stuff Nniuq wrote for t/73oracle.t SKIP: { - skip 'TEXT/IMAGE support does not work with FreeTDS', 18 + skip 'TEXT/IMAGE support does not work with FreeTDS', 22 if $schema->storage->using_freetds; my $dbh = $schema->storage->_dbh; @@ -434,7 +434,7 @@ SQL $rs->delete; - # now try insert_bulk with blobs + # now try insert_bulk with blobs and only blobs $new_str = $binstr{large} . 'bar'; lives_ok { $rs->populate([ @@ -457,6 +457,41 @@ SQL is((grep $_->clob eq $new_str, $rs->all), 2, 'TEXT column set correctly via insert_bulk'); + # now try insert_bulk with blobs and a non-blob which also happens to be an + # identity column + SKIP: { + skip 'no insert_bulk without placeholders', 4 + if $storage_type =~ /NoBindVars/i; + + $rs->delete; + $new_str = $binstr{large} . 'bar'; + lives_ok { + $rs->populate([ + { + id => 1, + bytea => 1, + blob => $binstr{large}, + clob => $new_str, + }, + { + id => 2, + bytea => 1, + blob => $binstr{large}, + clob => $new_str, + }, + ]); + } 'insert_bulk with blobs and explicit identity DOES not die'; + + is((grep $_->blob eq $binstr{large}, $rs->all), 2, + 'IMAGE column set correctly via insert_bulk with identity'); + + is((grep $_->clob eq $new_str, $rs->all), 2, + 'TEXT column set correctly via insert_bulk with identity'); + + is_deeply [ map $_->id, $rs->all ], [ 1,2 ], + 'explicit identities set correctly via insert_bulk with blobs'; + } + lives_and { $rs->delete; $rs->create({ blob => $binstr{large} }) for (1..2);