Joins work for search, some refactoring
Matt S Trout [Fri, 9 Sep 2005 05:25:57 +0000 (05:25 +0000)]
Build.PL
lib/DBIx/Class/CDBICompat/ImaDBI.pm
lib/DBIx/Class/Relationship/Base.pm
lib/DBIx/Class/ResultSet.pm
lib/DBIx/Class/Row.pm
t/16joins.t
t/lib/DBICTest/Schema/CD.pm

index fd130ca..e26b0a7 100644 (file)
--- a/Build.PL
+++ b/Build.PL
@@ -15,6 +15,7 @@ my %arguments = (
         'SQL::Abstract::Limit'      => 0.033,
         'DBD::SQLite'               => 1.08,
        'Tie::IxHash'               => 0,
+        'Storable'                  => 0,
     },
     create_makefile_pl => 'passthrough',
     create_readme      => 1,
index 556f211..b77ba18 100644 (file)
@@ -47,9 +47,8 @@ __PACKAGE__->mk_classdata('_transform_sql_handlers' =>
           _aliases => { self => $from, foreign => $to },
           _action => 'join',
         };
-        my $join = $from_class->storage->sql_maker->where(
+        my $join = $from_class->storage->sql_maker->_join_condition(
           $from_class->resolve_condition($rel_obj->{cond}, $attrs) );
-        $join =~ s/^\s*WHERE//i;
         return $join;
       }
         
index ba71b30..0b12a3b 100644 (file)
@@ -38,10 +38,7 @@ sub add_relationship {
   #warn %{$f_class->_columns};
 
   return unless eval { %{$f_class->_columns}; }; # Foreign class not loaded
-  my %join = (%$attrs, _action => 'join',
-    _aliases => { 'self' => 'me', 'foreign' => $rel },
-    _classes => { 'me' => $class, $rel => $f_class });
-  eval { $class->resolve_condition($cond, \%join) };
+  eval { $class->_resolve_join($rel, 'me') };
 
   if ($@) { # If the resolve failed, back out and re-throw the error
     delete $rels{$rel}; # 
@@ -51,6 +48,29 @@ sub add_relationship {
   1;
 }
 
+sub _resolve_join {
+  my ($class, $join, $alias) = @_;
+  if (ref $join eq 'ARRAY') {
+    return map { $class->_resolve_join($_, $alias) } @$join;
+  } elsif (ref $join eq 'HASH') {
+    return map { $class->_resolve_join($_, $alias),
+                 $class->_relationships->{$_}{class}->_resolve_join($join->{$_}, $_) }
+           keys %$join;
+  } elsif (ref $join) {
+    $class->throw("No idea how to resolve join reftype ".ref $join);
+  } else {
+    my $rel_obj = $class->_relationships->{$join};
+    $class->throw("No such relationship ${join}") unless $rel_obj;
+    my $j_class = $rel_obj->{class};
+    my %join = (_action => 'join',
+         _aliases => { 'self' => $alias, 'foreign' => $join },
+         _classes => { $alias => $class, $join => $j_class });
+    my $j_cond = $j_class->resolve_condition($rel_obj->{cond}, \%join);
+    return [ { $join => $j_class->_table_name,
+               -join_type => $rel_obj->{attrs}{join_type} || '' }, $j_cond ];
+  }
+}
+
 sub resolve_condition {
   my ($self, $cond, $attrs) = @_;
   if (ref $cond eq 'HASH') {
@@ -110,10 +130,7 @@ sub _cond_value {
       my $class = $attrs->{_classes}{$alias};
       $self->throw("Unknown column $field on $class as $alias")
         unless exists $class->_columns->{$field};
-      my $ret = join('.', $alias, $field);
-      # return { '=' => \$ret }; # SQL::Abstract doesn't handle this yet :(
-      $ret = " = ${ret}";
-      return \$ret;
+      return join('.', $alias, $field);
     } else {
       $self->throw( "Unable to resolve type ${type}: only have aliases for ".
             join(', ', keys %{$attrs->{_aliases} || {}}) );
index f2a0b37..e8513b2 100644 (file)
@@ -12,11 +12,16 @@ sub new {
   #use Data::Dumper; warn Dumper(@_);
   $it_class = ref $it_class if ref $it_class;
   $attrs = { %{ $attrs || {} } };
-  my $cols = [ $db_class->_select_columns ];
+  if ($attrs->{join}) {
+    $attrs->{from} = [ { 'me' => $db_class->_table_name },
+                         $db_class->_resolve_join($attrs->{join}, 'me') ];
+    $attrs->{cols} = [ map { "me.$_" } $db_class->_select_columns ];
+  }
   my $new = {
     class => $db_class,
-    cols => $cols,
+    cols => $attrs->{cols} || [ $db_class->_select_columns ],
     cond => $attrs->{where},
+    from => $attrs->{from} || $db_class->_table_name,
     count => undef,
     pager => undef,
     attrs => $attrs };
@@ -33,7 +38,7 @@ sub cursor {
     $attrs->{offset} = $self->pager->skipped;
   }
   return $self->{cursor}
-    ||= $db_class->storage->select($db_class->_table_name, $self->{cols},
+    ||= $db_class->storage->select($self->{from}, $self->{cols},
           $attrs->{where},$attrs);
 }
 
@@ -63,7 +68,7 @@ sub count {
     delete $attrs->{$_} for qw/offset order_by/;
         
     my @cols = 'COUNT(*)';
-    $self->{count} = $db_class->storage->select_single($db_class->_table_name, \@cols,
+    $self->{count} = $db_class->storage->select_single($self->{from}, \@cols,
                                               $self->{cond}, $attrs);
   }
   return 0 unless $self->{count};
index f789b29..cb086c8 100644 (file)
@@ -211,7 +211,11 @@ sub store_column {
 sub _row_to_object {
   my ($class, $cols, $row) = @_;
   my %vals;
-  $vals{$cols->[$_]} = $row->[$_] for 0 .. $#$cols;
+  foreach my $pos (0..$#$cols) {
+    my $c = $cols->[$pos];
+    $c =~ s/^me\.//;
+    $vals{$c} = $row->[$pos];
+  }
   my $new = $class->new(\%vals);
   $new->in_storage(1);
   return $new;
index 1faea6e..d8475a5 100644 (file)
@@ -5,7 +5,7 @@ BEGIN {
     eval "use DBD::SQLite";
     plan $@
         ? ( skip_all => 'needs DBD::SQLite for testing' )
-        : ( tests => 4 );
+        : ( tests => 12 );
 }
 
 use lib qw(t/lib);
@@ -53,3 +53,41 @@ my $match = 'person child INNER JOIN person father ON ( father.person_id = '
           ;
 
 is( $sa->_recurse_from(@j3), $match, 'join 3 (inner join) ok');
+
+my $rs = DBICTest::CD->search(
+           { 'year' => 2001, 'artist.name' => 'Caterwauler McCrae' },
+           { from => [ { 'cd' => 'cd' },
+                         [
+                           { artist => 'artist' },
+                           { 'cd.artist' => 'artist.artistid' }
+                         ] ] }
+         );
+
+cmp_ok( $rs->count, '==', 1, "Single record in resultset");
+
+is($rs->first->title, 'Forkful of bees', 'Correct record returned');
+
+$rs = DBICTest::CD->search(
+           { 'year' => 2001, 'artist.name' => 'Caterwauler McCrae' },
+           { join => 'artist' });
+
+cmp_ok( $rs->count, '==', 1, "Single record in resultset");
+
+is($rs->first->title, 'Forkful of bees', 'Correct record returned');
+
+$rs = DBICTest::CD->search(
+           { 'artist.name' => 'We Are Goth',
+             'liner_notes.notes' => 'Kill Yourself!' },
+           { join => [ qw/artist liner_notes/ ] });
+
+cmp_ok( $rs->count, '==', 1, "Single record in resultset");
+
+is($rs->first->title, 'Come Be Depressed With Us', 'Correct record returned');
+
+$rs = DBICTest::Artist->search(
+        { 'liner_notes.notes' => 'Kill Yourself!' },
+        { join => { 'cds' => 'liner_notes' } });
+
+cmp_ok( $rs->count, '==', 1, "Single record in resultset");
+
+is($rs->first->name, 'We Are Goth', 'Correct record returned');
index 7196722..d6b130a 100644 (file)
@@ -18,5 +18,9 @@ DBICTest::Schema::CD->add_relationship(
     { 'foreign.cd' => 'self.cdid' }
 );
 #DBICTest::Schema::CD->might_have(liner_notes => 'DBICTest::Schema::LinerNotes' => qw/notes/);
+DBICTest::Schema::CD->add_relationship(
+    liner_notes => 'DBICTest::Schema::LinerNotes',
+    { 'foreign.liner_id' => 'self.cdid' },
+    { join_type => 'LEFT' });
 
 1;