Merge 'trunk' into 'mssql_top_fixes'
Peter Rabbitson [Fri, 19 Jun 2009 16:41:45 +0000 (16:41 +0000)]
r6690@Thesaurus (orig r6689):  timbunce | 2009-06-16 15:06:07 +0200
Removed wording from txn_do that implies the coderef could be executed more than once.

r6691@Thesaurus (orig r6690):  timbunce | 2009-06-16 15:14:23 +0200
Added doc note that txn_commit does not perform an actual storage commit unless
there's a DBIx::Class transaction currently in effect

r6692@Thesaurus (orig r6691):  timbunce | 2009-06-16 15:40:11 +0200
Reverted doc patch r6689 for now, sadly. I'll open a ticket to explain.

r6694@Thesaurus (orig r6693):  ribasushi | 2009-06-16 17:22:59 +0200
Fix possible regression with prefetch select resolution
r6699@Thesaurus (orig r6698):  wintrmute | 2009-06-17 10:32:30 +0200
Replace vague language around whether load_classes/namespaces is preferred,
with an explanation that load_namespaces() is generally preferred, and explain
when load_classes is appropriate.
r6702@Thesaurus (orig r6701):  caelum | 2009-06-17 19:50:47 +0200
fix page with offset bug
r6704@Thesaurus (orig r6703):  ribasushi | 2009-06-18 08:40:18 +0200
Cleanup attribute handling - I deal with resolved attributes throughout, no point in complicating things further
r6705@Thesaurus (orig r6704):  abraxxa | 2009-06-18 12:30:01 +0200
added test for nested has_many prefetch without entries

r6707@Thesaurus (orig r6706):  ribasushi | 2009-06-18 12:43:36 +0200
HRI fix
r6708@Thesaurus (orig r6707):  ribasushi | 2009-06-18 14:05:42 +0200
wtf
r6714@Thesaurus (orig r6713):  caelum | 2009-06-19 01:03:01 +0200
fix broken link in manual
r6726@Thesaurus (orig r6725):  ribasushi | 2009-06-19 17:25:19 +0200
 r6706@Thesaurus (orig r6705):  ribasushi | 2009-06-18 12:30:08 +0200
 Branch to attempt prefetch with limit fix
 r6709@Thesaurus (orig r6708):  ribasushi | 2009-06-18 15:54:38 +0200
 This seems to be the prefetch+limit fix - ugly as hell but appears to work
 r6710@Thesaurus (orig r6709):  ribasushi | 2009-06-18 16:13:31 +0200
 More comments
 r6717@Thesaurus (orig r6716):  ribasushi | 2009-06-19 15:39:43 +0200
 single() throws with has_many prefetch
 r6718@Thesaurus (orig r6717):  ribasushi | 2009-06-19 15:40:38 +0200
 Rename test
 r6719@Thesaurus (orig r6718):  ribasushi | 2009-06-19 15:44:26 +0200
 cleanup svn attrs
 r6720@Thesaurus (orig r6719):  ash | 2009-06-19 16:31:11 +0200
 Add extra test for prefetch+has_many

 r6721@Thesaurus (orig r6720):  ribasushi | 2009-06-19 16:33:49 +0200
 no need to slice as use_prefetch already has a limit
 r6722@Thesaurus (orig r6721):  ribasushi | 2009-06-19 16:36:08 +0200
 throw in an extra limit, sophisticate test a bit
 r6723@Thesaurus (orig r6722):  ribasushi | 2009-06-19 16:36:54 +0200
 Fix the fix
 r6725@Thesaurus (orig r6724):  ribasushi | 2009-06-19 17:24:23 +0200
 Fix dubious optimization

16 files changed:
Changes
lib/DBIx/Class/Manual/Joining.pod
lib/DBIx/Class/ResultClass/HashRefInflator.pm
lib/DBIx/Class/ResultSet.pm
lib/DBIx/Class/Schema.pm
lib/DBIx/Class/Storage.pm
lib/DBIx/Class/Storage/DBI.pm
t/67pager.t
t/93storage_replication.t
t/cdbi/DeepAbstractSearch/01_search.t [changed mode: 0755->0644]
t/inflate/hri.t
t/lib/DBICTest/Schema/TwoKeys.pm [changed mode: 0755->0644]
t/prefetch/attrs_untouched.t
t/prefetch/double_prefetch.t [new file with mode: 0644]
t/prefetch/rows_bug.t [deleted file]
t/prefetch/with_limit.t [new file with mode: 0644]

diff --git a/Changes b/Changes
index e31367a..0ff9a38 100644 (file)
--- a/Changes
+++ b/Changes
@@ -1,5 +1,11 @@
 Revision history for DBIx::Class
 
+        - Fixed regression when both page and offset are specified on
+          a resultset
+        - Fixed HRI returning too many empty results on multilevel
+          nonexisting prefetch
+        - Fixed the prefetch with limit bug
+
 0.08107 2009-06-14 08:21:00 (UTC)
         - Fix serialization regression introduced in 0.08103 (affects
           Cursor::Cached)
index 700a1df..2a03c1a 100644 (file)
@@ -34,7 +34,7 @@ to fetch the tracks, or you can use a join. Compare:
 So, joins are a way of extending simple select statements to include
 fields from other, related, tables. There are various types of joins,
 depending on which combination of the data you wish to retrieve, see
-L<MySQL's doc on JOINs|http://dev.mysql.com/doc/refman/5.0/en/join.html>.
+MySQL's doc on JOINs: L<http://dev.mysql.com/doc/refman/5.0/en/join.html>.
 
 =head1 DEFINING JOINS AND RELATIONSHIPS
 
index 1b3f2d1..6d019ad 100644 (file)
@@ -73,8 +73,15 @@ $mk_hash = sub {
 
         # if there is at least one defined column consider the resultset real
         # (and not an emtpy has_many rel containing one empty hashref)
+        # an empty arrayref is an empty multi-sub-prefetch - don't consider
+        # those either
         for (values %$hash) {
-            return $hash if defined $_;
+            if (ref $_ eq 'ARRAY') {
+              return $hash if @$_;
+            }
+            elsif (defined $_) {
+              return $hash;
+            }
         }
 
         return undef;
index cfb736b..74940eb 100644 (file)
@@ -698,10 +698,14 @@ a warning:
 
   Query returned more than one row
 
-In this case, you should be using L</first> or L</find> instead, or if you really
+In this case, you should be using L</next> or L</find> instead, or if you really
 know what you are doing, use the L</rows> attribute to explicitly limit the size
 of the resultset.
 
+This method will also throw an exception if it is called on a resultset prefetching
+has_many, as such a prefetch implies fetching multiple rows from the database in
+order to assemble the resulting object.
+
 =back
 
 =cut
@@ -714,6 +718,12 @@ sub single {
 
   my $attrs = $self->_resolved_attrs_copy;
 
+  if (keys %{$attrs->{collapse}}) {
+    $self->throw_exception(
+      'single() can not be used on resultsets prefetching has_many. Use find( \%cond ) or next() instead'
+    );
+  }
+
   if ($where) {
     if (defined $attrs->{where}) {
       $attrs->{where} = {
@@ -1144,7 +1154,7 @@ sub count {
   return $self->search(@_)->count if @_ and defined $_[0];
   return scalar @{ $self->get_cache } if $self->get_cache;
 
-  my $meth = $self->_has_attr (qw/prefetch collapse distinct group_by/)
+  my $meth = $self->_has_resolved_attr (qw/collapse group_by/)
     ? 'count_grouped'
     : 'count'
   ;
@@ -1276,15 +1286,15 @@ sub _rs_update_delete {
 
   my $rsrc = $self->result_source;
 
-  my $needs_group_by_subq = $self->_has_attr (qw/prefetch distinct join seen_join group_by/);
-  my $needs_subq = $self->_has_attr (qw/row offset page/);
+  my $needs_group_by_subq = $self->_has_resolved_attr (qw/collapse group_by -join/);
+  my $needs_subq = $self->_has_resolved_attr (qw/row offset/);
 
   if ($needs_group_by_subq or $needs_subq) {
 
     # make a new $rs selecting only the PKs (that's all we really need)
     my $attrs = $self->_resolved_attrs_copy;
 
-    delete $attrs->{$_} for qw/prefetch collapse select +select as +as columns +columns/;
+    delete $attrs->{$_} for qw/collapse select as/;
     $attrs->{columns} = [ map { "$attrs->{alias}.$_" } ($self->result_source->primary_columns) ];
 
     if ($needs_group_by_subq) {
@@ -1808,14 +1818,14 @@ sub _is_deterministic_value {
   return 0;
 }
 
-# _has_attr
+# _has_resolved_attr
 #
 # determines if the resultset defines at least one
 # of the attributes supplied
 #
 # used to determine if a subquery is neccessary
 
-sub _has_attr {
+sub _has_resolved_attr {
   my ($self, @attr_names) = @_;
 
   my $attrs = $self->_resolved_attrs;
@@ -1823,7 +1833,7 @@ sub _has_attr {
   my $join_check_req;
 
   for my $n (@attr_names) {
-    ++$join_check_req if $n =~ /join/;
+    ++$join_check_req if $n eq '-join';
 
     my $attr =  $attrs->{$n};
 
@@ -1840,7 +1850,7 @@ sub _has_attr {
     }
   }
 
-  # a join can be expressed as a multi-level from
+  # a resolved join is expressed as a multi-level from
   return 1 if (
     $join_check_req
       and
@@ -2600,30 +2610,35 @@ sub _resolved_attrs {
     map { $prefix . $_ } ($source->primary_columns)
   ];
 
-  my $collapse = $attrs->{collapse} || {};
+  $attrs->{collapse} ||= {};
   if ( my $prefetch = delete $attrs->{prefetch} ) {
     $prefetch = $self->_merge_attr( {}, $prefetch );
-    my @pre_order;
-    foreach my $p ( ref $prefetch eq 'ARRAY' ? @$prefetch : ($prefetch) ) {
-
-      # bring joins back to level of current class
-      my $join_map = $self->_joinpath_aliases ($attrs->{from}, $attrs->{seen_join});
-      my @prefetch =
-        $source->_resolve_prefetch( $p, $alias, $join_map, \@pre_order, $collapse );
-      push( @{ $attrs->{select} }, map { $_->[0] } @prefetch );
-      push( @{ $attrs->{as} },     map { $_->[1] } @prefetch );
-    }
-    push( @{ $attrs->{order_by} }, @pre_order );
+
+    my $prefetch_ordering = [];
+
+    my $join_map = $self->_joinpath_aliases ($attrs->{from}, $attrs->{seen_join});
+
+    my @prefetch =
+      $source->_resolve_prefetch( $prefetch, $alias, $join_map, $prefetch_ordering, $attrs->{collapse} );
+
+    push( @{ $attrs->{select} }, map { $_->[0] } @prefetch );
+    push( @{ $attrs->{as} },     map { $_->[1] } @prefetch );
+
+    push( @{ $attrs->{order_by} }, @$prefetch_ordering );
+    $attrs->{_collapse_order_by} = \@$prefetch_ordering;
   }
 
+
   if (delete $attrs->{distinct}) {
     $attrs->{group_by} ||= [ grep { !ref($_) || (ref($_) ne 'HASH') } @{$attrs->{select}} ];
   }
 
-  $attrs->{collapse} = $collapse;
-
-  if ( $attrs->{page} and not defined $attrs->{offset} ) {
-    $attrs->{offset} = ( $attrs->{rows} * ( $attrs->{page} - 1 ) );
+  # if both page and offset are specified, produce a combined offset
+  # even though it doesn't make much sense, this is what pre 081xx has
+  # been doing
+  if (my $page = delete $attrs->{page}) {
+    $attrs->{offset} = ($attrs->{rows} * ($page - 1)) +
+      ($attrs->{offset} || 0);
   }
 
   return $self->{_attrs} = $attrs;
index 4b945cc..3ca9d5e 100644 (file)
@@ -285,8 +285,10 @@ sub load_namespaces {
 
 =back
 
-Alternative method to L</load_namespaces> which you should look at
-using if you can.
+L</load_classes> is an alternative method to L</load_namespaces>, both of
+which serve similar purposes, each with different advantages and disadvantages.
+In the general case you should use L</load_namespaces>, unless you need to
+be able to specify that only specific classes are loaded at runtime.
 
 With no arguments, this method uses L<Module::Find> to find all classes under
 the schema's namespace. Otherwise, this method loads the classes you specify
index a38cf47..bf50762 100644 (file)
@@ -248,6 +248,9 @@ sub txn_begin { die "Virtual method!" }
 
 Issues a commit of the current transaction.
 
+It does I<not> perform an actual storage commit unless there's a DBIx::Class
+transaction currently in effect (i.e. you called L</txn_begin>).
+
 =cut
 
 sub txn_commit { die "Virtual method!" }
index e8611cd..db8d2f0 100644 (file)
@@ -1226,21 +1226,15 @@ sub _select_args_to_query {
 }
 
 sub _select_args {
-  my ($self, $ident, $select, $condition, $attrs) = @_;
+  my ($self, $ident, $select, $where, $attrs) = @_;
 
   my $sql_maker = $self->sql_maker;
   $sql_maker->{_dbic_rs_attrs} = $attrs;
 
-  my $order = { map
-    { $attrs->{$_} ? ( $_ => $attrs->{$_} ) : ()  }
-    (qw/order_by group_by having _virtual_order_by/ )
-  };
-
-
-  my $bind_attrs = {};
-
   my $alias2source = $self->_resolve_ident_sources ($ident);
 
+  # calculate bind_attrs before possible $ident mangling
+  my $bind_attrs = {};
   for my $alias (keys %$alias2source) {
     my $bindtypes = $self->source_bind_attributes ($alias2source->{$alias}) || {};
     for my $col (keys %$bindtypes) {
@@ -1253,15 +1247,7 @@ sub _select_args {
     }
   }
 
-  # This would be the point to deflate anything found in $condition
-  # (and leave $attrs->{bind} intact). Problem is - inflators historically
-  # expect a row object. And all we have is a resultsource (it is trivial
-  # to extract deflator coderefs via $alias2source above).
-  #
-  # I don't see a way forward other than changing the way deflators are
-  # invoked, and that's just bad...
-
-  my @args = ('select', $attrs->{bind}, $ident, $bind_attrs, $select, $condition, $order);
+  my @limit;
   if ($attrs->{software_limit} ||
       $sql_maker->_default_limit_syntax eq "GenericSubQ") {
         $attrs->{software_limit} = 1;
@@ -1271,9 +1257,165 @@ sub _select_args {
 
     # MySQL actually recommends this approach.  I cringe.
     $attrs->{rows} = 2**48 if not defined $attrs->{rows} and defined $attrs->{offset};
-    push @args, $attrs->{rows}, $attrs->{offset};
+
+    if ($attrs->{rows} && keys %{$attrs->{collapse}}) {
+      ($ident, $select, $where, $attrs)
+        = $self->_adjust_select_args_for_limited_prefetch ($ident, $select, $where, $attrs);
+    }
+    else {
+      push @limit, $attrs->{rows}, $attrs->{offset};
+    }
   }
-  return @args;
+
+###
+  # This would be the point to deflate anything found in $where
+  # (and leave $attrs->{bind} intact). Problem is - inflators historically
+  # expect a row object. And all we have is a resultsource (it is trivial
+  # to extract deflator coderefs via $alias2source above).
+  #
+  # I don't see a way forward other than changing the way deflators are
+  # invoked, and that's just bad...
+###
+
+  my $order = { map
+    { $attrs->{$_} ? ( $_ => $attrs->{$_} ) : ()  }
+    (qw/order_by group_by having _virtual_order_by/ )
+  };
+
+
+  $sql_maker->{for} = delete $attrs->{for};
+
+  return ('select', $attrs->{bind}, $ident, $bind_attrs, $select, $where, $order, @limit);
+}
+
+sub _adjust_select_args_for_limited_prefetch {
+  my ($self, $from, $select, $where, $attrs) = @_;
+
+  if ($attrs->{group_by} and @{$attrs->{group_by}}) {
+    $self->throw_exception ('Prefetch with limit (rows/offset) is not supported on resultsets with a group_by attribute');
+  }
+
+  $self->throw_exception ('Prefetch with limit (rows/offset) is not supported on resultsets with a custom from attribute')
+    if (ref $from ne 'ARRAY');
+
+  # separate attributes
+  my $sub_attrs = { %$attrs };
+  delete $attrs->{$_} for qw/where bind rows offset/;
+  delete $sub_attrs->{$_} for qw/for collapse select order_by/;
+
+  my $alias = $attrs->{alias};
+
+  # create subquery select list
+  my $sub_select = [ grep { $_ =~ /^$alias\./ } @{$attrs->{select}} ];
+
+  # bring over all non-collapse-induced order_by into the inner query (if any)
+  # the outer one will have to keep them all
+  if (my $ord_cnt = @{$attrs->{order_by}} - @{$attrs->{_collapse_order_by}} ) {
+    $sub_attrs->{order_by} = [
+      @{$attrs->{order_by}}[ 0 .. ($#{$attrs->{order_by}} - $ord_cnt - 1) ]
+    ];
+  }
+
+
+  # mangle the head of the {from}
+  my $self_ident = shift @$from;
+
+  my %join_info = map { $_->[0]{-alias} => $_->[0] } (@$from);
+
+  my (%inner_joins);
+
+  # decide which parts of the join will remain on the inside
+  #
+  # this is not a very viable optimisation, but it was written
+  # before I realised this, so might as well remain. We can throw
+  # away _any_ branches of the join tree that are:
+  # 1) not mentioned in the condition/order
+  # 2) left-join leaves (or left-join leaf chains)
+  # Most of the join ocnditions will not satisfy this, but for real
+  # complex queries some might, and we might make some RDBMS happy.
+  #
+  #
+  # since we do not have introspectable SQLA, we fall back to ugly
+  # scanning of raw SQL for WHERE, and for pieces of ORDER BY
+  # in order to determine what goes into %inner_joins
+  # It may not be very efficient, but it's a reasonable stop-gap
+  {
+    # produce stuff unquoted, so it can be scanned
+    my $sql_maker = $self->sql_maker;
+    local $sql_maker->{quote_char};
+
+    my @order_by = (map
+      { ref $_ ? $_->[0] : $_ }
+      $sql_maker->_order_by_chunks ($sub_attrs->{order_by})
+    );
+
+    my $where_sql = $sql_maker->where ($where);
+
+    # sort needed joins
+    for my $alias (keys %join_info) {
+
+      # any table alias found on a column name in where or order_by
+      # gets included in %inner_joins
+      # Also any parent joins that are needed to reach this particular alias
+      for my $piece ($where_sql, @order_by ) {
+        if ($piece =~ /\b$alias\./) {
+          $inner_joins{$alias} = 1;
+        }
+      }
+    }
+  }
+
+  # scan for non-leaf/non-left joins and mark as needed
+  # also mark all ancestor joins that are needed to reach this particular alias
+  # (e.g.  join => { cds => 'tracks' } - tracks will bring cds too )
+  #
+  # traverse by the size of the -join_path i.e. reverse depth first
+  for my $alias (sort { @{$join_info{$b}{-join_path}} <=> @{$join_info{$a}{-join_path}} } (keys %join_info) ) {
+
+    my $j = $join_info{$alias};
+    $inner_joins{$alias} = 1 if (! $j->{-join_type} || ($j->{-join_type} !~ /^left$/i) );
+
+    if ($inner_joins{$alias}) {
+      $inner_joins{$_} = 1 for (@{$j->{-join_path}});
+    }
+  }
+
+  # construct the inner $from for the subquery
+  my $inner_from = [ $self_ident ];
+  if (keys %inner_joins) {
+    for my $j (@$from) {
+      push @$inner_from, $j if $inner_joins{$j->[0]{-alias}};
+    }
+
+    # if a multi-type join was needed in the subquery ("multi" is indicated by
+    # presence in {collapse}) - add a group_by to simulate the collapse in the subq
+    for my $alias (keys %inner_joins) {
+
+      # the dot comes from some weirdness in collapse
+      # remove after the rewrite
+      if ($attrs->{collapse}{".$alias"}) {
+        $sub_attrs->{group_by} = $sub_select;
+        last;
+      }
+    }
+  }
+
+  # generate the subquery
+  my $subq = $self->_select_args_to_query (
+    $inner_from,
+    $sub_select,
+    $where,
+    $sub_attrs
+  );
+
+  # put it back in $from
+  unshift @$from, { $alias => $subq };
+
+  # This is totally horrific - the $where ends up in both the inner and outer query
+  # Unfortunately not much can be done until SQLA2 introspection arrives
+  #
+  # OTOH it can be seen as a plus: <ash> (notes that this query would make a DBA cry ;)
+  return ($from, $select, $where, $attrs);
 }
 
 sub _resolve_ident_sources {
@@ -1338,8 +1480,8 @@ sub count {
 
   my $tmp_attrs = { %$attrs };
 
-  # take off any pagers, record_filter is cdbi, and no point of ordering a count
-  delete $tmp_attrs->{$_} for (qw/select as rows offset page order_by record_filter/);
+  # take off any limits, record_filter is cdbi, and no point of ordering a count
+  delete $tmp_attrs->{$_} for (qw/select as rows offset order_by record_filter/);
 
   # overwrite the selector
   $tmp_attrs->{select} = { count => '*' };
@@ -1363,7 +1505,7 @@ sub count_grouped {
   my $sub_attrs = { %$attrs };
 
   # these can not go in the subquery, and there is no point of ordering it
-  delete $sub_attrs->{$_} for qw/prefetch collapse select as order_by/;
+  delete $sub_attrs->{$_} for qw/collapse 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
   # simply deleting group_by suffices, as the code below will re-fill it
@@ -1380,7 +1522,7 @@ sub count_grouped {
   }];
 
   # the subquery replaces this
-  delete $attrs->{$_} for qw/where bind prefetch collapse group_by having having_bind rows offset page pager/;
+  delete $attrs->{$_} for qw/where bind collapse group_by having having_bind rows offset/;
 
   return $self->count ($source, $attrs);
 }
index 17b05c3..6a6aabc 100644 (file)
@@ -84,3 +84,19 @@ is($p->(), 10, 'default rows is 10');
 $schema->default_resultset_attributes({ rows => 5 });
 
 is($p->(), 5, 'default rows is 5');
+
+# test page with offset
+$it = $schema->resultset('CD')->search({}, {
+    rows => 2,
+    page => 2,
+    offset => 1,
+    order_by => 'cdid'
+});
+
+my $row = $schema->resultset('CD')->search({}, {
+    order_by => 'cdid', 
+    offset => 3,
+    rows => 1
+})->single;
+
+is($row->cdid, $it->first->cdid, 'page with offset');
index b85da4a..a2463e4 100644 (file)
@@ -361,10 +361,7 @@ is $artist1->name, 'Ozric Tentacles'
 
 ## Check that master_read_weight is honored
 {
-    no warnings 'once';
-
-    # turn off redefined warning
-    local $SIG{__WARN__} = sub {};
+    no warnings qw/once redefine/;
 
     local
     *DBIx::Class::Storage::DBI::Replicated::Balancer::Random::_random_number =
old mode 100755 (executable)
new mode 100644 (file)
index 3532900..1d32dd6 100644 (file)
@@ -6,7 +6,6 @@ use lib qw(t/lib);
 use DBICTest;
 my $schema = DBICTest->init_schema();
 
-
 # Under some versions of SQLite if the $rs is left hanging around it will lock
 # So we create a scope here cos I'm lazy
 {
@@ -124,3 +123,15 @@ $rs_hashrefinf = $schema->resultset ('Artist')->search ({ 'me.artistid' => 1}, {
 });
 $rs_hashrefinf->result_class('DBIx::Class::ResultClass::HashRefInflator');
 is_deeply [$rs_hashrefinf->all], \@hashrefinf, 'Check query using extended columns syntax';
+
+# check nested prefetching of has_many relationships which return nothing
+my $artist = $schema->resultset ('Artist')->create ({ name => 'unsuccessful artist without CDs'});
+$artist->discard_changes;
+my $rs_artists = $schema->resultset ('Artist')->search ({ 'me.artistid' => $artist->id}, {
+    prefetch => { cds => 'tracks' }, result_class => 'DBIx::Class::ResultClass::HashRefInflator',
+});
+is_deeply(
+  [$rs_artists->all],
+  [{ $artist->get_columns, cds => [] }],
+  'nested has_many prefetch without entries'
+);
old mode 100755 (executable)
new mode 100644 (file)
index 53894b8..41ad883 100644 (file)
@@ -8,16 +8,7 @@ use Data::Dumper;
 
 my $schema = DBICTest->init_schema();
 
-my $orig_debug = $schema->storage->debug;
-
-use IO::File;
-
-BEGIN {
-    eval "use DBD::SQLite";
-    plan $@
-        ? ( skip_all => 'needs DBD::SQLite for testing' )
-        : ( tests => 3 );
-}
+plan tests => 3;
 
 # bug in 0.07000 caused attr (join/prefetch) to be modifed by search
 # so we check the search & attr arrays are not modified
diff --git a/t/prefetch/double_prefetch.t b/t/prefetch/double_prefetch.t
new file mode 100644 (file)
index 0000000..b666220
--- /dev/null
@@ -0,0 +1,35 @@
+use warnings;  
+
+use Test::More;
+use Test::Exception;
+use lib qw(t/lib);
+use DBIC::SqlMakerTest;
+use DBICTest;
+
+my $schema = DBICTest->init_schema();
+
+plan tests => 1;
+
+# While this is a rather GIGO case, make sure it behaves as pre-103,
+# as it may result in hard-to-track bugs
+my $cds = $schema->resultset('Artist')
+            ->search_related ('cds')
+              ->search ({}, {
+                  prefetch => [ 'single_track', { single_track => 'cd' } ],
+                });
+
+is_same_sql(
+  ${$cds->as_query}->[0],
+  '(
+    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_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,
+      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
+      LEFT JOIN track single_track ON single_track.trackid = cds.single_track
+      LEFT JOIN track single_track_2 ON single_track_2.trackid = cds.single_track
+      LEFT JOIN cd cd ON cd.cdid = single_track_2.cd
+  )',
+);
diff --git a/t/prefetch/rows_bug.t b/t/prefetch/rows_bug.t
deleted file mode 100644 (file)
index 21bb6a2..0000000
+++ /dev/null
@@ -1,68 +0,0 @@
-# Test to ensure we get a consistent result set wether or not we use the
-# prefetch option in combination rows (LIMIT).
-use strict;
-use warnings;
-
-use Test::More;
-use lib qw(t/lib);
-use DBICTest;
-
-plan skip_all => 'fix pending';
-#plan tests => 4;
-
-my $schema = DBICTest->init_schema();
-my $no_prefetch = $schema->resultset('Artist')->search(
-  undef,
-  { rows => 3 }
-);
-
-my $use_prefetch = $schema->resultset('Artist')->search(
-  undef,
-  {
-    prefetch => 'cds',
-    rows     => 3
-  }
-);
-
-is($no_prefetch->count, $use_prefetch->count, '$no_prefetch->count == $use_prefetch->count');
-is(
-  scalar ($no_prefetch->all),
-  scalar ($use_prefetch->all),
-  "Amount of returned rows is right"
-);
-
-
-
-my $artist_many_cds = $schema->resultset('Artist')->search ( {}, {
-  join => 'cds',
-  group_by => 'me.artistid',
-  having => \ 'count(cds.cdid) > 1',
-})->first;
-
-
-$no_prefetch = $schema->resultset('Artist')->search(
-  { artistid => $artist_many_cds->id },
-  { rows => 1 }
-);
-
-$use_prefetch = $schema->resultset('Artist')->search(
-  { artistid => $artist_many_cds->id },
-  {
-    prefetch => 'cds',
-    rows     => 1
-  }
-);
-
-my $prefetch_artist = $use_prefetch->first;
-my $normal_artist = $no_prefetch->first;
-
-is(
-  $prefetch_artist->cds->count,
-  $normal_artist->cds->count,
-  "Count of child rel with prefetch + rows => 1 is right"
-);
-is (
-  scalar ($prefetch_artist->cds->all),
-  scalar ($normal_artist->cds->all),
-  "Amount of child rel rows with prefetch + rows => 1 is right"
-);
diff --git a/t/prefetch/with_limit.t b/t/prefetch/with_limit.t
new file mode 100644 (file)
index 0000000..08df104
--- /dev/null
@@ -0,0 +1,93 @@
+# Test to ensure we get a consistent result set wether or not we use the
+# prefetch option in combination rows (LIMIT).
+use strict;
+use warnings;
+
+use Test::More;
+use Test::Exception;
+use lib qw(t/lib);
+use DBICTest;
+
+plan tests => 9;
+
+my $schema = DBICTest->init_schema();
+
+
+my $no_prefetch = $schema->resultset('Artist')->search(
+  undef,
+  { rows => 3 }
+);
+
+my $use_prefetch = $schema->resultset('Artist')->search(
+  [   # search deliberately contrived
+    { 'artwork.cd_id' => undef },
+    { 'tracks.title' => { '!=' => 'blah-blah-1234568' }}
+  ],
+  {
+    prefetch => 'cds',
+    join => { cds => [qw/artwork tracks/] },
+    rows     => 3,
+    order_by => { -desc => 'name' },
+  }
+);
+
+is($no_prefetch->count, $use_prefetch->count, '$no_prefetch->count == $use_prefetch->count');
+is(
+  scalar ($no_prefetch->all),
+  scalar ($use_prefetch->all),
+  "Amount of returned rows is right"
+);
+
+my $artist_many_cds = $schema->resultset('Artist')->search ( {}, {
+  join => 'cds',
+  group_by => 'me.artistid',
+  having => \ 'count(cds.cdid) > 1',
+})->first;
+
+
+$no_prefetch = $schema->resultset('Artist')->search(
+  { artistid => $artist_many_cds->id },
+  { rows => 1 }
+);
+
+$use_prefetch = $no_prefetch->search ({}, { prefetch => 'cds' });
+
+my $normal_artist = $no_prefetch->single;
+my $prefetch_artist = $use_prefetch->find({ name => $artist_many_cds->name });
+my $prefetch2_artist = $use_prefetch->first;
+
+is(
+  $prefetch_artist->cds->count,
+  $normal_artist->cds->count,
+  "Count of child rel with prefetch + rows => 1 is right (find)"
+);
+is(
+  $prefetch2_artist->cds->count,
+  $normal_artist->cds->count,
+  "Count of child rel with prefetch + rows => 1 is right (first)"
+);
+
+is (
+  scalar ($prefetch_artist->cds->all),
+  scalar ($normal_artist->cds->all),
+  "Amount of child rel rows with prefetch + rows => 1 is right (find)"
+);
+is (
+  scalar ($prefetch2_artist->cds->all),
+  scalar ($normal_artist->cds->all),
+  "Amount of child rel rows with prefetch + rows => 1 is right (first)"
+);
+
+throws_ok (
+  sub { $use_prefetch->single },
+  qr/resultsets prefetching has_many/,
+  'single() with multiprefetch is illegal',
+);
+
+my $artist = $use_prefetch->search({'cds.title' => $artist_many_cds->cds->first->title })->next;
+is($artist->cds->count, 1, "count on search limiting prefetched has_many");
+
+# try with double limit
+my $artist2 = $use_prefetch->search({'cds.title' => { '!=' => $artist_many_cds->cds->first->title } })->slice (0,0)->next;
+is($artist2->cds->count, 2, "count on search limiting prefetched has_many");
+