Merge 'trunk' into 'sybase'
Rafael Kitover [Sun, 16 Aug 2009 01:59:42 +0000 (01:59 +0000)]
r9155@hlagh (orig r7288):  caelum | 2009-08-10 04:13:19 -0400
make _determine_driver more reentrant
r9161@hlagh (orig r7294):  michaelr | 2009-08-10 16:40:33 -0400
Added exception when resultset called without an argument

r9162@hlagh (orig r7295):  andyg | 2009-08-10 18:34:13 -0400
Add failing test for RT 47779, group_by as a scalar ref
r9165@hlagh (orig r7298):  ribasushi | 2009-08-11 03:52:03 -0400
Extra intro pod
r9166@hlagh (orig r7299):  mo | 2009-08-11 07:20:37 -0400
removed TODO test
r9167@hlagh (orig r7300):  ribasushi | 2009-08-11 08:16:28 -0400
Sanify group_by handling in complex prefetch rewrites
r9168@hlagh (orig r7301):  ribasushi | 2009-08-11 11:52:49 -0400
cleanup
r9169@hlagh (orig r7302):  ribasushi | 2009-08-11 13:40:59 -0400
Whitespace
r9170@hlagh (orig r7303):  ribasushi | 2009-08-11 14:00:11 -0400
Fix an obscure regression when inserting an object with a serialize-deflating column set
r9177@hlagh (orig r7311):  ribasushi | 2009-08-12 10:11:24 -0400
Remove needless inflate in Ordered
r9178@hlagh (orig r7312):  ribasushi | 2009-08-12 10:13:48 -0400
Remove leftovers from frew's tests
r9179@hlagh (orig r7313):  ribasushi | 2009-08-12 10:16:08 -0400
Grrrr
r9182@hlagh (orig r7314):  ribasushi | 2009-08-13 01:40:44 -0400
Caelum was right to make _get_dbh private - reverting (and some code refactoring)
r9183@hlagh (orig r7315):  ribasushi | 2009-08-13 01:41:43 -0400
Add a db/txn_do retry debugger (interesting results)
r9184@hlagh (orig r7316):  ribasushi | 2009-08-13 01:42:51 -0400
Adjust the storage DESTROY and the tests to accomodate the new global RaiseError=1
r9185@hlagh (orig r7317):  ribasushi | 2009-08-13 02:12:08 -0400
Last bit

32 files changed:
Changes
lib/DBIx/Class/Manual/Intro.pod
lib/DBIx/Class/Ordered.pm
lib/DBIx/Class/ResultSet.pm
lib/DBIx/Class/Row.pm
lib/DBIx/Class/Schema.pm
lib/DBIx/Class/Storage/DBI.pm
lib/DBIx/Class/Storage/DBI/MSSQL.pm
lib/DBIx/Class/Storage/DBI/ODBC.pm
lib/DBIx/Class/Storage/DBI/ODBC/Microsoft_SQL_Server.pm
lib/DBIx/Class/Storage/DBI/Oracle.pm
lib/DBIx/Class/Storage/DBI/Oracle/Generic.pm
lib/DBIx/Class/Storage/DBI/Pg.pm
lib/DBIx/Class/Storage/DBI/Sybase.pm
lib/DBIx/Class/Storage/DBI/Sybase/Base.pm
lib/DBIx/Class/Storage/DBI/Sybase/Microsoft_SQL_Server.pm
lib/DBIx/Class/Storage/DBI/mysql.pm
t/60core.t
t/745db2.t
t/746db2_400.t
t/92storage.t
t/92storage_ping_count.t
t/93nobindvars.t
t/create/set_column.t [deleted file]
t/inflate/core.t
t/inflate/serialize.t
t/lib/DBICTest.pm
t/lib/DBICTest/Schema/Serialized.pm
t/lib/DBICTest/Schema/Track.pm
t/lib/sqlite.sql
t/prefetch/double_prefetch.t
t/prefetch/grouped.t

diff --git a/Changes b/Changes
index b9e7b1b..818684f 100644 (file)
--- a/Changes
+++ b/Changes
@@ -39,11 +39,9 @@ Revision history for DBIx::Class
             prefetch
         - Massive optimization of the DBI storage layer - reduce the
           amount of connected() calls
-        - New ::Storage::DBI method last_dbh() - it will still return a
-          newly connected $dbh if we start unconnected, but will not ping
-          the server on every invocation unlike dbh()
         - Some fixes of multi-create corner cases
         - Multiple POD improvements
+        - Added exception when resultset is called without an argument
 
 0.08108 2009-07-05 23:15:00 (UTC)
         - Fixed the has_many prefetch with limit/group deficiency -
index dd37d0e..3a275e6 100644 (file)
@@ -200,8 +200,12 @@ many options.
 
 =head2 Connecting
 
-To connect to your Schema, you need to provide the connection details.  The
-arguments are the same as for L<DBI/connect>:
+To connect to your Schema, you need to provide the connection details or a
+database handle.
+
+=head3 Via connection details
+
+The arguments are the same as for L<DBI/connect>:
 
   my $schema = My::Schema->connect('dbi:SQLite:/home/me/myapp/my.db');
 
@@ -227,6 +231,16 @@ a special fifth argument to connect:
 See L<DBIx::Class::Schema::Storage::DBI/connect_info> for more information about
 this and other special C<connect>-time options.
 
+=head3 Via a database handle
+
+The supplied coderef is expected to return a single connected database handle
+(e.g. a L<DBI> C<$dbh>)
+
+  my $schema = My::Schema->connect (
+    sub { Some::DBH::Factory->connect },
+    \%extra_attrs,
+  );
+
 =head2 Basic usage
 
 Once you've defined the basic classes, either manually or using
index ed726de..59162bb 100644 (file)
@@ -564,7 +564,7 @@ sub update {
     # these steps are necessary to keep the external appearance of
     # ->update($upd) so that other things overloading update() will
     # work properly
-    my %original_values = $self->get_inflated_columns;
+    my %original_values = $self->get_columns;
     my %existing_changes = $self->get_dirty_columns;
 
     # See if any of the *supplied* changes would affect the ordering
index e2dab82..77732e0 100644 (file)
@@ -1266,8 +1266,8 @@ sub _count_subq_rs {
   # extra selectors do not go in the subquery and there is no point of ordering it
   delete $sub_attrs->{$_} for qw/collapse select _prefetch_select as order_by/;
 
-  # if we prefetch, we group_by primary keys only as this is what we would get out of the rs via ->next/->all
-  # clobber old group_by regardless
+  # if we prefetch, we group_by primary keys only as this is what we would get out
+  # of the rs via ->next/->all. We DO WANT to clobber old group_by regardless
   if ( keys %{$attrs->{collapse}} ) {
     $sub_attrs->{group_by} = [ map { "$attrs->{alias}.$_" } ($rsrc->primary_columns) ]
   }
@@ -1509,7 +1509,8 @@ sub _rs_update_delete {
       if (my $g = $attrs->{group_by}) {
         my @current_group_by = map
           { $_ =~ /\./ ? $_ : "$attrs->{alias}.$_" }
-          (ref $g eq 'ARRAY' ? @$g : $g );
+          @$g
+        ;
 
         if (
           join ("\x00", sort @current_group_by)
@@ -2885,7 +2886,7 @@ sub _resolved_attrs {
     );
   }
 
-  if ($attrs->{group_by} and ! ref $attrs->{group_by}) {
+  if ($attrs->{group_by} and ref $attrs->{group_by} ne 'ARRAY') {
     $attrs->{group_by} = [ $attrs->{group_by} ];
   }
 
index 89f1de3..5d6285f 100644 (file)
@@ -354,18 +354,17 @@ sub insert {
   $self->{related_resultsets} = {};
 
   foreach my $relname (keys %related_stuff) {
-    my $rel_obj = $related_stuff{$relname};
-    my @cands;
-    if (Scalar::Util::blessed($rel_obj)
-          && $rel_obj->isa('DBIx::Class::Row'))
-    {
-      @cands = ($rel_obj);
-    }
-    elsif (ref $rel_obj eq 'ARRAY') {
-      @cands = @$rel_obj;
-    }
+    next unless $source->has_relationship ($relname);
+
+    my @cands = ref $related_stuff{$relname} eq 'ARRAY'
+      ? @{$related_stuff{$relname}}
+      : $related_stuff{$relname}
+    ;
 
-    if (@cands) {
+    if (@cands
+          && Scalar::Util::blessed($cands[0])
+            && $cands[0]->isa('DBIx::Class::Row')
+    ) {
       my $reverse = $source->reverse_relationship_info($relname);
       foreach my $obj (@cands) {
         $obj->set_from_related($_, $self) for keys %$reverse;
index 57b4b99..a7080e2 100644 (file)
@@ -543,6 +543,8 @@ name.
 
 sub resultset {
   my ($self, $moniker) = @_;
+  $self->throw_exception('resultset() expects a source name')
+    unless defined $moniker;
   return $self->source($moniker)->resultset;
 }
 
index 341d9f9..a872e95 100644 (file)
@@ -452,7 +452,11 @@ sub connect_info {
 }
 
 sub _default_dbi_connect_attributes {
-  return { AutoCommit => 1 };
+  return {
+    AutoCommit => 1,
+    RaiseError => 1,
+    PrintError => 0,
+  };
 }
 
 =head2 on_connect_do
@@ -551,6 +555,7 @@ sub dbh_do {
     }
   };
 
+  # ->connected might unset $@ - copy
   my $exception = $@;
   if(!$exception) { return $want_array ? @result : $result[0] }
 
@@ -558,6 +563,8 @@ sub dbh_do {
 
   # We were not connected - reconnect and retry, but let any
   #  exception fall right through this time
+  carp "Retrying $code after catching disconnected exception: $exception"
+    if $ENV{DBIC_DBIRETRY_DEBUG};
   $self->_populate_dbh;
   $self->$code($self->_dbh, @_);
 }
@@ -598,6 +605,7 @@ sub txn_do {
       $self->txn_commit;
     };
 
+    # ->connected might unset $@ - copy
     my $exception = $@;
     if(!$exception) { return $want_array ? @result : $result[0] }
 
@@ -619,6 +627,8 @@ sub txn_do {
 
     # We were not connected, and was first try - reconnect and retry
     # via the while loop
+    carp "Retrying $coderef after catching disconnected exception: $exception"
+      if $ENV{DBIC_DBIRETRY_DEBUG};
     $self->_populate_dbh;
   }
 }
@@ -688,22 +698,32 @@ answering, etc.) This method is used internally by L</dbh>.
 =cut
 
 sub connected {
-  my ($self) = @_;
+  my $self = shift;
+  return 0 unless $self->_seems_connected;
 
-  if(my $dbh = $self->_dbh) {
-      if(defined $self->_conn_tid && $self->_conn_tid != threads->tid) {
-          $self->_dbh(undef);
-          $self->{_dbh_gen}++;
-          return;
-      }
-      else {
-          $self->_verify_pid;
-          return 0 if !$self->_dbh;
-      }
-      return ($dbh->FETCH('Active') && $self->_ping);
+  #be on the safe side
+  local $self->_dbh->{RaiseError} = 1;
+
+  return $self->_ping;
+}
+
+sub _seems_connected {
+  my $self = shift;
+
+  my $dbh = $self->_dbh
+    or return 0;
+
+  if(defined $self->_conn_tid && $self->_conn_tid != threads->tid) {
+    $self->_dbh(undef);
+    $self->{_dbh_gen}++;
+    return 0;
+  }
+  else {
+    $self->_verify_pid;
+    return 0 if !$self->_dbh;
   }
 
-  return 0;
+  return $dbh->FETCH('Active');
 }
 
 sub _ping {
@@ -740,7 +760,9 @@ sub ensure_connected {
 
 Returns a C<$dbh> - a data base handle of class L<DBI>. The returned handle
 is guaranteed to be healthy by implicitly calling L</connected>, and if
-necessary performing a reconnection before returning.
+necessary performing a reconnection before returning. Keep in mind that this
+is very B<expensive> on some database engines. Consider using L<dbh_do>
+instead.
 
 =cut
 
@@ -755,17 +777,8 @@ sub dbh {
   return $self->_dbh;
 }
 
-=head2 last_dbh
-
-This returns the B<last> available C<$dbh> if any, or attempts to
-connect and returns the resulting handle. This method differs from
-L</dbh> by not validating if a preexisting handle is still healthy
-via L</connected>. Make sure you take appropriate precautions
-when using this method, as the C<$dbh> may be useless at this point.
-
-=cut
-
-sub last_dbh {
+# this is the internal "get dbh or connect (don't check)" method
+sub _get_dbh {
   my $self = shift;
   $self->_populate_dbh unless $self->_dbh;
   return $self->_dbh;
@@ -777,7 +790,7 @@ sub _sql_maker_args {
     return (
       bindtype=>'columns',
       array_datatypes => 1,
-      limit_dialect => $self->last_dbh,
+      limit_dialect => $self->_get_dbh,
       %{$self->_sql_maker_opts}
     );
 }
@@ -798,6 +811,7 @@ sub _populate_dbh {
   my ($self) = @_;
 
   my @info = @{$self->_dbi_connect_info || []};
+  $self->_dbh(undef); # in case ->connected failed we might get sent here
   $self->_dbh($self->_connect(@info));
 
   $self->_conn_pid($$);
@@ -1250,7 +1264,7 @@ sub insert {
         $updated_cols->{$col} = $to_insert->{$col} = $self->_sequence_fetch(
           'nextval',
           $col_info->{sequence} ||
-            $self->_dbh_get_autoinc_seq($self->last_dbh, $source)
+            $self->_dbh_get_autoinc_seq($self->_get_dbh, $source)
         );
       }
     }
@@ -2060,7 +2074,7 @@ Returns the database driver name.
 
 =cut
 
-sub sqlt_type { shift->last_dbh->{Driver}->{Name} }
+sub sqlt_type { shift->_get_dbh->{Driver}->{Name} }
 
 =head2 bind_attribute_by_data_type
 
@@ -2306,8 +2320,6 @@ See L<SQL::Translator/METHODS> for a list of values for C<$sqlt_args>.
 
 sub deployment_statements {
   my ($self, $schema, $type, $version, $dir, $sqltargs) = @_;
-  # Need to be connected to get the correct sqlt_type
-  $self->last_dbh() unless $type;
   $type ||= $self->sqlt_type;
   $version ||= $schema->schema_version || '1.x';
   $dir ||= './';
@@ -2352,10 +2364,9 @@ sub deploy {
     return if $line =~ /^\s+$/; # skip whitespace only
     $self->_query_start($line);
     eval {
-      # a previous error may invalidate $dbh - thus we need to use dbh()
-      # to guarantee a healthy $dbh (this is temporary until we get
-      # proper error handling on deploy() )
-      $self->dbh->do($line);
+      # do a dbh_do cycle here, as we need some error checking in
+      # place (even though we will ignore errors)
+      $self->dbh_do (sub { $_[1]->do($line) });
     };
     if ($@) {
       carp qq{$@ (running "${line}")};
@@ -2384,7 +2395,7 @@ Returns the datetime parser class
 sub datetime_parser {
   my $self = shift;
   return $self->{datetime_parser} ||= do {
-    $self->last_dbh;
+    $self->_populate_dbh unless $self->_dbh;
     $self->build_datetime_parser(@_);
   };
 }
@@ -2455,8 +2466,13 @@ sub lag_behind_master {
 
 sub DESTROY {
   my $self = shift;
-  return if !$self->_dbh;
-  $self->_verify_pid;
+  $self->_verify_pid if $self->_dbh;
+
+  # some databases need this to stop spewing warnings
+  if (my $dbh = $self->_dbh) {
+    eval { $dbh->disconnect };
+  }
+
   $self->_dbh(undef);
 }
 
index 674d458..b0da553 100644 (file)
@@ -30,14 +30,14 @@ sub insert_bulk {
 
   if ($identity_insert) {
     my $table = $source->from;
-    $self->last_dbh->do("SET IDENTITY_INSERT $table ON");
+    $self->_get_dbh->do("SET IDENTITY_INSERT $table ON");
   }
 
   $self->next::method(@_);
 
   if ($identity_insert) {
     my $table = $source->from;
-    $self->last_dbh->do("SET IDENTITY_INSERT $table OFF");
+    $self->_get_dbh->do("SET IDENTITY_INSERT $table OFF");
   }
 }
 
@@ -68,7 +68,7 @@ sub insert {
     grep { not exists $to_insert->{$_} } (@pk_guids, @auto_guids);
 
   for my $guid_col (@get_guids_for) {
-    my ($new_guid) = $self->last_dbh->selectrow_array('SELECT NEWID()');
+    my ($new_guid) = $self->_get_dbh->selectrow_array('SELECT NEWID()');
     $updated_cols->{$guid_col} = $to_insert->{$guid_col} = $new_guid;
   }
 
@@ -145,7 +145,7 @@ sub last_insert_id { shift->_identity }
 sub _svp_begin {
   my ($self, $name) = @_;
 
-  $self->last_dbh->do("SAVE TRANSACTION $name");
+  $self->_get_dbh->do("SAVE TRANSACTION $name");
 }
 
 # A new SAVE TRANSACTION with the same name releases the previous one.
@@ -154,7 +154,7 @@ sub _svp_release { 1 }
 sub _svp_rollback {
   my ($self, $name) = @_;
 
-  $self->last_dbh->do("ROLLBACK TRANSACTION $name");
+  $self->_get_dbh->do("ROLLBACK TRANSACTION $name");
 }
 
 sub build_datetime_parser {
index 7c3b739..d9b810a 100644 (file)
@@ -8,7 +8,7 @@ use mro 'c3';
 sub _rebless {
     my ($self) = @_;
 
-    my $dbtype = eval { $self->last_dbh->get_info(17) };
+    my $dbtype = eval { $self->_get_dbh->get_info(17) };
 
     unless ( $@ ) {
         # Translate the backend name into a perl identifier
index 77c20ad..5279502 100644 (file)
@@ -137,7 +137,7 @@ sub connect_call_use_server_cursors {
   my $self            = shift;
   my $sql_rowset_size = shift || 2;
 
-  $self->_dbh->{odbc_SQL_ROWSET_SIZE} = $sql_rowset_size;
+  $self->_get_dbh->{odbc_SQL_ROWSET_SIZE} = $sql_rowset_size;
 }
 
 =head2 connect_call_use_MARS
@@ -165,9 +165,9 @@ sub connect_call_use_MARS {
 
   if ($dsn !~ /MARS_Connection=/) {
     $self->_dbi_connect_info->[0] = "$dsn;MARS_Connection=Yes";
-    my $connected = defined $self->_dbh;
+    my $was_connected = defined $self->_dbh;
     $self->disconnect;
-    $self->ensure_connected if $connected;
+    $self->ensure_connected if $was_connected;
   }
 }
 
index c0edb9a..7a49b50 100644 (file)
@@ -9,7 +9,7 @@ use mro 'c3';
 sub _rebless {
     my ($self) = @_;
 
-    my $version = eval { $self->last_dbh->get_info(18); };
+    my $version = eval { $self->_get_dbh->get_info(18); };
 
     if ( !$@ ) {
         my ($major, $minor, $patchlevel) = split(/\./, $version);
index 8b09f11..6998e86 100644 (file)
@@ -76,7 +76,7 @@ sub _dbh_get_autoinc_seq {
 
 sub _sequence_fetch {
   my ( $self, $type, $seq ) = @_;
-  my ($id) = $self->last_dbh->selectrow_array("SELECT ${seq}.${type} FROM DUAL");
+  my ($id) = $self->_get_dbh->selectrow_array("SELECT ${seq}.${type} FROM DUAL");
   return $id;
 }
 
@@ -206,6 +206,12 @@ sub connect_call_datetime_setup {
 "alter session set nls_timestamp_tz_format='$timestamp_tz_format'");
 }
 
+sub _svp_begin {
+    my ($self, $name) = @_;
+
+    $self->_get_dbh->do("SAVEPOINT $name");
+}
+
 =head2 source_bind_attributes
 
 Handle LOB types in Oracle.  Under a certain size (4k?), you can get away
@@ -263,7 +269,7 @@ sub _svp_release { 1 }
 sub _svp_rollback {
     my ($self, $name) = @_;
 
-    $self->last_dbh->do("ROLLBACK TO SAVEPOINT $name")
+    $self->_get_dbh->do("ROLLBACK TO SAVEPOINT $name")
 }
 
 =head1 AUTHOR
index 91418a4..a664808 100644 (file)
@@ -15,7 +15,7 @@ warn "DBD::Pg 2.9.2 or greater is strongly recommended\n"
 sub with_deferred_fk_checks {
   my ($self, $sub) = @_;
 
-  $self->last_dbh->do('SET CONSTRAINTS ALL DEFERRED');
+  $self->_get_dbh->do('SET CONSTRAINTS ALL DEFERRED');
   $sub->();
 }
 
@@ -90,26 +90,26 @@ sub bind_attribute_by_data_type {
 
 sub _sequence_fetch {
   my ( $self, $type, $seq ) = @_;
-  my ($id) = $self->last_dbh->selectrow_array("SELECT nextval('${seq}')");
+  my ($id) = $self->_get_dbh->selectrow_array("SELECT nextval('${seq}')");
   return $id;
 }
 
 sub _svp_begin {
     my ($self, $name) = @_;
 
-    $self->last_dbh->pg_savepoint($name);
+    $self->_get_dbh->pg_savepoint($name);
 }
 
 sub _svp_release {
     my ($self, $name) = @_;
 
-    $self->last_dbh->pg_release($name);
+    $self->_get_dbh->pg_release($name);
 }
 
 sub _svp_rollback {
     my ($self, $name) = @_;
 
-    $self->last_dbh->pg_rollback_to($name);
+    $self->_get_dbh->pg_rollback_to($name);
 }
 
 1;
index 7995334..7e68b38 100644 (file)
@@ -48,7 +48,7 @@ sub _rebless {
 
   if (ref($self) eq 'DBIx::Class::Storage::DBI::Sybase') {
     my $dbtype = eval {
-      @{$self->last_dbh->selectrow_arrayref(qq{sp_server_info \@attribute_id=1})}[2]
+      @{$self->_get_dbh->selectrow_arrayref(qq{sp_server_info \@attribute_id=1})}[2]
     } || '';
 
     my $exception = $@;
index 4681d64..08b5ca4 100644 (file)
@@ -44,7 +44,7 @@ errors, see L</placeholders_with_type_conversion_supported>.
 
 sub placeholders_supported {
   my $self = shift;
-  my $dbh  = $self->last_dbh;
+  my $dbh  = $self->_get_dbh;
 
   return eval {
 # There's also $dbh->{syb_dynamic_supported} but it can be inaccurate for this
index 5c3fb4c..8992356 100644 (file)
@@ -11,7 +11,7 @@ use mro 'c3';
 
 sub _rebless {
   my $self = shift;
-  my $dbh  = $self->last_dbh;
+  my $dbh  = $self->_get_dbh;
 
   if (not $self->placeholders_with_type_conversion_supported) {
     bless $self,
index 89c8f89..6224d53 100644 (file)
@@ -40,28 +40,28 @@ sub sqlt_type {
 sub _svp_begin {
     my ($self, $name) = @_;
 
-    $self->last_dbh->do("SAVEPOINT $name");
+    $self->_get_dbh->do("SAVEPOINT $name");
 }
 
 sub _svp_release {
     my ($self, $name) = @_;
 
-    $self->last_dbh->do("RELEASE SAVEPOINT $name");
+    $self->_get_dbh->do("RELEASE SAVEPOINT $name");
 }
 
 sub _svp_rollback {
     my ($self, $name) = @_;
 
-    $self->last_dbh->do("ROLLBACK TO SAVEPOINT $name")
+    $self->_get_dbh->do("ROLLBACK TO SAVEPOINT $name")
 }
 
 sub is_replicating {
-    my $status = shift->last_dbh->selectrow_hashref('show slave status');
+    my $status = shift->_get_dbh->selectrow_hashref('show slave status');
     return ($status->{Slave_IO_Running} eq 'Yes') && ($status->{Slave_SQL_Running} eq 'Yes');
 }
 
 sub lag_behind_master {
-    return shift->last_dbh->selectrow_hashref('show slave status')->{Seconds_Behind_Master};
+    return shift->_get_dbh->selectrow_hashref('show slave status')->{Seconds_Behind_Master};
 }
 
 # MySql can not do subquery update/deletes, only way is slow per-row operations.
index c494b74..d430398 100644 (file)
@@ -452,4 +452,6 @@ SKIP: {
     }
 }
 
+throws_ok { $schema->resultset} qr/resultset\(\) expects a source name/, 'resultset with no argument throws exception';
+
 done_testing;
index 3f635f8..3ba8579 100644 (file)
@@ -85,5 +85,6 @@ is_deeply($type_info, $test_type_info, 'columns_info_for - column data types');
 
 # clean up our mess
 END {
+    my $dbh = eval { $schema->storage->_dbh };
     $dbh->do("DROP TABLE artist") if $dbh;
 }
index 21c72df..359c13e 100644 (file)
@@ -82,6 +82,6 @@ is_deeply($type_info, $test_type_info, 'columns_info_for - column data types');
 
 # clean up our mess
 END {
+    my $dbh = eval { $schema->storage->_dbh };
     $dbh->do("DROP TABLE artist") if $dbh;
 }
-
index 94bdfd3..c8a0bba 100644 (file)
@@ -93,6 +93,7 @@ my $invocations = {
           'bar',
           undef,
           {
+            %{$storage->_default_dbi_connect_attributes || {} },
             PrintError => 0,
             AutoCommit => 1,
           },
@@ -122,8 +123,8 @@ my $invocations = {
       args => [
           {
             on_connect_do => [qw/a b c/],
-            PrintError => 0,
-            AutoCommit => 1,
+            PrintError => 1,
+            AutoCommit => 0,
             on_disconnect_do => [qw/d e f/],
             user => 'bar',
             dsn => 'foo',
@@ -138,8 +139,9 @@ my $invocations = {
           'bar',
           undef,
           {
-            PrintError => 0,
-            AutoCommit => 1,
+            %{$storage->_default_dbi_connect_attributes || {} },
+            PrintError => 1,
+            AutoCommit => 0,
           },
       ],
   },
index 9dddd64..07659cb 100644 (file)
@@ -22,15 +22,13 @@ my $ping_count = 0;
 }
 
 
-# We do not count pings during deploy() because of the flux
-# around sqlt. Eventually there should be no pings at all
+# measure pings around deploy() separately
 my $schema = DBICTest->init_schema( sqlite_use_file => 1, no_populate => 1 );
 
-TODO: {
-  local $TODO = 'Unable to fix before proper deploy() error handling';
-  is ($ping_count, 0, 'no _ping() calls during deploy');
-  $ping_count = 0;
-}
+is ($ping_count, 0, 'no _ping() calls during deploy');
+$ping_count = 0;
+
+
 
 DBICTest->populate_schema ($schema);
 
index ea77526..e6ee0eb 100644 (file)
@@ -65,5 +65,6 @@ is( $it->next, undef, "next past end of resultset ok" );
 
 # clean up our mess
 END {
+    my $dbh = eval { $schema->storage->_dbh };
     $dbh->do("DROP TABLE artist") if $dbh;
 }
diff --git a/t/create/set_column.t b/t/create/set_column.t
deleted file mode 100644 (file)
index 9a2edd2..0000000
+++ /dev/null
@@ -1,45 +0,0 @@
-use warnings;
-use strict;
-
-use Test::More;
-use Test::Exception;
-
-use lib qw(t/lib);
-use DBICTest;
-
-my $schema = DBICTest->init_schema();
-
-TODO: {
-    local $TODO = 'call accessors when calling create() or update()';
-
-    my $row =
-      $schema->resultset('Track')->new_result( { title => 'foo', cd => 1 } );
-    $row->increment(1);
-    $row->insert;
-    is( $row->increment, 2 );
-
-    $row =
-      $schema->resultset('Track')
-      ->create( { title => 'bar', cd => 1, increment => 1 } );
-    is( $row->increment, 2 );
-
-    # $row isa DBICTest::Schema::Track
-    $row->get_from_storage;
-    is( $row->increment, 2 );
-
-    $row->update( { increment => 3 } );
-    $row->get_from_storage;
-    is( $row->increment, 4 );
-
-    $row->increment(3);
-    $row->get_from_storage;
-    is( $row->increment, 4 );
-
-    throws_ok (sub {
-        $row =
-          $schema->resultset('Track')
-          ->create( { title => 'bar', cd => 2, set_increment => 1 } );
-    }, qr/no such column/i);
-}
-
-done_testing;
index cb5bcaf..b98801e 100644 (file)
@@ -1,7 +1,8 @@
 use strict;
-use warnings;  
+use warnings;
 
 use Test::More;
+use Test::Exception;
 use lib qw(t/lib);
 use DBICTest;
 
@@ -10,8 +11,6 @@ my $schema = DBICTest->init_schema();
 eval { require DateTime };
 plan skip_all => "Need DateTime for inflation tests" if $@;
 
-plan tests => 22;
-
 $schema->class('CD') ->inflate_column( 'year',
     { inflate => sub { DateTime->new( year => shift ) },
       deflate => sub { shift->year } }
@@ -54,10 +53,10 @@ eval { $cd->set_inflated_column('year', $now) };
 ok(!$@, 'set_inflated_column with DateTime object');
 $cd->update;
 
-$cd = $schema->resultset("CD")->find(3);                 
+$cd = $schema->resultset("CD")->find(3);
 is( $cd->year->year, $now->year, 'deflate ok' );
 
-$cd = $schema->resultset("CD")->find(3);                 
+$cd = $schema->resultset("CD")->find(3);
 my $before_year = $cd->year->year;
 eval { $cd->set_inflated_column('year', \'year + 1') };
 ok(!$@, 'set_inflated_column to "year + 1"');
@@ -66,18 +65,17 @@ $cd->update;
 TODO: {
   local $TODO = 'this was left in without a TODO - should it work?';
 
-  eval {
+  lives_ok (sub {
     $cd->store_inflated_column('year', \'year + 1');
     is_deeply( $cd->year, \'year + 1', 'deflate ok' );
-  };
-  ok(!$@, 'store_inflated_column to "year + 1"');
+  }, 'store_inflated_column to "year + 1"');
 }
 
-$cd = $schema->resultset("CD")->find(3);                 
+$cd = $schema->resultset("CD")->find(3);
 is( $cd->year->year, $before_year+1, 'deflate ok' );
 
 # store_inflated_column test
-$cd = $schema->resultset("CD")->find(3);                 
+$cd = $schema->resultset("CD")->find(3);
 eval { $cd->store_inflated_column('year', $now) };
 ok(!$@, 'store_inflated_column with DateTime object');
 $cd->update;
@@ -85,21 +83,21 @@ $cd->update;
 is( $cd->year->year, $now->year, 'deflate ok' );
 
 # update tests
-$cd = $schema->resultset("CD")->find(3);                 
+$cd = $schema->resultset("CD")->find(3);
 eval { $cd->update({'year' => $now}) };
 ok(!$@, 'update using DateTime object ok');
 is($cd->year->year, $now->year, 'deflate ok');
 
-$cd = $schema->resultset("CD")->find(3);                 
+$cd = $schema->resultset("CD")->find(3);
 $before_year = $cd->year->year;
 eval { $cd->update({'year' => \'year + 1'}) };
 ok(!$@, 'update using scalarref ok');
 
-$cd = $schema->resultset("CD")->find(3);                 
+$cd = $schema->resultset("CD")->find(3);
 is($cd->year->year, $before_year + 1, 'deflate ok');
 
 # discard_changes test
-$cd = $schema->resultset("CD")->find(3);                 
+$cd = $schema->resultset("CD")->find(3);
 # inflate the year
 $before_year = $cd->year->year;
 $cd->update({ year => \'year + 1'});
@@ -110,4 +108,5 @@ is($cd->year->year, $before_year + 1, 'discard_changes clears the inflated value
 my $copy = $cd->copy({ year => $now, title => "zemoose" });
 
 isnt( $copy->year->year, $before_year, "copy" );
+
+done_testing;
index c2be971..e9b51df 100644 (file)
@@ -10,13 +10,13 @@ my $schema = DBICTest->init_schema();
 use Data::Dumper;
 
 my @serializers = (
-    {  module => 'YAML.pm',
-       inflater => sub { YAML::Load (shift) },
-       deflater => sub { die "Expecting a reference" unless (ref $_[0]); YAML::Dump (shift) },
+    { module => 'YAML.pm',
+      inflater => sub { YAML::Load (shift) },
+      deflater => sub { die "Expecting a reference" unless (ref $_[0]); YAML::Dump (shift) },
     },
-    {  module => 'Storable.pm',
-       inflater => sub { Storable::thaw (shift) },
-       deflater => sub { die "Expecting a reference" unless (ref $_[0]); Storable::nfreeze (shift) },
+    { module => 'Storable.pm',
+      inflater => sub { Storable::thaw (shift) },
+      deflater => sub { die "Expecting a reference" unless (ref $_[0]); Storable::nfreeze (shift) },
     },
 );
 
@@ -25,14 +25,13 @@ my $selected;
 foreach my $serializer (@serializers) {
     eval { require $serializer->{module} };
     unless ($@) {
-       $selected = $serializer;
-       last;
+      $selected = $serializer;
+      last;
     }
 }
 
 plan (skip_all => "No suitable serializer found") unless $selected;
 
-plan (tests => 11);
 DBICTest::Schema::Serialized->inflate_column( 'serialized',
     { inflate => $selected->{inflater},
       deflate => $selected->{deflater},
@@ -42,17 +41,17 @@ Class::C3->reinitialize;
 
 my $struct_hash = {
     a => 1,
-    b => [ 
+    b => [
         { c => 2 },
     ],
     d => 3,
 };
 
 my $struct_array = [
-    'a', 
-    { 
-       b => 1,
-       c => 2
+    'a',
+    {
+      b => 1,
+      c => 2,
     },
     'd',
 ];
@@ -63,7 +62,6 @@ my $inflated;
 #======= testing hashref serialization
 
 my $object = $rs->create( { 
-    id => 1,
     serialized => '',
 } );
 ok($object->update( { serialized => $struct_hash } ), 'hashref deflation');
@@ -71,13 +69,19 @@ ok($inflated = $object->serialized, 'hashref inflation');
 is_deeply($inflated, $struct_hash, 'inflated hash matches original');
 
 $object = $rs->create( { 
-    id => 2,
     serialized => '',
 } );
-eval { $object->set_inflated_column('serialized', $struct_hash) };
-ok(!$@, 'set_inflated_column to a hashref');
+$object->set_inflated_column('serialized', $struct_hash);
 is_deeply($object->serialized, $struct_hash, 'inflated hash matches original');
 
+$object = $rs->new({});
+$object->serialized ($struct_hash);
+$object->insert;
+is_deeply (
+  $rs->find ({id => $object->id})->serialized,
+  $struct_hash,
+  'new/insert works',
+);
 
 #====== testing arrayref serialization
 
@@ -85,8 +89,16 @@ ok($object->update( { serialized => $struct_array } ), 'arrayref deflation');
 ok($inflated = $object->serialized, 'arrayref inflation');
 is_deeply($inflated, $struct_array, 'inflated array matches original');
 
+$object = $rs->new({});
+$object->serialized ($struct_array);
+$object->insert;
+is_deeply (
+  $rs->find ({id => $object->id})->serialized,
+  $struct_array,
+  'new/insert works',
+);
 
-#===== make sure make_column_dirty ineracts reasonably with inflation
+#===== make sure make_column_dirty interacts reasonably with inflation
 $object = $rs->first;
 $object->update ({serialized => { x => 'y'}});
 
@@ -98,3 +110,5 @@ $object->make_column_dirty('serialized');
 $object->update;
 
 is_deeply ($rs->first->serialized, { x => 'z' }, 'changes made it to the db' );
+
+done_testing;
index c69d229..ee55792 100644 (file)
@@ -135,7 +135,7 @@ sub deploy_schema {
         close IN;
         for my $chunk ( split (/;\s*\n+/, $sql) ) {
           if ( $chunk =~ / ^ (?! --\s* ) \S /xm ) {  # there is some real sql in the chunk - a non-space at the start of the string which is not a comment
-            $schema->storage->dbh->do($chunk) or print "Error on SQL: $chunk\n";
+            $schema->storage->dbh_do(sub { $_[1]->do($chunk) }) or print "Error on SQL: $chunk\n";
           }
         }
     }
index 92c210f..d7737bd 100644 (file)
@@ -5,7 +5,7 @@ use base qw/DBICTest::BaseResult/;
 
 __PACKAGE__->table('serialized');
 __PACKAGE__->add_columns(
-  'id' => { data_type => 'integer' },
+  'id' => { data_type => 'integer', is_auto_increment => 1 },
   'serialized' => { data_type => 'text' },
 );
 __PACKAGE__->set_primary_key('id');
index 948597d..a6de595 100644 (file)
@@ -34,11 +34,6 @@ __PACKAGE__->add_columns(
     data_type => 'smalldatetime',
     is_nullable => 1
   },
-  increment => {
-      data_type => 'integer',
-      is_nullable => 1,
-      accessor => '_increment',
-  }
 );
 __PACKAGE__->set_primary_key('trackid');
 
@@ -55,20 +50,4 @@ __PACKAGE__->belongs_to( disc => 'DBICTest::Schema::CD' => 'cd');
 __PACKAGE__->might_have( cd_single => 'DBICTest::Schema::CD', 'single_track' );
 __PACKAGE__->might_have( lyrics => 'DBICTest::Schema::Lyrics', 'track_id' );
 
-sub increment {
-    my $self = shift;
-    if(@_) {
-        return $self->_increment($_[0] + 1);
-    }
-    return $self->_increment();
-}
-
-sub set_increment {
-    my $self = shift;
-    if(@_) {
-        return $self->_increment($_[0]);
-    }
-    return $self->_increment();
-}
-
 1;
index 814bb94..b1a3950 100644 (file)
@@ -1,4 +1,4 @@
--- Created on Thu Jul 30 09:37:43 2009
+-- Created on Wed Aug 12 16:10:43 2009
 -- 
 
 
@@ -283,8 +283,7 @@ CREATE TABLE track (
   title varchar(100) NOT NULL,
   last_updated_on datetime,
   last_updated_at datetime,
-  small_dt smalldatetime,
-  increment integer
+  small_dt smalldatetime
 );
 
 CREATE INDEX track_idx_cd ON track (cd);
index f518f73..6142098 100644 (file)
@@ -23,8 +23,8 @@ is_same_sql(
   '(
     SELECT
       cds.cdid, cds.artist, cds.title, cds.year, cds.genreid, cds.single_track,
-      single_track.trackid, single_track.cd, single_track.position, single_track.title, single_track.last_updated_on, single_track.last_updated_at, single_track.small_dt, single_track.increment,
-      single_track_2.trackid, single_track_2.cd, single_track_2.position, single_track_2.title, single_track_2.last_updated_on, single_track_2.last_updated_at, single_track_2.small_dt, single_track_2.increment,
+      single_track.trackid, single_track.cd, single_track.position, single_track.title, single_track.last_updated_on, single_track.last_updated_at, single_track.small_dt,
+      single_track_2.trackid, single_track_2.cd, single_track_2.position, single_track_2.title, single_track_2.last_updated_on, single_track_2.last_updated_at, single_track_2.small_dt,
       cd.cdid, cd.artist, cd.title, cd.year, cd.genreid, cd.single_track
     FROM artist me
       LEFT JOIN cd cds ON cds.artist = me.artistid
index bd50a9d..7ee4e7c 100644 (file)
@@ -163,7 +163,7 @@ for ($cd_rs->all) {
     $most_tracks_rs->as_query,
     '(
       SELECT  me.cdid, me.track_count,
-              tracks.trackid, tracks.cd, tracks.position, tracks.title, tracks.last_updated_on, tracks.last_updated_at, tracks.small_dt, tracks.increment,
+              tracks.trackid, tracks.cd, tracks.position, tracks.title, tracks.last_updated_on, tracks.last_updated_at, tracks.small_dt,
               liner_notes.liner_id, liner_notes.notes
         FROM (
           SELECT me.cdid, COUNT( tracks.trackid ) AS track_count
@@ -235,4 +235,40 @@ for ($cd_rs->all) {
   is ($rs->count, 5, 'Correct count of CDs');
 }
 
+# RT 47779, test group_by as a scalar ref
+{
+  my $track_rs = $schema->resultset ('Track')->search (
+    { 'me.cd' => { -in => [ $cd_rs->get_column ('cdid')->all ] } },
+    {
+      select => [
+        'me.cd',
+        { count => 'me.trackid' },
+      ],
+      as => [qw/
+        cd
+        track_count
+      /],
+      group_by => \'SUBSTR(me.cd, 1, 1)',
+      prefetch => 'cd',
+    },
+  );
+
+  is_same_sql_bind (
+    $track_rs->count_rs->as_query,
+    '(
+      SELECT COUNT( * )
+        FROM (
+          SELECT SUBSTR(me.cd, 1, 1)
+            FROM track me
+            JOIN cd cd ON cd.cdid = me.cd
+          WHERE ( me.cd IN ( ?, ?, ?, ?, ? ) )
+          GROUP BY SUBSTR(me.cd, 1, 1)
+        )
+      count_subq
+    )',
+    [ map { [ 'me.cd' => $_] } ($cd_rs->get_column ('cdid')->all) ],
+    'count() query generated expected SQL',
+  );
+}
+
 done_testing;