From: Matt S Trout Date: Fri, 9 Sep 2005 05:25:57 +0000 (+0000) Subject: Joins work for search, some refactoring X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=fef5d100c208d604c7a7b3c33eb6e32946d71848;p=dbsrgits%2FDBIx-Class-Historic.git Joins work for search, some refactoring --- diff --git a/Build.PL b/Build.PL index fd130ca..e26b0a7 100644 --- 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, diff --git a/lib/DBIx/Class/CDBICompat/ImaDBI.pm b/lib/DBIx/Class/CDBICompat/ImaDBI.pm index 556f211..b77ba18 100644 --- a/lib/DBIx/Class/CDBICompat/ImaDBI.pm +++ b/lib/DBIx/Class/CDBICompat/ImaDBI.pm @@ -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; } diff --git a/lib/DBIx/Class/Relationship/Base.pm b/lib/DBIx/Class/Relationship/Base.pm index ba71b30..0b12a3b 100644 --- a/lib/DBIx/Class/Relationship/Base.pm +++ b/lib/DBIx/Class/Relationship/Base.pm @@ -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} || {}}) ); diff --git a/lib/DBIx/Class/ResultSet.pm b/lib/DBIx/Class/ResultSet.pm index f2a0b37..e8513b2 100644 --- a/lib/DBIx/Class/ResultSet.pm +++ b/lib/DBIx/Class/ResultSet.pm @@ -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}; diff --git a/lib/DBIx/Class/Row.pm b/lib/DBIx/Class/Row.pm index f789b29..cb086c8 100644 --- a/lib/DBIx/Class/Row.pm +++ b/lib/DBIx/Class/Row.pm @@ -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; diff --git a/t/16joins.t b/t/16joins.t index 1faea6e..d8475a5 100644 --- a/t/16joins.t +++ b/t/16joins.t @@ -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'); diff --git a/t/lib/DBICTest/Schema/CD.pm b/t/lib/DBICTest/Schema/CD.pm index 7196722..d6b130a 100644 --- a/t/lib/DBICTest/Schema/CD.pm +++ b/t/lib/DBICTest/Schema/CD.pm @@ -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;