redo Pg auto-columns using INSERT RETURNING
Rafael Kitover [Mon, 22 Mar 2010 15:10:38 +0000 (15:10 +0000)]
lib/DBIx/Class/Storage/DBI/InterBase.pm
lib/DBIx/Class/Storage/DBI/Pg.pm
t/72pg.t
t/lib/DBICTest/Schema.pm
t/lib/DBICTest/Schema/TimestampPrimaryKey.pm [new file with mode: 0644]
t/lib/sqlite.sql

index 2a789b5..f8b31a2 100644 (file)
@@ -1,7 +1,5 @@
 package DBIx::Class::Storage::DBI::InterBase;
 
-# partly stolen from DBIx::Class::Storage::DBI::MSSQL
-
 use strict;
 use warnings;
 use base qw/DBIx::Class::Storage::DBI/;
index 92153ec..10e7c5b 100644 (file)
@@ -7,44 +7,124 @@ use base qw/DBIx::Class::Storage::DBI::MultiColumnIn/;
 use mro 'c3';
 
 use DBD::Pg qw(:pg_types);
+use Scope::Guard ();
+use Context::Preserve ();
 
 # Ask for a DBD::Pg with array support
 warn __PACKAGE__.": DBD::Pg 2.9.2 or greater is strongly recommended\n"
   if ($DBD::Pg::VERSION < 2.009002);  # pg uses (used?) version::qv()
 
+__PACKAGE__->mk_group_accessors(simple => qw/
+  _auto_cols
+/);
+
+sub _prep_for_execute {
+  my $self = shift;
+  my ($op, $extra_bind, $ident, $args) = @_;
+
+  if ($op eq 'insert') {
+    $self->_auto_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;
+
+    if (@auto_inc_cols) {
+      $args->[1]{returning} = \@auto_inc_cols;
+
+      $self->_auto_cols->[0] = \@auto_inc_cols;
+    }
+  }
+
+  return $self->next::method(@_);
+}
+
+sub _execute {
+  my $self = shift;
+  my ($op) = @_;
+
+  my ($rv, $sth, @bind) = $self->dbh_do($self->can('_dbh_execute'), @_);
+
+  if ($op eq 'insert' && $self->_auto_cols) {
+    local $@;
+    my (@auto_cols) = eval {
+      local $SIG{__WARN__} = sub {};
+      $sth->fetchrow_array
+    };
+    $self->_auto_cols->[1] = \@auto_cols;
+    $sth->finish;
+  }
+
+  return wantarray ? ($rv, $sth, @bind) : $rv;
+}
+
+
 sub with_deferred_fk_checks {
   my ($self, $sub) = @_;
 
-  $self->_get_dbh->do('SET CONSTRAINTS ALL DEFERRED');
-  $sub->();
+  my $txn_scope_guard = $self->txn_scope_guard;
+
+  $self->_do_query('SET CONSTRAINTS ALL DEFERRED');
+  
+  my $sg = Scope::Guard->new(sub {
+    $self->_do_query('SET CONSTRAINTS ALL IMMEDIATE');
+  });
+
+  return Context::Preserve::preserve_context(sub { $sub->() },
+    after => sub { $txn_scope_guard->commit });
 }
 
-sub last_insert_id {
-  my ($self,$source,@cols) = @_;
+sub insert {
+  my $self = shift;
 
-  my @values;
+  my $updated_cols = $self->next::method(@_);
 
-  for my $col (@cols) {
-    my $seq = ( $source->column_info($col)->{sequence} ||= $self->dbh_do('_dbh_get_autoinc_seq', $source, $col) )
-      or $self->throw_exception( sprintf(
-        'could not determine sequence for column %s.%s, please consider adding a schema-qualified sequence to its column info',
-          $source->name,
-          $col,
-      ));
+  if ($self->_auto_cols->[0]) {
+    my %auto_cols;
+    @auto_cols{ @{ $self->_auto_cols->[0] } } = @{ $self->_auto_cols->[1] };
 
-    push @values, $self->_dbh_last_insert_id ($self->_dbh, $seq);
+    $updated_cols = { %$updated_cols, %auto_cols };
   }
 
-  return @values;
+  return $updated_cols;
 }
 
-# there seems to be absolutely no reason to have this as a separate method,
-# but leaving intact in case someone is already overriding it
-sub _dbh_last_insert_id {
-  my ($self, $dbh, $seq) = @_;
-  $dbh->last_insert_id(undef, undef, undef, undef, {sequence => $seq});
+sub last_insert_id {
+  my ($self, $source, @cols) = @_;
+  my @result;
+
+  my %auto_cols;
+  @auto_cols{ @{ $self->_auto_cols->[0] } } =
+    @{ $self->_auto_cols->[1] };
+
+  push @result, $auto_cols{$_} for @cols;
+
+  return @result;
 }
 
+sub _sequence_fetch {
+  my ($self, $function, $sequence) = @_;
+
+  $self->throw_exception('No sequence to fetch') unless $sequence;
+  
+  my ($val) = $self->_get_dbh->selectrow_array(
+    sprintf "select $function('%s')",
+      $sequence
+  );
+
+  return $val;
+} 
 
 sub _dbh_get_autoinc_seq {
   my ($self, $dbh, $source, $col) = @_;
@@ -155,12 +235,6 @@ sub bind_attribute_by_data_type {
   }
 }
 
-sub _sequence_fetch {
-  my ( $self, $type, $seq ) = @_;
-  my ($id) = $self->_get_dbh->selectrow_array("SELECT nextval('${seq}')");
-  return $id;
-}
-
 sub _svp_begin {
     my ($self, $name) = @_;
 
index 5a4d162..b306295 100644 (file)
--- a/t/72pg.t
+++ b/t/72pg.t
@@ -257,12 +257,19 @@ SKIP: {
 $schema->source("SequenceTest")->name("dbic_t_schema.sequence_test");
 for (1..5) {
     my $st = $schema->resultset('SequenceTest')->create({ name => 'foo' });
-    is($st->pkid1, $_, "Oracle Auto-PK without trigger: First primary key");
-    is($st->pkid2, $_ + 9, "Oracle Auto-PK without trigger: Second primary key");
-    is($st->nonpkid, $_ + 19, "Oracle Auto-PK without trigger: Non-primary key");
+    is($st->pkid1, $_, "Auto-PK for sequence without default: First primary key");
+    is($st->pkid2, $_ + 9, "Auto-PK for sequence without default: Second primary key");
+    is($st->nonpkid, $_ + 19, "Auto-PK for sequence without default: Non-primary key");
 }
 my $st = $schema->resultset('SequenceTest')->create({ name => 'foo', pkid1 => 55 });
-is($st->pkid1, 55, "Oracle Auto-PK without trigger: First primary key set manually");
+is($st->pkid1, 55, "Auto-PK for sequence without default: First primary key set manually");
+
+
+######## test non-integer non-serial auto-pk
+
+$schema->source('TimestampPrimaryKey')->name('dbic_t_schema.timestamp_primary_key_test');
+my $row = $schema->resultset('TimestampPrimaryKey')->create({});
+ok $row->id;
 
 done_testing;
 
@@ -296,6 +303,12 @@ EOS
 
       $dbh->do("CREATE SCHEMA dbic_t_schema");
       $dbh->do("CREATE TABLE dbic_t_schema.artist $std_artist_table");
+
+      $dbh->do(<<EOS);
+CREATE TABLE dbic_t_schema.timestamp_primary_key_test (
+    id timestamp default current_timestamp
+)
+EOS
       $dbh->do(<<EOS);
 CREATE TABLE dbic_t_schema.sequence_test (
     pkid1 integer
index a3e4484..f326bd4 100644 (file)
@@ -22,6 +22,7 @@ __PACKAGE__->load_classes(qw/
   Year1999CDs
   CustomSql
   Money
+  TimestampPrimaryKey
   /,
   { 'DBICTest::Schema' => [qw/
     LinerNotes
diff --git a/t/lib/DBICTest/Schema/TimestampPrimaryKey.pm b/t/lib/DBICTest/Schema/TimestampPrimaryKey.pm
new file mode 100644 (file)
index 0000000..9c35db2
--- /dev/null
@@ -0,0 +1,17 @@
+package # hide from PAUSE 
+    DBICTest::Schema::TimestampPrimaryKey;
+
+use base qw/DBICTest::BaseResult/;
+
+__PACKAGE__->table('timestamp_primary_key_test');
+
+__PACKAGE__->add_columns(
+  'id' => {
+    data_type => 'timestamp',
+    default_value => \'current_timestamp',
+  },
+);
+
+__PACKAGE__->set_primary_key('id');
+
+1;
index ba1ffae..741375d 100644 (file)
@@ -1,7 +1,8 @@
 -- 
 -- Created by SQL::Translator::Producer::SQLite
--- Created on Tue Mar 16 16:49:23 2010
+-- Created on Mon Mar 22 11:08:33 2010
 -- 
+;
 
 --
 -- Table: artist
@@ -168,6 +169,14 @@ CREATE TABLE serialized (
 );
 
 --
+-- Table: timestamp_primary_key_test
+--
+CREATE TABLE timestamp_primary_key_test (
+  id timestamp NOT NULL DEFAULT current_timestamp,
+  PRIMARY KEY (id)
+);
+
+--
 -- Table: treelike
 --
 CREATE TABLE treelike (
@@ -447,4 +456,4 @@ CREATE INDEX fourkeys_to_twokeys_idx_t_artist_t_cd ON fourkeys_to_twokeys (t_art
 -- View: year2000cds
 --
 CREATE VIEW year2000cds AS
-    SELECT cdid, artist, title, year, genreid, single_track FROM cd WHERE year = "2000";
+    SELECT cdid, artist, title, year, genreid, single_track FROM cd WHERE year = "2000"
\ No newline at end of file