[Bast-commits] r9297 - in DBIx-Class/0.08/trunk: lib/DBIx/Class lib/DBIx/Class/Manual lib/DBIx/Class/Storage lib/DBIx/Class/Storage/DBI t t/count t/prefetch

ribasushi at dev.catalyst.perl.org ribasushi at dev.catalyst.perl.org
Tue May 4 08:00:11 GMT 2010


Author: ribasushi
Date: 2010-05-04 09:00:11 +0100 (Tue, 04 May 2010)
New Revision: 9297

Modified:
   DBIx-Class/0.08/trunk/lib/DBIx/Class/Manual/Cookbook.pod
   DBIx-Class/0.08/trunk/lib/DBIx/Class/ResultSet.pm
   DBIx-Class/0.08/trunk/lib/DBIx/Class/Storage/DBI.pm
   DBIx-Class/0.08/trunk/lib/DBIx/Class/Storage/DBI/Replicated.pm
   DBIx-Class/0.08/trunk/t/90join_torture.t
   DBIx-Class/0.08/trunk/t/count/count_rs.t
   DBIx-Class/0.08/trunk/t/count/prefetch.t
   DBIx-Class/0.08/trunk/t/prefetch/grouped.t
Log:
Refactor count handling, make count-resultset attribute lists inclusive rather than exclusive (side effect - solves RT#56257

Modified: DBIx-Class/0.08/trunk/lib/DBIx/Class/Manual/Cookbook.pod
===================================================================
--- DBIx-Class/0.08/trunk/lib/DBIx/Class/Manual/Cookbook.pod	2010-05-04 07:44:47 UTC (rev 9296)
+++ DBIx-Class/0.08/trunk/lib/DBIx/Class/Manual/Cookbook.pod	2010-05-04 08:00:11 UTC (rev 9297)
@@ -292,7 +292,7 @@
   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
 

Modified: DBIx-Class/0.08/trunk/lib/DBIx/Class/ResultSet.pm
===================================================================
--- DBIx-Class/0.08/trunk/lib/DBIx/Class/ResultSet.pm	2010-05-04 07:44:47 UTC (rev 9296)
+++ DBIx-Class/0.08/trunk/lib/DBIx/Class/ResultSet.pm	2010-05-04 08:00:11 UTC (rev 9297)
@@ -1235,11 +1235,12 @@
   my $rsrc = $self->result_source;
   $attrs ||= $self->_resolved_attrs;
 
-  my $tmp_attrs = { %$attrs };
+  # only take pieces we need for a simple count
+  my $tmp_attrs = { map
+    { $_ => $attrs->{$_} }
+    qw/ alias from where bind join /
+  };
 
-  # 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 (supplied by the storage)
   $tmp_attrs->{select} = $rsrc->storage->_count_select ($rsrc, $tmp_attrs);
   $tmp_attrs->{as} = 'count';
@@ -1256,37 +1257,48 @@
   my ($self, $attrs) = @_;
 
   my $rsrc = $self->result_source;
-  $attrs ||= $self->_resolved_attrs_copy;
+  $attrs ||= $self->_resolved_attrs;
 
-  my $sub_attrs = { %$attrs };
+  my $sub_attrs = { map
+    { $_ => $attrs->{$_} }
+    qw/ alias from where bind join group_by having rows offset /
+  };
 
-  # 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 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->_pri_cols) ]
   }
 
-  $sub_attrs->{select} = $rsrc->storage->_subq_count_select ($rsrc, $attrs);
+  # Calculate subquery selector
+  if (my $g = $sub_attrs->{group_by}) {
 
+    # 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});
+    }
+
+    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 ];
+  }
+
+
   # 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;
 
-  my $sub_rs = $rsrc->resultset_class->new ($rsrc, $sub_attrs);
-
-  $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/;
-
-  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 @@
 =cut
 
 sub as_subselect_rs {
-   my $self = shift;
+  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 $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 @@
   # ->_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 @@
       -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

Modified: DBIx-Class/0.08/trunk/lib/DBIx/Class/Storage/DBI/Replicated.pm
===================================================================
--- DBIx-Class/0.08/trunk/lib/DBIx/Class/Storage/DBI/Replicated.pm	2010-05-04 07:44:47 UTC (rev 9296)
+++ DBIx-Class/0.08/trunk/lib/DBIx/Class/Storage/DBI/Replicated.pm	2010-05-04 08:00:11 UTC (rev 9297)
@@ -308,7 +308,6 @@
     is_datatype_numeric
     _supports_insert_returning
     _count_select
-    _subq_count_select
     _subq_update_delete
     svp_rollback
     svp_begin

Modified: DBIx-Class/0.08/trunk/lib/DBIx/Class/Storage/DBI.pm
===================================================================
--- DBIx-Class/0.08/trunk/lib/DBIx/Class/Storage/DBI.pm	2010-05-04 07:44:47 UTC (rev 9296)
+++ DBIx-Class/0.08/trunk/lib/DBIx/Class/Storage/DBI.pm	2010-05-04 08:00:11 UTC (rev 9297)
@@ -2017,47 +2017,7 @@
   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) = @_;
 

Modified: DBIx-Class/0.08/trunk/t/90join_torture.t
===================================================================
--- DBIx-Class/0.08/trunk/t/90join_torture.t	2010-05-04 07:44:47 UTC (rev 9296)
+++ DBIx-Class/0.08/trunk/t/90join_torture.t	2010-05-04 08:00:11 UTC (rev 9297)
@@ -146,7 +146,7 @@
             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
     )',
     [],
   );

Modified: DBIx-Class/0.08/trunk/t/count/count_rs.t
===================================================================
--- DBIx-Class/0.08/trunk/t/count/count_rs.t	2010-05-04 07:44:47 UTC (rev 9296)
+++ DBIx-Class/0.08/trunk/t/count/count_rs.t	2010-05-04 08:00:11 UTC (rev 9297)
@@ -54,7 +54,7 @@
           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 @@
           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 @@
         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',

Modified: DBIx-Class/0.08/trunk/t/count/prefetch.t
===================================================================
--- DBIx-Class/0.08/trunk/t/count/prefetch.t	2010-05-04 07:44:47 UTC (rev 9296)
+++ DBIx-Class/0.08/trunk/t/count/prefetch.t	2010-05-04 08:00:11 UTC (rev 9297)
@@ -31,7 +31,7 @@
             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 @@
           WHERE ( genre.name = ? )
           GROUP BY genre.genreid
         )
-      count_subq
+      genre
     )',
     [ [ 'genre.name' => 'emo' ] ],
   );

Modified: DBIx-Class/0.08/trunk/t/prefetch/grouped.t
===================================================================
--- DBIx-Class/0.08/trunk/t/prefetch/grouped.t	2010-05-04 07:44:47 UTC (rev 9296)
+++ DBIx-Class/0.08/trunk/t/prefetch/grouped.t	2010-05-04 08:00:11 UTC (rev 9297)
@@ -76,7 +76,7 @@
           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 @@
           WHERE ( me.cdid IS NOT NULL )
           GROUP BY me.cdid
           LIMIT 2
-        ) count_subq
+        ) me
     )',
     [],
     'count() query generated expected SQL',
@@ -262,7 +262,7 @@
           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',




More information about the Bast-commits mailing list