From: Peter Rabbitson <ribasushi@cpan.org>
Date: Tue, 18 Jan 2011 13:49:21 +0000 (+0100)
Subject: Fix incorrect error detection during populate() on Oracle
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=a5a27e7a8da524a8ad4154f3c3a167d98a89a2f0;p=dbsrgits%2FDBIx-Class-Historic.git

Fix incorrect error detection during populate() on Oracle
---

diff --git a/Changes b/Changes
index 1bc1108..c721d14 100644
--- a/Changes
+++ b/Changes
@@ -5,6 +5,7 @@ Revision history for DBIx::Class
           invocation
         - Fix sloppy refactor of RSrc::sequence back from 89170201
           (RT#64839)
+        - Fix incorrect error detection during populate() on Oracle
 
 0.08126_01 2011-01-14 14:00 (UTC)
     * New Features / Changes
diff --git a/lib/DBIx/Class/Storage/DBI.pm b/lib/DBIx/Class/Storage/DBI.pm
index a986557..70dcb24 100644
--- a/lib/DBIx/Class/Storage/DBI.pm
+++ b/lib/DBIx/Class/Storage/DBI.pm
@@ -1829,6 +1829,14 @@ sub _execute_array {
     $err = shift;
   };
 
+  # Not all DBDs are create equal. Some throw on error, some return
+  # an undef $rv, and some set $sth->err - try whatever we can
+  $err = ($sth->errstr || 'UNKNOWN ERROR ($sth->errstr is unset)') if (
+    ! defined $err
+      and
+    ( !defined $rv or $sth->err )
+  );
+
   # Statement must finish even if there was an exception.
   try {
     $sth->finish
@@ -1837,9 +1845,6 @@ sub _execute_array {
     $err = shift unless defined $err
   };
 
-  $err = $sth->errstr
-    if (! defined $err and $sth->err);
-
   if (defined $err) {
     my $i = 0;
     ++$i while $i <= $#$tuple_status && !ref $tuple_status->[$i];
diff --git a/lib/DBIx/Class/Storage/DBI/Oracle/Generic.pm b/lib/DBIx/Class/Storage/DBI/Oracle/Generic.pm
index 939b837..220eb26 100644
--- a/lib/DBIx/Class/Storage/DBI/Oracle/Generic.pm
+++ b/lib/DBIx/Class/Storage/DBI/Oracle/Generic.pm
@@ -308,6 +308,15 @@ sub _dbh_execute {
   return wantarray ? @res : $res[0];
 }
 
+sub _dbh_execute_array {
+  #my ($self, $sth, $tuple_status, @extra) = @_;
+
+  # DBD::Oracle warns loudly on partial execute_array failures
+  local $_[1]->{PrintWarn} = 0;
+
+  shift->next::method(@_);
+}
+
 =head2 get_autoinc_seq
 
 Returns the sequence name for an autoincrement column
diff --git a/t/73oracle.t b/t/73oracle.t
index ca373cb..b372adb 100644
--- a/t/73oracle.t
+++ b/t/73oracle.t
@@ -359,6 +359,57 @@ sub _run_tests {
     $schema->storage->debug ($orig_debug);
   }}
 
+# test populate (identity, success and error handling)
+  my $art_rs = $schema->resultset('Artist');
+
+  my $seq_pos = $art_rs->get_column('artistid')->max;
+  ok($seq_pos, 'Starting with something in the artist table');
+
+
+  my $pop_rs = $schema->resultset('Artist')->search(
+    { name => { -like => 'pop_art_%' } },
+    { order_by => 'artistid' }
+  );
+
+  $art_rs->delete;
+  lives_ok {
+    $pop_rs->populate([
+      map { +{ name => "pop_art_$_" } }
+      (1,2,3)
+    ]);
+
+    is_deeply (
+      [ $pop_rs->get_column('artistid')->all ],
+      [ map { $seq_pos + $_ } (1,2,3) ],
+      'Sequence works after empty-table insertion'
+    );
+  } 'Populate without identity does not throw';
+
+  lives_ok {
+    $pop_rs->populate([
+      map { +{ artistid => $_, name => "pop_art_$_" } }
+      (1,2,3)
+    ]);
+
+    is_deeply (
+      [ $pop_rs->get_column('artistid')->all ],
+      [ 1,2,3, map { $seq_pos + $_ } (1,2,3) ],
+      'Explicit id population works'
+    );
+  } 'Populate with identity does not throw';
+
+  throws_ok {
+    $pop_rs->populate([
+      map { +{ artistid => $_, name => "pop_art_$_" } }
+      (200, 1, 300)
+    ]);
+  } qr/unique constraint.+populate slice.+name => "pop_art_1"/s, 'Partially failed populate throws';
+
+  is_deeply (
+    [ $pop_rs->get_column('artistid')->all ],
+    [ 1,2,3, map { $seq_pos + $_ } (1,2,3) ],
+    'Partially failed populate did not alter table contents'
+  );
 
 # test sequence detection from a different schema
   SKIP: {