makes search_related on extended rels without the optimized version work. involves...
Daniel Ruoso [Wed, 2 Jun 2010 20:13:24 +0000 (20:13 +0000)]
lib/DBIx/Class/Relationship/Base.pm
lib/DBIx/Class/Relationship/ManyToMany.pm
lib/DBIx/Class/ResultSource.pm
lib/DBIx/Class/SQLMaker.pm
t/lib/DBICTest/Schema/Artist.pm
t/relationship/custom.t

index c23ebdf..71554c7 100644 (file)
@@ -269,7 +269,7 @@ sub related_resultset {
     # resultsource, in that case, the condition might provide an
     # additional condition in order to avoid an unecessary join if
     # that is at all possible.
-    my ($cond, $cond2) = try {
+    my ($cond, $extended_cond) = try {
       $source->_resolve_condition( $rel_info->{cond}, $rel, $self )
     }
     catch {
@@ -292,6 +292,35 @@ sub related_resultset {
         }
       }
     }
+
+    # this is where we're going to check if we have an extended
+    # rel. In that case, we need to: 1) If there's a second
+    # condition, we use that instead.  2) If there is only one
+    # condition, we need to join the current resultsource and have
+    # additional conditions.
+    if (ref $rel_info->{cond} eq 'CODE') {
+      # this is an extended relationship.
+      if ($extended_cond) {
+        $cond = $extended_cond;
+
+      } else {
+
+        # it's a bit hard to find out what to do with other joins
+        $self->throw_exception('Extended relationship '.$rel.' with additional join requires optimized declaration')
+          if exists $attrs->{join} && $attrs->{join};
+
+        # aliases get a bit more complicated, so we won't accept additional queries
+        $self->throw_exception('Extended relationship '.$rel.' with additional query requires optimized declaration')
+          if $query;
+
+        $attrs->{from} =
+          [ { $rel => $self->result_source->from },
+            [ { 'me' => $self->result_source->related_source($rel)->from }, { 1 => 1 } ] ];
+
+        $cond->{"${rel}.${_}"} = $self->get_column($_) for $self->result_source->primary_columns;
+      }
+    }
+
     if (ref $cond eq 'ARRAY') {
       $cond = [ map {
         if (ref $_ eq 'HASH') {
@@ -306,28 +335,8 @@ sub related_resultset {
         }
       } @$cond ];
     } elsif (ref $cond eq 'HASH') {
-
-      # this is where we're going to check if we have an extended
-      # rel. In that case, we need to: 1) If there's a second
-      # condition, we use that instead.  2) If there is only one
-      # condition, we need to join the current resultsource and have
-      # additional conditions.
-      if (ref $rel_info->{cond} eq 'CODE') {
-        # this is an extended relationship.
-        if ($cond2) {
-          $cond = $cond2;
-        } else {
-          if (exists $attrs->{join} && $attrs->{join}) {
-            # it's a bit hard to find out what to do here.
-            $self->throw_exception('Extended relationship '.$rel.' with additional join not supported');
-          } else {
-            $attrs->{join} = $rel;
-          }
-        }
-      } else {
-        foreach my $key (grep { ! /\./ } keys %$cond) {
-          $cond->{"me.$key"} = delete $cond->{$key};
-        }
+      foreach my $key (grep { ! /\./ } keys %$cond) {
+        $cond->{"me.$key"} = delete $cond->{$key};
       }
     }
 
index 6e507c7..c588419 100644 (file)
@@ -135,10 +135,10 @@ EOW
       my $cond = $rel_source->relationship_info($f_rel)->{cond};
       my $link_cond;
       if (ref $cond eq 'CODE') {
-        my ($cond1, $cond2) = $rel_source->_resolve_condition
+        my ($cond_should_join, $cond_optimized) = $rel_source->_resolve_condition
           ($cond, $obj, $f_rel);
-        if ($cond2) {
-          $link_cond = $cond2;
+        if ($cond_optimized) {
+          $link_cond = $cond_optimized;
         } else {
           $self->throw_exception('Extended relationship '.$rel.
                                  ' requires optimized version for ManyToMany.');
index 9d5f0dc..3e74860 100644 (file)
@@ -1568,7 +1568,11 @@ sub _resolve_condition {
       $self->throw_exception ('Unable to determine relationship name for condition resolution');
     }
 
-    return $cond->($for, ref $for ? 'me' : $as, $self, $rel, ref $for ? $for : undef);
+    return $cond->(ref $for ? $as : $for,
+                   ref $for ? 'me' : $as,
+                   $self,
+                   $rel,
+                   ref $for ? $for : undef);
 
   } elsif (ref $cond eq 'HASH') {
     my %ret;
index 287dcc8..3cd017d 100644 (file)
@@ -183,7 +183,7 @@ sub select {
   if ($limit) {
     # this is legacy code-flow from SQLA::Limit, it is not set in stone
 
-    ($sql, @bind) = $self->next::method ($table, $fields, $where);
+    ($sql, @bind) = $self->__overriden_select($table, $fields, $where);
 
     my $limiter =
       $self->can ('emulate_limit')  # also backcompat hook from SQLA::Limit
@@ -199,7 +199,7 @@ sub select {
     $sql = $self->$limiter ($sql, $rs_attrs, $limit, $offset);
   }
   else {
-    ($sql, @bind) = $self->next::method ($table, $fields, $where, $rs_attrs);
+    ($sql, @bind) = $self->__overriden_select($table, $fields, $where, $rs_attrs);
   }
 
   push @{$self->{where_bind}}, @bind;
@@ -213,6 +213,25 @@ sub select {
   return wantarray ? ($sql, @all_bind) : $sql;
 }
 
+sub __overriden_select {
+  my $self   = shift;
+  my ($table, @bind)  = $self->_table(shift);
+  my $fields = shift || '*';
+  my $where  = shift;
+  my $order  = shift;
+
+  my($where_sql, @morebind) = $self->where($where, $order);
+  push @bind, @morebind;
+
+  my $f = (ref $fields eq 'ARRAY') ? join ', ', map { $self->_quote($_) } @$fields
+                                   : $fields;
+  my $sql = join(' ', $self->_sqlcase('select'), $f,
+                      $self->_sqlcase('from'),   $table)
+          . $where_sql;
+
+  return wantarray ? ($sql, @bind) : $sql;
+}
+
 sub _assemble_binds {
   my $self = shift;
   return map { @{ (delete $self->{"${_}_bind"}) || [] } } (qw/select from where group having order/);
@@ -378,7 +397,7 @@ sub _generate_join_clause {
 
 sub _recurse_from {
   my ($self, $from, @join) = @_;
-  my @sqlf;
+  my (@sqlf, @binds);
   push @sqlf, $self->_from_chunk_to_sql($from);
 
   for (@join) {
@@ -397,13 +416,17 @@ sub _recurse_from {
     push @sqlf, $self->_generate_join_clause( $join_type );
 
     if (ref $to eq 'ARRAY') {
-      push(@sqlf, '(', $self->_recurse_from(@$to), ')');
+      my ($sql, @local_bind) = $self->_recurse_from(@$to);
+      push(@sqlf, '(', $sql , ')');
+      push @binds, @local_bind;
     } else {
       push(@sqlf, $self->_from_chunk_to_sql($to));
     }
-    push(@sqlf, ' ON ', $self->_join_condition($on));
+    my ($sql, @local_bind) = $self->_join_condition($on);
+    push(@sqlf, ' ON ', $sql);
+    push @binds, @local_bind;
   }
-  return join('', @sqlf);
+  return join('', @sqlf), @binds;
 }
 
 sub _from_chunk_to_sql {
@@ -450,9 +473,16 @@ sub _join_condition {
         my $x = '= '.$self->_quote($v); $j{$_} = \$x;
       }
     };
-    return scalar($self->_recurse_where(\%j));
+    return $self->_recurse_where(\%j);
   } elsif (ref $cond eq 'ARRAY') {
-    return join(' OR ', map { $self->_join_condition($_) } @$cond);
+    my @parts;
+    my @binds;
+    foreach my $c (@$cond) {
+      my ($sql, @bind) = $self->_join_condition($c);
+      push @binds, @bind;
+      push @parts, $sql;
+    }
+    return join(' OR ', @parts), @binds;
   } else {
     croak "Can't handle this yet!";
   }
index f519fe4..0a20128 100644 (file)
@@ -63,6 +63,30 @@ __PACKAGE__->has_many(
   }
 );
 
+__PACKAGE__->has_many(
+    cds_80s_noopt => 'DBICTest::Schema::CD',
+    sub {
+      my ( $me_alias, $rel_alias, $me_result_source, $rel_name, $optional_me_object ) = @_;
+      return
+        ({ "${rel_alias}.artist"  => { '=' => \"${me_alias}.artistid"},
+           "${rel_alias}.year"    => { '>', "1979",
+                                       '<', "1990" }
+         });
+  }
+);
+
+__PACKAGE__->has_many(
+    cds_90s => 'DBICTest::Schema::CD',
+    sub {
+      my ( $me_alias, $rel_alias, $me_result_source, $rel_name, $optional_me_object ) = @_;
+      return
+        ({ "${rel_alias}.artist"  => { '=' => \"${me_alias}.artistid"},
+           "${rel_alias}.year"    => { '>', "1989",
+                                       '<', "2000" }
+         });
+  }
+);
+
 
 __PACKAGE__->has_many(
     cds_unordered => 'DBICTest::Schema::CD'
index 6d4e85b..c94758c 100644 (file)
@@ -10,19 +10,28 @@ my $schema = DBICTest->init_schema();
 
 
 my $artist = $schema->resultset("Artist")->create({ name => 'Michael Jackson' });
-
 foreach my $year (1975..1985) {
   $artist->create_related('cds', { year => $year, title => 'Compilation from ' . $year });
 }
 
-my @cds_80s = $artist->cds_80s;
+my $artist2 = $schema->resultset("Artist")->create({ name => 'Chico Buarque' }) ;
+foreach my $year (1975..1995) {
+  $artist2->create_related('cds', { year => $year, title => 'Compilation from ' . $year });
+}
 
+my @cds_80s = $artist->cds_80s;
 is(@cds_80s, 6, '6 80s cds found');
 
-map { ok($_->year < 1990 && $_->year > 1979) } @cds_80s;
+my @cds_90s = $artist2->cds_90s;
+is(@cds_90s, 6, '6 90s cds found even with non-optimized search');
 
+map { ok($_->year < 1990 && $_->year > 1979) } @cds_80s;
 
 
+# search for all artists prefetching published cds in the 80s...
+my @all_cds_80s = $schema->resultset("Artist")->search
+  ({ 'cds_80s_noopt.cdid' => { '!=' => undef } }, { join => 'cds_80s_noopt' });
+is(@all_cds_80s, 16, '16 cds found even with the non-optimized search');
 
 my @last_track_ids;
 for my $cd ($schema->resultset('CD')->search ({}, { order_by => 'cdid'})->all) {