From: Peter Rabbitson <ribasushi@cpan.org>
Date: Sun, 17 Jan 2010 12:44:00 +0000 (+0000)
Subject: Getting warmer
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=539ffe8768e85b2061aa3bb3616da4f848a582f3;p=dbsrgits%2FDBIx-Class-Historic.git

Getting warmer
---

diff --git a/lib/DBIx/Class/ResultSet.pm b/lib/DBIx/Class/ResultSet.pm
index 5827fb4..b0d2e12 100644
--- a/lib/DBIx/Class/ResultSet.pm
+++ b/lib/DBIx/Class/ResultSet.pm
@@ -1253,16 +1253,15 @@ 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. We DO WANT to clobber old group_by regardless
-  if ( keys %{$attrs->{collapse}} ) {
+  # if we multi-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) ]
   }
 
   $sub_attrs->{select} = $rsrc->storage->_subq_count_select ($rsrc, $sub_attrs);
 
   # this is so that the query can be simplified e.g.
-  # * non-limiting joins can be pruned
   # * ordering can be thrown away in things like Top limit
   $sub_attrs->{-for_count_only} = 1;
 
@@ -2927,7 +2926,7 @@ sub _joinpath_aliases {
     my $jpath = $j->[0]{-join_path};
 
     my $p = $paths;
-    $p = $p->{$_} ||= {} for @{$jpath}[$cur_depth/2 .. $#$jpath]; #only even depths are actual jpath boundaries
+    $p = $p->{$_} ||= {} for @{$jpath}[ ($cur_depth/2) .. $#$jpath]; #only even depths are actual jpath boundaries
     push @{$p->{-join_aliases} }, $j->[0]{-alias};
   }
 
diff --git a/lib/DBIx/Class/ResultSource.pm b/lib/DBIx/Class/ResultSource.pm
index c3b9c74..04b634c 100644
--- a/lib/DBIx/Class/ResultSource.pm
+++ b/lib/DBIx/Class/ResultSource.pm
@@ -1262,6 +1262,7 @@ sub _resolve_join {
                   : $rel_info->{attrs}{join_type}
                 ,
                -join_path => [@$jpath, $join],
+               -is_single => (List::Util::first { $rel_info->{attrs}{accessor} eq $_ } (qw/single filter/) ),
                -alias => $as,
                -relation_chain_depth => $seen->{-relation_chain_depth} || 0,
              },
diff --git a/lib/DBIx/Class/Storage/DBI.pm b/lib/DBIx/Class/Storage/DBI.pm
index 846decc..2c742a5 100644
--- a/lib/DBIx/Class/Storage/DBI.pm
+++ b/lib/DBIx/Class/Storage/DBI.pm
@@ -1740,7 +1740,7 @@ sub _select_args {
     select => $select,
     from => $ident,
     where => $where,
-    $rs_alias
+    $rs_alias && $alias2source->{$rs_alias}
       ? ( _source_handle => $alias2source->{$rs_alias}->handle )
       : ()
     ,
@@ -1858,6 +1858,9 @@ sub _select_args {
     push @limit, $attrs->{rows}, $attrs->{offset};
   }
 
+  # try to simplify the joinmap further (prune unreferenced type-single joins)
+  $ident = $self->_prune_unused_joins ($ident, $select, $where, $attrs);
+
 ###
   # This would be the point to deflate anything found in $where
   # (and leave $attrs->{bind} intact). Problem is - inflators historically
diff --git a/lib/DBIx/Class/Storage/DBIHacks.pm b/lib/DBIx/Class/Storage/DBIHacks.pm
index f8dae36..f549204 100644
--- a/lib/DBIx/Class/Storage/DBIHacks.pm
+++ b/lib/DBIx/Class/Storage/DBIHacks.pm
@@ -17,9 +17,7 @@ use Carp::Clan qw/^DBIx::Class/;
 
 #
 # This code will remove non-selecting/non-restricting joins from
-# {from} specs, aiding the RDBMS query optimizer. It will leave any
-# unused type-multi joins, if the amount of returned rows is
-# important (i.e. count without collapse)
+# {from} specs, aiding the RDBMS query optimizer.
 #
 sub _prune_unused_joins {
   my $self = shift;
@@ -29,13 +27,17 @@ sub _prune_unused_joins {
     return $from;   # only standard {from} specs are supported
   }
 
-  my $aliastypes = $self->_resolve_aliases_from_select_args($from, @_);
+  my $aliastypes = $self->_resolve_aliastypes_from_select_args($from, @_);
 
   my @newfrom = $from->[0]; # FROM head is always present
 
   my %need_joins = (map { %{$_||{}} } (values %$aliastypes) );
   for my $j (@{$from}[1..$#$from]) {
-    push @newfrom, $j if $need_joins{$j->[0]{-alias}};
+    push @newfrom, $j if (
+      ! $j->[0]{-alias} # legacy crap
+        ||
+      $need_joins{$j->[0]{-alias}}
+    );
   }
 
   return \@newfrom;
@@ -92,19 +94,13 @@ sub _adjust_select_args_for_complex_prefetch {
   # construct the inner $from for the subquery
   my $inner_from = $self->_prune_unused_joins ($from, $inner_select, $where, $inner_attrs);
 
-  # 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
-  unless ($inner_attrs->{group_by}) {
-    for my $alias (map { $_->[0]{-alias} } (@{$inner_from}[1 .. $#$inner_from]) ) {
-
-      # the dot comes from some weirdness in collapse
-      # remove after the rewrite
-      if ($attrs->{collapse}{".$alias"}) {
-        $inner_attrs->{group_by} ||= $inner_select;
-        last;
-      }
-    }
-  }
+  # if a multi-type join was needed in the subquery - add a group_by to simulate the
+  # collapse in the subq
+  $inner_attrs->{group_by} ||= $inner_select
+    if List::Util::first
+      { ! $_->[0]{-is_single} }
+      (@{$inner_from}[1 .. $#$inner_from])
+  ;
 
   # generate the subquery
   my $subq = $self->_select_args_to_query (
@@ -153,7 +149,7 @@ sub _adjust_select_args_for_complex_prefetch {
   # scan the from spec against different attributes, and see which joins are needed
   # in what role
   my $outer_aliastypes =
-    $self->_resolve_aliases_from_select_args( $from, $outer_select, $where, $outer_attrs );
+    $self->_resolve_aliastypes_from_select_args( $from, $outer_select, $where, $outer_attrs );
 
   # see what's left - throw away if not selecting/restricting
   # also throw in a group_by if restricting to guard against
@@ -167,22 +163,7 @@ sub _adjust_select_args_for_complex_prefetch {
     }
     elsif ($outer_aliastypes->{restrict}{$alias}) {
       push @outer_from, $j;
-
-      # FIXME - this should be obviated by SQLA2, as I'll be able to 
-      # have restrict_inner and restrict_outer... or something to that
-      # effect... I think...
-
-      # FIXME2 - I can't find a clean way to determine if a particular join
-      # is a multi - instead I am just treating everything as a potential
-      # explosive join (ribasushi)
-      #
-      # if (my $handle = $j->[0]{-source_handle}) {
-      #   my $rsrc = $handle->resolve;
-      #   ... need to bail out of the following if this is not a multi,
-      #       as it will be much easier on the db ...
-
-          $outer_attrs->{group_by} ||= $outer_select;
-      # }
+      $outer_attrs->{group_by} ||= $outer_select unless $j->[0]{-is_single};
     }
   }
 
@@ -208,7 +189,7 @@ sub _adjust_select_args_for_complex_prefetch {
 # happen is for it to fail due to an unqualified column, which in
 # turn will result in a vocal exception. Qualifying the column will
 # invariably solve the problem.
-sub _resolve_aliases_from_select_args {
+sub _resolve_aliastypes_from_select_args {
   my ( $self, $from, $select, $where, $attrs ) = @_;
 
   $self->throw_exception ('Unable to analyze custom {from}')
@@ -219,10 +200,15 @@ sub _resolve_aliases_from_select_args {
 
   # see what aliases are there to work with
   my $alias_list;
-  my @from = @$from; # if I don't copy weird shit happens
-  for my $j (@from) {
+  for (@$from) {
+    my $j = $_;
     $j = $j->[0] if ref $j eq 'ARRAY';
-    $alias_list->{$j->{-alias}} = $j;
+    my $al = $j->{-alias}
+      or next;
+
+    $alias_list->{$al} = $j;
+    $aliases_by_type->{multiplying}{$al} = 1
+      unless $j->{-is_single};
   }
 
   # set up a botched SQLA
diff --git a/t/count/count_rs.t b/t/count/count_rs.t
index acf696c..0f2a1a0 100644
--- a/t/count/count_rs.t
+++ b/t/count/count_rs.t
@@ -35,7 +35,6 @@ my $schema = DBICTest->init_schema();
       FROM cd me
       JOIN track tracks ON tracks.cd = me.cdid
       JOIN cd disc ON disc.cdid = tracks.cd
-      LEFT JOIN lyrics lyrics ON lyrics.track_id = tracks.trackid 
      WHERE ( ( position = ? OR position = ? ) )
     ',
     [ qw/'1' '2'/ ],
@@ -53,7 +52,6 @@ my $schema = DBICTest->init_schema();
           FROM cd me
           JOIN track tracks ON tracks.cd = me.cdid
           JOIN cd disc ON disc.cdid = tracks.cd
-          LEFT JOIN lyrics lyrics ON lyrics.track_id = tracks.trackid 
         WHERE ( ( position = ? OR position = ? ) )
         LIMIT 3 OFFSET 8
        ) count_subq
diff --git a/t/count/prefetch.t b/t/count/prefetch.t
index ad4b23c..654520b 100644
--- a/t/count/prefetch.t
+++ b/t/count/prefetch.t
@@ -72,7 +72,7 @@ my $schema = DBICTest->init_schema();
 {
   my $rs = $schema->resultset("CD")
             ->search_related('tracks',
-                { position => [1,2] },
+                { position => [1,2], 'lyrics.lyric_id' => undef },
                 { prefetch => [qw/disc lyrics/] },
             );
   is ($rs->all, 10, 'Correct number of objects');
@@ -88,7 +88,7 @@ my $schema = DBICTest->init_schema();
         JOIN track tracks ON tracks.cd = me.cdid
         JOIN cd disc ON disc.cdid = tracks.cd
         LEFT JOIN lyrics lyrics ON lyrics.track_id = tracks.trackid
-      WHERE position = ? OR position = ?
+      WHERE lyrics.lyric_id IS NULL AND (position = ? OR position = ?)
     )',
     [ map { [ position => $_ ] } (1, 2) ],
   );
diff --git a/t/prefetch/grouped.t b/t/prefetch/grouped.t
index a7a16e0..5d0905e 100644
--- a/t/prefetch/grouped.t
+++ b/t/prefetch/grouped.t
@@ -149,7 +149,6 @@ for ($cd_rs->all) {
           SELECT me.cdid
             FROM cd me
             LEFT JOIN track tracks ON tracks.cd = me.cdid
-            LEFT JOIN liner_notes liner_notes ON liner_notes.liner_id = me.cdid
           WHERE ( me.cdid IS NOT NULL )
           GROUP BY me.cdid
           LIMIT 2
diff --git a/t/search/preserve_original_rs.t b/t/search/preserve_original_rs.t
index 8ba6d18..8913121 100644
--- a/t/search/preserve_original_rs.t
+++ b/t/search/preserve_original_rs.t
@@ -89,4 +89,3 @@ for my $s (qw/a2a artw cd artw_back/) {
 
   is_same_sql_bind ($rs->as_query, $q{$s}{query}, "$s resultset unmodified (as_query matches)" );
 }
-
diff --git a/t/search/subquery.t b/t/search/subquery.t
index 5afc9f3..5d0a255 100644
--- a/t/search/subquery.t
+++ b/t/search/subquery.t
@@ -76,9 +76,13 @@ my @tests = (
   {
     rs => $art_rs,
     attrs => {
-      from => [ { 'me' => 'artist' }, 
-        [ { 'cds' => $cdrs->search({},{ 'select' => [\'me.artist as cds_artist' ]})->as_query },
-        { 'me.artistid' => 'cds_artist' } ] ]
+      from => [
+        { 'me' => 'artist' },
+        [
+          { 'cds' => $cdrs->search({}, { 'select' => [\'me.artist as cds_artist' ]})->as_query },
+          { 'me.artistid' => 'cds_artist' } 
+        ]
+      ]
     },
     sqlbind => \[
       "( SELECT me.artistid, me.name, me.rank, me.charfield FROM artist me JOIN (SELECT me.artist as cds_artist FROM cd me) cds ON me.artistid = cds_artist )"