Merge 'trunk' into 'prefetch_redux'
Peter Rabbitson [Tue, 4 Aug 2009 10:36:20 +0000 (10:36 +0000)]
r7179@Thesaurus (orig r7176):  ribasushi | 2009-08-03 11:51:42 +0200
 r6983@Thesaurus (orig r6982):  ribasushi | 2009-07-04 11:46:57 +0200
 New branch to experiment with a sanifying mysql on_connect_call
 r6984@Thesaurus (orig r6983):  ribasushi | 2009-07-04 11:49:44 +0200
 Initial set_ansi_mode code - make sure to utilize _do_query instead of dbh->do, so the result is visible in the trace
 r6987@Thesaurus (orig r6986):  ribasushi | 2009-07-04 12:40:47 +0200
 Fix POD
 r7178@Thesaurus (orig r7175):  ribasushi | 2009-08-03 11:51:15 +0200
 Wrap up set_strict_mode for mysql

r7181@Thesaurus (orig r7178):  ribasushi | 2009-08-03 12:41:32 +0200
Sanify unqualified column bindtype handling
Silence a warning when using a custom {from}

Makefile.PL
lib/DBIx/Class/ResultSet.pm
lib/DBIx/Class/Storage/DBI.pm
t/count/prefetch.t
t/lib/DBIC/SqlMakerTest.pm
t/prefetch/grouped.t

index d21951b..832a1be 100644 (file)
@@ -86,6 +86,7 @@ my %force_requires_if_author = (
   # t/93storage_replication.t
   'Moose',                    => 0.87,
   'MooseX::AttributeHelpers'  => 0.21,
+  'MooseX::Traits',           => 0.06,
   'MooseX::Types',            => 0.16,
   'namespace::clean'          => 0.11,
   'Hash::Merge',              => 0.11,
index 9fba9fc..e24dafe 100644 (file)
@@ -1264,7 +1264,7 @@ sub _count_subq_rs {
   my $sub_attrs = { %$attrs };
 
   # extra selectors do not go in the subquery and there is no point of ordering it
-  delete $sub_attrs->{$_} for qw/collapse prefetch_select select as order_by/;
+  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
   # clobber old group_by regardless
@@ -2875,6 +2875,12 @@ sub _resolved_attrs {
     $attrs->{group_by} = [ $attrs->{group_by} ];
   }
 
+  # generate the distinct induced group_by early, as prefetch will be carried via a
+  # subquery (since a group_by is present)
+  if (delete $attrs->{distinct}) {
+    $attrs->{group_by} ||= [ grep { !ref($_) || (ref($_) ne 'HASH') } @{$attrs->{select}} ];
+  }
+
   $attrs->{collapse} ||= {};
   if ( my $prefetch = delete $attrs->{prefetch} ) {
     $prefetch = $self->_merge_attr( {}, $prefetch );
@@ -2886,19 +2892,16 @@ sub _resolved_attrs {
     my @prefetch =
       $source->_resolve_prefetch( $prefetch, $alias, $join_map, $prefetch_ordering, $attrs->{collapse} );
 
-    $attrs->{prefetch_select} = [ map { $_->[0] } @prefetch ];
-    push @{ $attrs->{select} }, @{$attrs->{prefetch_select}};
+    # we need to somehow mark which columns came from prefetch
+    $attrs->{_prefetch_select} = [ map { $_->[0] } @prefetch ];
+
+    push @{ $attrs->{select} }, @{$attrs->{_prefetch_select}};
     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}} ];
-  }
-
   # 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
index 5a97bb3..bfd7edf 100644 (file)
@@ -1443,7 +1443,7 @@ sub _select_args {
     ( $attrs->{rows} && keys %{$attrs->{collapse}} )
        ||
     ( $attrs->{group_by} && @{$attrs->{group_by}} &&
-      $attrs->{prefetch_select} && @{$attrs->{prefetch_select}} )
+      $attrs->{_prefetch_select} && @{$attrs->{_prefetch_select}} )
   ) {
     ($ident, $select, $where, $attrs)
       = $self->_adjust_select_args_for_complex_prefetch ($ident, $select, $where, $attrs);
@@ -1488,14 +1488,15 @@ sub _adjust_select_args_for_complex_prefetch {
   # separate attributes
   my $sub_attrs = { %$attrs };
   delete $attrs->{$_} for qw/where bind rows offset group_by having/;
-  delete $sub_attrs->{$_} for qw/for collapse prefetch_select _collapse_order_by select as/;
+  delete $sub_attrs->{$_} for qw/for collapse _prefetch_select _collapse_order_by select as/;
 
   my $alias = $attrs->{alias};
   my $sql_maker = $self->sql_maker;
 
   # create subquery select list - consider only stuff *not* brought in by the prefetch
   my $sub_select = [];
-  for my $i (0 .. @{$attrs->{select}} - @{$attrs->{prefetch_select}} - 1) {
+  my $sub_group_by;
+  for my $i (0 .. @{$attrs->{select}} - @{$attrs->{_prefetch_select}} - 1) {
     my $sel = $attrs->{select}[$i];
 
     # alias any functions to the dbic-side 'as' label
@@ -1608,13 +1609,15 @@ sub _adjust_select_args_for_complex_prefetch {
 
   # 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;
+  unless ($sub_attrs->{group_by}) {
+    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;
+      }
     }
   }
 
index 2032a4b..925be3d 100644 (file)
@@ -6,9 +6,8 @@ use lib qw(t/lib);
 use Test::More;
 use DBICTest;
 use DBIC::SqlMakerTest;
-use DBIC::DebugObj;
 
-plan tests => 6;
+plan tests => 14;
 
 my $schema = DBICTest->init_schema();
 
@@ -20,20 +19,58 @@ my $schema = DBICTest->init_schema();
                 { prefetch => [qw/tracks artist/] },
             );
   is ($rs->all, 5, 'Correct number of objects');
+  is ($rs->count, 5, 'Correct count');
 
+  is_same_sql_bind (
+    $rs->count_rs->as_query,
+    '(
+      SELECT COUNT( * )
+        FROM (
+          SELECT cds.cdid
+            FROM artist me
+            JOIN cd cds ON cds.artist = me.artistid
+            LEFT JOIN track tracks ON tracks.cd = cds.cdid
+            JOIN artist artist ON artist.artistid = cds.artist
+          WHERE tracks.position = ? OR tracks.position = ?
+          GROUP BY cds.cdid
+        ) count_subq
+    )',
+    [ map { [ 'tracks.position' => $_ ] } (1, 2) ],
+  );
+}
 
-  my ($sql, @bind);
-  $schema->storage->debugobj(DBIC::DebugObj->new(\$sql, \@bind));
-  $schema->storage->debug(1);
-
-
-  is ($rs->count, 5, 'Correct count');
+# collapsing prefetch with distinct
+{
+  my $first_cd = $schema->resultset('Artist')->first->cds->first;
+  $first_cd->update ({
+    genreid => $first_cd->create_related (
+      genre => ({ name => 'vague genre' })
+    )->id
+  });
+
+  my $rs = $schema->resultset("Artist")->search(undef, {distinct => 1})
+            ->search_related('cds')->search_related('genre',
+                { 'genre.name' => { '!=', 'foo' } },
+                { prefetch => q(cds) },
+            );
+  is ($rs->all, 1, 'Correct number of objects');
+  is ($rs->count, 1, 'Correct count');
 
   is_same_sql_bind (
-    $sql,
-    \@bind,
-    'SELECT COUNT( * ) FROM (SELECT cds.cdid FROM artist me JOIN cd cds ON cds.artist = me.artistid LEFT JOIN track tracks ON tracks.cd = cds.cdid JOIN artist artist ON artist.artistid = cds.artist WHERE tracks.position = ? OR tracks.position = ? GROUP BY cds.cdid) count_subq',
-    [ qw/'1' '2'/ ],
+    $rs->count_rs->as_query,
+    '(
+      SELECT COUNT( * )
+        FROM (
+          SELECT genre.genreid
+            FROM artist me
+            JOIN cd cds ON cds.artist = me.artistid
+            JOIN genre genre ON genre.genreid = cds.genreid
+            LEFT JOIN cd cds_2 ON cds_2.genreid = genre.genreid
+          WHERE ( genre.name != ? )
+          GROUP BY genre.genreid
+        ) count_subq
+    )',
+    [ [ 'genre.name' => 'foo' ] ],
   );
 }
 
@@ -47,17 +84,57 @@ my $schema = DBICTest->init_schema();
   is ($rs->all, 10, 'Correct number of objects');
 
 
-  my ($sql, @bind);
-  $schema->storage->debugobj(DBIC::DebugObj->new(\$sql, \@bind));
-  $schema->storage->debug(1);
-
-
   is ($rs->count, 10, 'Correct count');
 
   is_same_sql_bind (
-    $sql,
-    \@bind,
-    'SELECT COUNT( * ) 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'/ ],
+    $rs->count_rs->as_query,
+    '(
+      SELECT COUNT( * )
+        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 = ?
+    )',
+    [ map { [ position => $_ ] } (1, 2) ],
   );
 }
+
+{
+    my $rs = $schema->resultset("Artwork")->search(undef, {distinct => 1})
+              ->search_related('artwork_to_artist')->search_related('artist',
+                 undef,
+                  { prefetch => q(cds) },
+              );
+    is($rs->all, 0, 'failure without WHERE');
+
+    $rs = $schema->resultset("Artwork")->search(undef, {distinct => 1})
+              ->search_related('artwork_to_artist')->search_related('artist',
+                 { 'cds.title' => 'foo' }, # this line has changed
+                  { prefetch => q(cds) },
+              );
+    is($rs->all, 0, 'success with WHERE');
+
+
+    # different case
+    $rs = $schema->resultset("Artist")->search(undef)#, {distinct => 1})
+                ->search_related('cds')->search_related('genre',
+                    { 'genre.name' => 'foo' },
+                    { prefetch => q(cds) },
+                 );
+    is($rs->all, 0, 'success without distinct');
+
+    $rs = $schema->resultset("Artist")->search(undef, {distinct => 1})
+                ->search_related('cds')->search_related('genre',
+                    { 'genre.name' => 'foo' },
+                    #{ prefetch => q(cds) },
+                 );
+    is($rs->all, 0, 'success without prefetch');
+
+    $rs = $schema->resultset("Artist")->search(undef, {distinct => 1})
+                ->search_related('cds')->search_related('genre',
+                    { 'genre.name' => 'foo' },
+                    { prefetch => q(cds) },
+                 );
+    is($rs->all, 0, 'failure with distinct');
+}
index 1098e25..44ccb4b 100644 (file)
@@ -41,7 +41,8 @@ sub is_same_sql_bind {
   croak "Unexpected argument(s) supplied to is_same_sql_bind: " . join ('; ', @_)
     if @_;
 
-  SQL::Abstract::Test::is_same_sql_bind (@args);
+  @_ = @args;
+  goto &SQL::Abstract::Test::is_same_sql_bind;
 }
 
 *is_same_sql = \&SQL::Abstract::Test::is_same_sql;
index 19fa923..1cb670c 100644 (file)
@@ -6,7 +6,6 @@ use lib qw(t/lib);
 use DBICTest;
 use DBIC::SqlMakerTest;
 
-#plan tests => 6;
 plan 'no_plan';
 
 my $schema = DBICTest->init_schema();
@@ -203,3 +202,33 @@ for ($cd_rs->all) {
   $schema->storage->debugcb (undef);
   $schema->storage->debug ($sdebug);
 }
+
+# make sure that distinct still works
+{
+  my $rs = $schema->resultset("CD")->search({}, {
+    prefetch => 'tags',
+    order_by => 'cdid',
+    distinct => 1,
+  });
+
+  is_same_sql_bind (
+    $rs->as_query,
+    '(
+      SELECT me.cdid, me.artist, me.title, me.year, me.genreid, me.single_track,
+             tags.tagid, tags.cd, tags.tag 
+        FROM (
+          SELECT me.cdid, me.artist, me.title, me.year, me.genreid, me.single_track
+            FROM cd me
+          GROUP BY me.cdid, me.artist, me.title, me.year, me.genreid, me.single_track
+          ORDER BY cdid
+        ) me
+        LEFT JOIN tags tags ON tags.cd = me.cdid
+      ORDER BY cdid, tags.cd, tags.tag
+    )',
+    [],
+    'Prefetch + distinct resulted in correct group_by',
+  );
+
+  is ($rs->all, 5, 'Correct number of CD objects');
+  is ($rs->count, 5, 'Correct count of CDs');
+}