Refactor count handling, make count-resultset attribute lists inclusive rather than...
Peter Rabbitson [Tue, 4 May 2010 08:00:11 +0000 (08:00 +0000)]
lib/DBIx/Class/Manual/Cookbook.pod
lib/DBIx/Class/ResultSet.pm
lib/DBIx/Class/Storage/DBI.pm
lib/DBIx/Class/Storage/DBI/Replicated.pm
t/90join_torture.t
t/count/count_rs.t
t/count/prefetch.t
t/prefetch/grouped.t

index 590e439..e31245f 100644 (file)
@@ -292,7 +292,7 @@ See also L</Using SQL functions on the left hand side of a comparison>.
   my $count = $rs->count;
 
   # Equivalent SQL:
-  # SELECT COUNT( * ) FROM (SELECT me.name FROM artist me GROUP BY me.name) count_subq:
+  # SELECT COUNT( * ) FROM (SELECT me.name FROM artist me GROUP BY me.name) me:
 
 =head2 Grouping results
 
index a360d3e..3805812 100644 (file)
@@ -1235,10 +1235,11 @@ sub _count_rs {
   my $rsrc = $self->result_source;
   $attrs ||= $self->_resolved_attrs;
 
-  my $tmp_attrs = { %$attrs };
-
-  # 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/);
+  # only take pieces we need for a simple count
+  my $tmp_attrs = { map
+    { $_ => $attrs->{$_} }
+    qw/ alias from where bind join /
+  };
 
   # overwrite the selector (supplied by the storage)
   $tmp_attrs->{select} = $rsrc->storage->_count_select ($rsrc, $tmp_attrs);
@@ -1256,12 +1257,12 @@ sub _count_subq_rs {
   my ($self, $attrs) = @_;
 
   my $rsrc = $self->result_source;
-  $attrs ||= $self->_resolved_attrs_copy;
-
-  my $sub_attrs = { %$attrs };
+  $attrs ||= $self->_resolved_attrs;
 
-  # 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/;
+  my $sub_attrs = { map
+    { $_ => $attrs->{$_} }
+    qw/ alias from where bind join group_by having rows offset /
+  };
 
   # 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
@@ -1269,24 +1270,35 @@ sub _count_subq_rs {
     $sub_attrs->{group_by} = [ map { "$attrs->{alias}.$_" } ($rsrc->_pri_cols) ]
   }
 
-  $sub_attrs->{select} = $rsrc->storage->_subq_count_select ($rsrc, $attrs);
+  # Calculate subquery selector
+  if (my $g = $sub_attrs->{group_by}) {
 
-  # this is so that the query can be simplified e.g.
-  # * ordering can be thrown away in things like Top limit
-  $sub_attrs->{-for_count_only} = 1;
+    # necessary as the group_by may refer to aliased functions
+    my $sel_index;
+    for my $sel (@{$attrs->{select}}) {
+      $sel_index->{$sel->{-as}} = $sel
+        if (ref $sel eq 'HASH' and $sel->{-as});
+    }
 
-  my $sub_rs = $rsrc->resultset_class->new ($rsrc, $sub_attrs);
+    for my $g_part (@$g) {
+      push @{$sub_attrs->{select}}, $sel_index->{$g_part} || $g_part;
+    }
+  }
+  else {
+    my @pcols = map { "$attrs->{alias}.$_" } ($rsrc->primary_columns);
+    $sub_attrs->{select} = @pcols ? \@pcols : [ 1 ];
+  }
 
-  $attrs->{from} = [{
-    -alias => 'count_subq',
-    -source_handle => $rsrc->handle,
-    count_subq => $sub_rs->as_query,
-  }];
 
-  # the subquery replaces this
-  delete $attrs->{$_} for qw/where bind collapse group_by having having_bind rows offset/;
+  # this is so that the query can be simplified e.g.
+  # * ordering can be thrown away in things like Top limit
+  $sub_attrs->{-for_count_only} = 1;
 
-  return $self->_count_rs ($attrs);
+  return $rsrc->resultset_class
+               ->new ($rsrc, $sub_attrs)
+                ->as_subselect_rs
+                 ->search ({}, { columns => { count => $rsrc->storage->_count_select ($rsrc, $attrs) } })
+                  -> get_column ('count');
 }
 
 sub _bool {
@@ -2668,15 +2680,18 @@ but because we isolated the group by into a subselect the above works.
 =cut
 
 sub as_subselect_rs {
-   my $self = shift;
-
-   return $self->result_source->resultset->search( undef, {
-      alias => $self->current_source_alias,
-      from => [{
-            $self->current_source_alias => $self->as_query,
-            -alias         => $self->current_source_alias,
-            -source_handle => $self->result_source->handle,
-         }]
+  my $self = shift;
+
+  my $attrs = $self->_resolved_attrs;
+
+  return $self->result_source->resultset->search( undef, {
+    from => [{
+      $attrs->{alias} => $self->as_query,
+      -alias         => $attrs->{alias},
+      -source_handle => $self->result_source->handle,
+    }],
+    map { $_ => $attrs->{$_} } qw/select as alias/
+
    });
 }
 
@@ -2702,7 +2717,7 @@ sub _chain_relationship {
   # ->_resolve_join as otherwise they get lost - captainL
   my $join = $self->_merge_attr( $attrs->{join}, $attrs->{prefetch} );
 
-  delete @{$attrs}{qw/join prefetch collapse distinct select as columns +select +as +columns/};
+  delete @{$attrs}{qw/join prefetch collapse group_by distinct select as columns +select +as +columns/};
 
   my $seen = { %{ (delete $attrs->{seen_join}) || {} } };
 
@@ -2728,7 +2743,7 @@ sub _chain_relationship {
       -alias => $attrs->{alias},
       $attrs->{alias} => $rs_copy->as_query,
     }];
-    delete @{$attrs}{@force_subq_attrs, 'where'};
+    delete @{$attrs}{@force_subq_attrs, qw/where bind/};
     $seen->{-relation_chain_depth} = 0;
   }
   elsif ($attrs->{from}) {  #shallow copy suffices
index 1acd60e..4616b7b 100644 (file)
@@ -2017,46 +2017,6 @@ sub _count_select {
   return { count => '*' };
 }
 
-# Returns a SELECT which will end up in the subselect
-# There may or may not be a group_by, as the subquery
-# might have been called to accomodate a limit
-#
-# Most databases would be happy with whatever ends up
-# here, but some choke in various ways.
-#
-sub _subq_count_select {
-  my ($self, $source, $rs_attrs) = @_;
-
-  if (my $groupby = $rs_attrs->{group_by}) {
-
-    my $avail_columns = $self->_resolve_column_info ($rs_attrs->{from});
-
-    my $sel_index;
-    for my $sel (@{$rs_attrs->{select}}) {
-      if (ref $sel eq 'HASH' and $sel->{-as}) {
-        $sel_index->{$sel->{-as}} = $sel;
-      }
-    }
-
-    my @selection;
-    for my $g_part (@$groupby) {
-      if (ref $g_part or $avail_columns->{$g_part}) {
-        push @selection, $g_part;
-      }
-      elsif ($sel_index->{$g_part}) {
-        push @selection, $sel_index->{$g_part};
-      }
-      else {
-        $self->throw_exception ("group_by criteria '$g_part' not contained within current resultset source(s)");
-      }
-    }
-
-    return \@selection;
-  }
-
-  my @pcols = map { join '.', $rs_attrs->{alias}, $_ } ($source->primary_columns);
-  return @pcols ? \@pcols : [ 1 ];
-}
 
 sub source_bind_attributes {
   my ($self, $source) = @_;
index ef0cdaf..930a3be 100644 (file)
@@ -308,7 +308,6 @@ has 'write_handler' => (
     is_datatype_numeric
     _supports_insert_returning
     _count_select
-    _subq_count_select
     _subq_update_delete
     svp_rollback
     svp_begin
index cd7d2ef..fb745dc 100644 (file)
@@ -146,7 +146,7 @@ lives_ok ( sub {
             JOIN cd cd ON cd.cdid = me.cd_id
             JOIN artist artist_2 ON artist_2.artistid = cd.artist
           GROUP BY me.cd_id
-        ) count_subq
+        ) me
     )',
     [],
   );
index 0f2a1a0..f947a9b 100644 (file)
@@ -54,7 +54,7 @@ my $schema = DBICTest->init_schema();
           JOIN cd disc ON disc.cdid = tracks.cd
         WHERE ( ( position = ? OR position = ? ) )
         LIMIT 3 OFFSET 8
-       ) count_subq
+       ) tracks
     )',
     [ [ position => 1 ], [ position => 2 ] ],
     'count_rs db-side limit applied',
@@ -88,7 +88,7 @@ my $schema = DBICTest->init_schema();
           JOIN artist artist ON artist.artistid = cds.artist
         WHERE tracks.position = ? OR tracks.position = ?
         GROUP BY cds.cdid
-      ) count_subq
+      ) cds
     ',
     [ qw/'1' '2'/ ],
     'count softlimit applied',
@@ -109,7 +109,7 @@ my $schema = DBICTest->init_schema();
         WHERE tracks.position = ? OR tracks.position = ?
         GROUP BY cds.cdid
         LIMIT 3 OFFSET 4
-      ) count_subq
+      ) cds
     )',
     [ [ 'tracks.position' => 1 ], [ 'tracks.position' => 2 ] ],
     'count_rs db-side limit applied',
index 7bc4708..f3818c1 100644 (file)
@@ -31,7 +31,7 @@ my $schema = DBICTest->init_schema();
             JOIN artist artist ON artist.artistid = cds.artist
           WHERE tracks.position = ? OR tracks.position = ?
           GROUP BY cds.cdid
-        ) count_subq
+        ) cds
     )',
     [ map { [ 'tracks.position' => $_ ] } (1, 2) ],
   );
@@ -63,7 +63,7 @@ my $schema = DBICTest->init_schema();
           WHERE ( genre.name = ? )
           GROUP BY genre.genreid
         )
-      count_subq
+      genre
     )',
     [ [ 'genre.name' => 'emo' ] ],
   );
index edb69b6..d2d1f17 100644 (file)
@@ -76,7 +76,7 @@ for ($cd_rs->all) {
           WHERE ( me.cd IN ( ?, ?, ?, ?, ? ) )
           GROUP BY me.cd
         )
-      count_subq
+      me
     )',
     [ map { [ 'me.cd' => $_] } ($cd_rs->get_column ('cdid')->all) ],
     'count() query generated expected SQL',
@@ -151,7 +151,7 @@ for ($cd_rs->all) {
           WHERE ( me.cdid IS NOT NULL )
           GROUP BY me.cdid
           LIMIT 2
-        ) count_subq
+        ) me
     )',
     [],
     'count() query generated expected SQL',
@@ -262,7 +262,7 @@ for ($cd_rs->all) {
           WHERE ( me.cd IN ( ?, ?, ?, ?, ? ) )
           GROUP BY SUBSTR(me.cd, 1, 1)
         )
-      count_subq
+      me
     )',
     [ map { [ 'me.cd' => $_] } ($cd_rs->get_column ('cdid')->all) ],
     'count() query generated expected SQL',