From: Peter Rabbitson <ribasushi@cpan.org>
Date: Tue, 6 Apr 2010 03:36:04 +0000 (+0000)
Subject: Fix embarassing join optimizer bug
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=faeb2407c49717d38874b21749659ac2c632bad7;p=dbsrgits%2FDBIx-Class-Historic.git

Fix embarassing join optimizer bug
---

diff --git a/Changes b/Changes
index 9b4f636..d8659c5 100644
--- a/Changes
+++ b/Changes
@@ -17,6 +17,8 @@ Revision history for DBIx::Class
           is unresolvable
         - Fix the join-optimiser to consider unqualified column names
           whenever possible
+        - Fix an issue with multiple same-table joins confusing the join
+          optimizier
         - Add has_relationship method to row objects
         - Fix regression in set_column on PK-less objects
         - Better error text on malformed/missing relationships
diff --git a/lib/DBIx/Class/Storage/DBIHacks.pm b/lib/DBIx/Class/Storage/DBIHacks.pm
index 2d10e6c..32e0ea3 100644
--- a/lib/DBIx/Class/Storage/DBIHacks.pm
+++ b/lib/DBIx/Class/Storage/DBIHacks.pm
@@ -273,7 +273,6 @@ sub _resolve_aliastypes_from_select_args {
         $aliases_by_type->{$type}{$alias} = 1 if ($piece =~ $al_re);
       }
     }
-
   }
 
   # now loop through unqualified column names, and try to locate them within
@@ -308,7 +307,7 @@ sub _resolve_aliastypes_from_select_args {
   for my $type (keys %$aliases_by_type) {
     for my $alias (keys %{$aliases_by_type->{$type}}) {
       $aliases_by_type->{$type}{$_} = 1
-        for (map { keys %$_ } @{ $alias_list->{$alias}{-join_path} || [] });
+        for (map { values %$_ } @{ $alias_list->{$alias}{-join_path} || [] });
     }
   }
 
@@ -453,7 +452,7 @@ sub _straight_join_to_node {
   # anyway, and deep cloning is just too fucking expensive
   # So replace the first hashref in the node arrayref manually 
   my @new_from = ($from->[0]);
-  my $sw_idx = { map { values %$_ => 1 } @$switch_branch };
+  my $sw_idx = { map { (values %$_), 1 } @$switch_branch }; #there's one k/v per join-path
 
   for my $j (@{$from}[1 .. $#$from]) {
     my $jalias = $j->[0]{-alias};
diff --git a/t/90join_torture.t b/t/90join_torture.t
index 6eeda5a..cd7d2ef 100644
--- a/t/90join_torture.t
+++ b/t/90join_torture.t
@@ -1,13 +1,13 @@
 use strict;
-use warnings;  
+use warnings;
 
 use Test::More;
+use Test::Exception;
 use lib qw(t/lib);
 use DBICTest;
+use DBIC::SqlMakerTest;
 my $schema = DBICTest->init_schema();
 
-plan tests => 22;
-
  {
    my $rs = $schema->resultset( 'CD' )->search(
      {
@@ -25,11 +25,10 @@ plan tests => 22;
        ],
      }
    );
-  
-   eval {
+
+   lives_ok {
      my @rows = $rs->all();
    };
-   is( $@, '' );
  }
 
 
@@ -106,7 +105,7 @@ my $merge_rs_2 = $schema->resultset("Artist")->search({ }, { join => 'cds' })->s
 is(scalar(@{$merge_rs_2->{attrs}->{join}}), 1, 'only one join kept when inherited');
 my $merge_rs_2_cd = $merge_rs_2->next;
 
-eval {
+lives_ok (sub {
 
   my @rs_with_prefetch = $schema->resultset('TreeLike')
                                 ->search(
@@ -115,9 +114,7 @@ eval {
     prefetch => [ 'parent', { 'children' => 'parent' } ],
     });
 
-};
-
-ok(!$@, "pathological prefetch ok");
+}, 'pathological prefetch ok');
 
 my $rs = $schema->resultset("Artist")->search({}, { join => 'twokeys' });
 my $second_search_rs = $rs->search({ 'cds_2.cdid' => '2' }, { join =>
@@ -125,4 +122,36 @@ my $second_search_rs = $rs->search({ 'cds_2.cdid' => '2' }, { join =>
 is(scalar(@{$second_search_rs->{attrs}->{join}}), 3, 'both joins kept');
 ok($second_search_rs->next, 'query on double joined rel runs okay');
 
-1;
+# test joinmap pruner
+lives_ok ( sub {
+  my $rs = $schema->resultset('Artwork')->search (
+    {
+    },
+    {
+      distinct => 1,
+      join => [
+        { artwork_to_artist => 'artist' },
+        { cd => 'artist' },
+      ],
+    },
+  );
+
+  is_same_sql_bind (
+    $rs->count_rs->as_query,
+    '(
+      SELECT COUNT( * )
+        FROM (
+          SELECT me.cd_id
+            FROM cd_artwork me
+            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
+    )',
+    [],
+  );
+
+  ok (defined $rs->count);
+});
+
+done_testing;