Even cleaner way of handling returning (no column interrogation in storage)

diff --git a/lib/DBIx/Class/Row.pm b/lib/DBIx/Class/Row.pm
index 2777114..07cfe71 100644 (file)
--- a/lib/DBIx/Class/Row.pm
+++ b/lib/DBIx/Class/Row.pm
@@ -342,31 +342,48 @@ sub insert {
     $rollback_guard ||= $source->storage->txn_scope_guard
   }
 
+  ## PK::Auto
+  my %auto_pri;
+  my $auto_idx = 0;
+  for ($self->primary_columns) {
+    if (
+      not defined $self->get_column($_)
+        ||
+      (ref($self->get_column($_)) eq 'SCALAR')
+    ) {
+      my $col_info = $source->column_info($_);
+      $auto_pri{$_} = $auto_idx++ unless $col_info->{auto_nextval};   # auto_nextval's are pre-fetched in the storage
+    }
+  }
+
   MULTICREATE_DEBUG and do {
     no warnings 'uninitialized';
     warn "MC $self inserting (".join(', ', $self->get_columns).")\n";
   };
-  my $updated_cols = $source->storage->insert($source, { $self->get_columns });
+  my $updated_cols = $source->storage->insert(
+    $source,
+    { $self->get_columns },
+    keys %auto_pri
+      ? { returning => [ sort { $auto_pri{$a} <=> $auto_pri{$b} } keys %auto_pri ] }
+      : ()
+    ,
+  );
+
   foreach my $col (keys %$updated_cols) {
     $self->store_column($col, $updated_cols->{$col});
+    delete $auto_pri{$col};
   }
 
-  ## PK::Auto
-  my @auto_pri = grep {
-                  (not defined $self->get_column($_))
-                    ||
-                  (ref($self->get_column($_)) eq 'SCALAR')
-                 } $self->primary_columns;
-
-  if (@auto_pri) {
-    MULTICREATE_DEBUG and warn "MC $self fetching missing PKs ".join(', ', @auto_pri)."\n";
+  if (keys %auto_pri) {
+    my @missing = sort { $auto_pri{$a} <=> $auto_pri{$b} } keys %auto_pri;
+    MULTICREATE_DEBUG and warn "MC $self fetching missing PKs ".join(', ', @missing )."\n";
     my $storage = $self->result_source->storage;
     $self->throw_exception( "Missing primary key but Storage doesn't support last_insert_id" )
       unless $storage->can('last_insert_id');
-    my @ids = $storage->last_insert_id($self->result_source,@auto_pri);
+    my @ids = $storage->last_insert_id($self->result_source, @missing);
     $self->throw_exception( "Can't get last insert id" )
-      unless (@ids == @auto_pri);
-    $self->store_column($auto_pri[$_] => $ids[$_]) for 0 .. $#ids;
+      unless (@ids == @missing);
+    $self->store_column($missing[$_] => $ids[$_]) for 0 .. $#missing;
   }
 
   $self->{_dirty_columns} = {};

diff --git a/lib/DBIx/Class/Storage/DBI.pm b/lib/DBIx/Class/Storage/DBI.pm
index e746385..74d39bc 100644 (file)
--- a/lib/DBIx/Class/Storage/DBI.pm
+++ b/lib/DBIx/Class/Storage/DBI.pm
@@ -1363,20 +1363,17 @@ sub _execute {
     $self->dbh_do('_dbh_execute', @_);  # retry over disconnects
 }
 
-sub insert {
+sub _prefetch_insert_auto_nextvals {
   my ($self, $source, $to_insert) = @_;
 
-  my $ident = $source->from;
-  my $bind_attributes = $self->source_bind_attributes($source);
-
-  my $updated_cols = {};
+  my $upd = {};
 
   foreach my $col ( $source->columns ) {
     if ( !defined $to_insert->{$col} ) {
       my $col_info = $source->column_info($col);
 
       if ( $col_info->{auto_nextval} ) {
-        $updated_cols->{$col} = $to_insert->{$col} = $self->_sequence_fetch(
+        $upd->{$col} = $to_insert->{$col} = $self->_sequence_fetch(
           'nextval',
           $col_info->{sequence} ||=
             $self->_dbh_get_autoinc_seq($self->_get_dbh, $source, $col)
@@ -1385,6 +1382,17 @@ sub insert {
     }
   }
 
+  return $upd;
+}
+
+sub insert {
+  my $self = shift;
+  my ($source, $to_insert) = @_;
+
+  my $updated_cols = $self->_prefetch_insert_auto_nextvals (@_);
+
+  my $bind_attributes = $self->source_bind_attributes($source);
+
   $self->_execute('insert' => [], $source, $bind_attributes, $to_insert);
 
   return $updated_cols;

diff --git a/lib/DBIx/Class/Storage/DBI/InsertReturning.pm b/lib/DBIx/Class/Storage/DBI/InsertReturning.pm
index 1948a2f..f110cb5 100644 (file)
--- a/lib/DBIx/Class/Storage/DBI/InsertReturning.pm
+++ b/lib/DBIx/Class/Storage/DBI/InsertReturning.pm
@@ -6,10 +6,6 @@ use warnings;
 use base qw/DBIx::Class::Storage::DBI/;
 use mro 'c3';
 
-__PACKAGE__->mk_group_accessors(simple => qw/
-  _returning_cols
-/);
-
 =head1 NAME
 
 DBIx::Class::Storage::DBI::InsertReturning - Storage component for RDBMSes
@@ -19,91 +15,42 @@ supporting INSERT ... RETURNING
 
 Provides Auto-PK and
 L<is_auto_increment|DBIx::Class::ResultSource/is_auto_increment> support for
-databases supporting the C<INSERT ... RETURNING> syntax. Currently
-L<PostgreSQL|DBIx::Class::Storage::DBI::Pg> and
-L<Firebird|DBIx::Class::Storage::DBI::InterBase>.
+databases supporting the C<INSERT ... RETURNING> syntax.
 
 =cut
 
-sub _prep_for_execute {
+sub insert {
   my $self = shift;
-  my ($op, $extra_bind, $ident, $args) = @_;
-
-  if ($op eq 'insert') {
-    $self->_returning_cols([]);
-
-    my %pk;
-    @pk{$ident->primary_columns} = ();
-
-    my @auto_inc_cols = grep {
-      my $inserting = $args->[0]{$_};
-
-      ($ident->column_info($_)->{is_auto_increment}
-        || exists $pk{$_})
-      && (
-        (not defined $inserting)
-        ||
-        (ref $inserting eq 'SCALAR' && $$inserting =~ /^null\z/i)
-      )
-    } $ident->columns;
+  my ($source, $to_insert, $opts) = @_;
 
-    if (@auto_inc_cols) {
-      $args->[1]{returning} = \@auto_inc_cols;
+  return $self->next::method (@_) unless ($opts && $opts->{returning});
 
-      $self->_returning_cols->[0] = \@auto_inc_cols;
-    }
-  }
-
-  return $self->next::method(@_);
-}
+  my $updated_cols = $self->_prefetch_insert_auto_nextvals ($source, $to_insert);
 
-sub _execute {
-  my $self = shift;
-  my ($op) = @_;
+  my $bind_attributes = $self->source_bind_attributes($source);
+  my ($rv, $sth) = $self->_execute (insert => [], $source, $bind_attributes, $to_insert, $opts);
 
-  my ($rv, $sth, @bind) = $self->dbh_do($self->can('_dbh_execute'), @_);
+  if (my @ret_cols = @{$opts->{returning}}) {
 
-  if ($op eq 'insert' && $self->_returning_cols) {
-    local $@;
-    my (@returning_cols) = eval {
+    my @ret_vals = eval {
       local $SIG{__WARN__} = sub {};
-      $sth->fetchrow_array
+      my @r = $sth->fetchrow_array;
+      $sth->finish;
+      @r;
     };
-    $self->_returning_cols->[1] = \@returning_cols;
-    $sth->finish;
-  }
 
-  return wantarray ? ($rv, $sth, @bind) : $rv;
-}
+    my %ret;
+    @ret{@ret_cols} = @ret_vals if (@ret_vals);
 
-sub insert {
-  my $self = shift;
-
-  my $updated_cols = $self->next::method(@_);
-
-  if ($self->_returning_cols->[0]) {
-    my %returning_cols;
-    @returning_cols{ @{ $self->_returning_cols->[0] } } = @{ $self->_returning_cols->[1] };
-
-    $updated_cols = { %$updated_cols, %returning_cols };
+    $updated_cols = {
+      %$updated_cols,
+      %ret,
+    };
   }
 
   return $updated_cols;
 }
 
-sub last_insert_id {
-  my ($self, $source, @cols) = @_;
-  my @result;
-
-  my %returning_cols;
-  @returning_cols{ @{ $self->_returning_cols->[0] } } =
-    @{ $self->_returning_cols->[1] };
-
-  push @result, $returning_cols{$_} for @cols;
-
-  return @result;
-}
-
 =head1 AUTHOR
 
 See L<DBIx::Class/AUTHOR> and L<DBIx::Class/CONTRIBUTORS>