From: Matt S Trout <mst@shadowcat.co.uk>
Date: Mon, 8 May 2006 23:03:00 +0000 (+0000)
Subject: couple bugfixes
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=0823196cc624de941880cecd19588c3bac024e73;p=dbsrgits%2FDBIx-Class-Historic.git

couple bugfixes
---

diff --git a/Changes b/Changes
index 8efad83..21968ef 100644
--- a/Changes
+++ b/Changes
@@ -1,6 +1,8 @@
 Revision history for DBIx::Class
 
 0.06003
+        - fix for has_many prefetch with 0 related rows
+        - make limit error if rows => 0
         - added memory cycle tests and a long-needed weaken call
 
 0.06002 2006-04-20 00:42:41
diff --git a/lib/DBIx/Class/ResultSet.pm b/lib/DBIx/Class/ResultSet.pm
index feb8232..8089056 100644
--- a/lib/DBIx/Class/ResultSet.pm
+++ b/lib/DBIx/Class/ResultSet.pm
@@ -228,7 +228,7 @@ sub search {
 
   unless (@_) { # no search, effectively just a clone
     my $rows = $self->get_cache;
-    if( @{$rows} ) {
+    if ($rows) {
       $rs->set_cache($rows);
     }
   }
@@ -500,9 +500,9 @@ first record from the resultset.
 
 sub next {
   my ($self) = @_;
-  if (@{$self->{all_cache} || []}) {
+  if (my $cache = $self->get_cache) {
     $self->{all_cache_position} ||= 0;
-    return $self->{all_cache}->[$self->{all_cache_position}++];
+    return $cache->[$self->{all_cache_position}++];
   }
   if ($self->{attrs}{cache}) {
     $self->{all_cache_position} = 1;
@@ -592,9 +592,9 @@ sub _collapse_result {
       last unless (@raw = $self->cursor->next);
       $row = $self->{stashed_row} = \@raw;
       $tree = $self->_collapse_result($as, $row, $c_prefix);
-      #warn Data::Dumper::Dumper($tree, $row);
     }
-    @$target = @final;
+    @$target = (@final ? @final : [ {}, {} ]);
+      # single empty result to indicate an empty prefetched has_many
   }
 
   return $info;
@@ -641,7 +641,7 @@ clause.
 sub count {
   my $self = shift;
   return $self->search(@_)->count if @_ and defined $_[0];
-  return scalar @{ $self->get_cache } if @{ $self->get_cache };
+  return scalar @{ $self->get_cache } if $self->get_cache;
 
   my $count = $self->_count;
   return 0 unless $count;
@@ -718,7 +718,7 @@ is returned in list context.
 
 sub all {
   my ($self) = @_;
-  return @{ $self->get_cache } if @{ $self->get_cache };
+  return @{ $self->get_cache } if $self->get_cache;
 
   my @obj;
 
@@ -1183,7 +1183,7 @@ Gets the contents of the cache for the resultset, if the cache is set.
 =cut
 
 sub get_cache {
-  shift->{all_cache} || [];
+  shift->{all_cache};
 }
 
 =head2 set_cache
@@ -1206,13 +1206,7 @@ than re-querying the database even if the cache attr is not set.
 sub set_cache {
   my ( $self, $data ) = @_;
   $self->throw_exception("set_cache requires an arrayref")
-    if ref $data ne 'ARRAY';
-  my $result_class = $self->result_class;
-  foreach( @$data ) {
-    $self->throw_exception(
-      "cannot cache object of type '$_', expected '$result_class'"
-    ) if ref $_ ne $result_class;
-  }
+    if defined($data) && (ref $data ne 'ARRAY');
   $self->{all_cache} = $data;
 }
 
@@ -1231,7 +1225,7 @@ Clears the cache for the resultset.
 =cut
 
 sub clear_cache {
-  shift->set_cache([]);
+  shift->set_cache(undef);
 }
 
 =head2 related_resultset
diff --git a/lib/DBIx/Class/Storage/DBI.pm b/lib/DBIx/Class/Storage/DBI.pm
index bcd4606..62d2304 100644
--- a/lib/DBIx/Class/Storage/DBI.pm
+++ b/lib/DBIx/Class/Storage/DBI.pm
@@ -20,6 +20,8 @@ sub select {
   my ($self, $table, $fields, $where, $order, @rest) = @_;
   $table = $self->_quote($table) unless ref($table);
   @rest = (-1) unless defined $rest[0];
+  die "LIMIT 0 Does Not Compute" if $rest[0] == 0;
+    # and anyway, SQL::Abstract::Limit will cause a barf if we don't first
   local $self->{having_bind} = [];
   my ($sql, @ret) = $self->SUPER::select(
     $table, $self->_recurse_fields($fields), $where, $order, @rest
@@ -566,6 +568,8 @@ sub _select {
       $self->sql_maker->_default_limit_syntax eq "GenericSubQ") {
         $attrs->{software_limit} = 1;
   } else {
+    $self->throw_exception("rows attribute must be positive if present")
+      if (defined($attrs->{rows}) && !($attrs->{rows} > 0));
     push @args, $attrs->{rows}, $attrs->{offset};
   }
   return $self->_execute(@args);
diff --git a/t/run/16joins.tl b/t/run/16joins.tl
index c83aa7c..15603aa 100644
--- a/t/run/16joins.tl
+++ b/t/run/16joins.tl
@@ -7,7 +7,7 @@ BEGIN {
     eval "use DBD::SQLite";
     plan $@
         ? ( skip_all => 'needs DBD::SQLite for testing' )
-        : ( tests => 42 );
+        : ( tests => 44 );
 }
 
 # figure out if we've got a version of sqlite that is older than 3.2.6, in
@@ -101,6 +101,10 @@ $rs = $schema->resultset("CD")->search(
 );
 cmp_ok( scalar $rs->all, '==', scalar $rs->slice(0, $rs->count - 1), 'slice() with join has same count as all()' );
 
+eval { $rs->search(undef, { rows => 0, offset => 3 })->all; };
+
+ok($@, "rows => 0 errors: $@");
+
 $rs = $schema->resultset("Artist")->search(
         { 'liner_notes.notes' => 'Kill Yourself!' },
         { join => { 'cds' => 'liner_notes' } });
@@ -273,6 +277,25 @@ $schema->storage->debug(0);
 
 cmp_ok($queries, '==', 1, 'Only one query run');
 
+# has_many resulting in an additional select if no records available despite prefetch
+my $track = $schema->resultset("Artist")->create( {
+  artistid  => 4,
+  name      => 'Artist without CDs',
+} );
+
+$queries = 0;
+$schema->storage->debug(1);
+
+my $artist_without_cds = $schema->resultset("Artist")->find(4, {
+    join        => [qw/ cds /],
+    prefetch    => [qw/ cds /],
+});
+my @no_cds = $artist_without_cds->cds;
+
+is($queries, 1, 'prefetch ran only 1 sql statement');
+
+$schema->storage->debug(0);
+
 } # end run_tests
 
 1;
diff --git a/t/run/23cache.tl b/t/run/23cache.tl
index 74a6ae9..a822601 100644
--- a/t/run/23cache.tl
+++ b/t/run/23cache.tl
@@ -6,7 +6,7 @@ $schema->storage->debugcb( sub{ $queries++ } );
 
 eval "use DBD::SQLite";
 plan skip_all => 'needs DBD::SQLite for testing' if $@;
-plan tests => 23;
+plan tests => 22;
 
 my $rs = $schema->resultset("Artist")->search(
   { artistid => 1 }
@@ -14,7 +14,7 @@ my $rs = $schema->resultset("Artist")->search(
 
 my $artist = $rs->first;
 
-is( scalar @{ $rs->get_cache }, 0, 'cache is not populated without cache attribute' );
+ok( !defined($rs->get_cache), 'cache is not populated without cache attribute' );
 
 $rs = $schema->resultset('Artist')->search( undef, { cache => 1 } );
 my $artists = [ $rs->all ];
@@ -23,7 +23,7 @@ is( scalar @{$rs->get_cache}, 3, 'all() populates cache for search with cache at
 
 $rs->clear_cache;
 
-is( scalar @{$rs->get_cache}, 0, 'clear_cache is functional' );
+ok( !defined($rs->get_cache), 'clear_cache is functional' );
 
 $rs->next;
 
@@ -38,12 +38,6 @@ $cd = $schema->resultset('CD')->find(1);
 
 $rs->clear_cache;
 
-eval {
-  $rs->set_cache( [ $cd ] );
-};
-
-is( scalar @{$rs->get_cache}, 0, 'set_cache() only accepts objects of correct type for the resultset' );
-
 $queries = 0;
 $schema->storage->debug(1);