From: Peter Rabbitson Date: Wed, 17 Feb 2010 07:43:28 +0000 (+0000) Subject: Now collapse is a flag, not a list X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=22e40557eee805a1f3269981bc205a6afb159b1f;p=dbsrgits%2FDBIx-Class-Historic.git Now collapse is a flag, not a list --- diff --git a/lib/DBIx/Class/ResultSet.pm b/lib/DBIx/Class/ResultSet.pm index cbf8be4..0cc7cd1 100644 --- a/lib/DBIx/Class/ResultSet.pm +++ b/lib/DBIx/Class/ResultSet.pm @@ -536,7 +536,7 @@ sub find { # Run the query my $rs = $self->search ($query, {result_class => $self->result_class, %$attrs}); - if (keys %{$rs->_resolved_attrs->{collapse}}) { + if ($rs->_resolved_attrs->{collapse}) { my $row = $rs->next; carp "Query returned more than one row" if $rs->next; return $row; @@ -723,7 +723,7 @@ sub single { my $attrs = $self->_resolved_attrs_copy; - if (keys %{$attrs->{collapse}}) { + if ($attrs->{collapse}) { $self->throw_exception( 'single() can not be used on resultsets prefetching has_many. Use find( \%cond ) or next() instead' ); @@ -1001,7 +1001,7 @@ sub _collapse_result { } return undef unless $has_def; - my $collapse = keys %{ $self->{_attrs}{collapse} || {} }; + my $collapse = $self->_resolved_attrs->{collapse}; my $rows = []; my @row = @$row_ref; do { @@ -1236,7 +1236,7 @@ sub _count_subq_rs { # if we multi-prefetch we group_by primary keys only as this is what we would # get out of the rs via ->next/->all. We *DO WANT* to clobber old group_by regardless - if ( keys %{$attrs->{collapse}} ) { + if ($attrs->{collapse}) { $sub_attrs->{group_by} = [ map { "$attrs->{alias}.$_" } ($rsrc->primary_columns) ] } @@ -1306,7 +1306,7 @@ sub all { my @obj; - if (keys %{$self->_resolved_attrs->{collapse}}) { + if ($self->_resolved_attrs->{collapse}) { # Using $self->cursor->all is really just an optimisation. # If we're collapsing has_many prefetches it probably makes # very little difference, and this is cleaner than hacking @@ -2841,7 +2841,6 @@ sub _resolved_attrs { } } else { - # otherwise we intialise select & as to empty $attrs->{select} = []; $attrs->{as} = []; @@ -2931,9 +2930,8 @@ sub _resolved_attrs { } } - $attrs->{collapse} ||= {}; if ( my $prefetch = delete $attrs->{prefetch} ) { - $prefetch = $self->_merge_attr( {}, $prefetch ); + $attrs->{collapse} = 1; my $prefetch_ordering = []; @@ -2958,8 +2956,7 @@ sub _resolved_attrs { } } - my @prefetch = - $source->_resolve_prefetch( $prefetch, $alias, $join_map, $prefetch_ordering, $attrs->{collapse} ); + my @prefetch = $source->_resolve_prefetch( $prefetch, $alias, $join_map, $prefetch_ordering ); # we need to somehow mark which columns came from prefetch $attrs->{_prefetch_select} = [ map { $_->[0] } @prefetch ]; @@ -2971,6 +2968,30 @@ sub _resolved_attrs { $attrs->{_collapse_order_by} = \@$prefetch_ordering; } + # run through the resulting joinstructure (starting from our current slot) + # and unset collapse if proven unnesessary + if ($attrs->{collapse} && ref $attrs->{from} eq 'ARRAY') { + + if (@{$attrs->{from}} > 1) { + + # find where our table-spec starts and consider only things after us + my @fromlist = @{$attrs->{from}}; + while (@fromlist) { + my $t = shift @fromlist; + $t = $t->[0] if ref $t eq 'ARRAY'; #me vs join from-spec mismatch + last if ($t->{-alias} && $t->{-alias} eq $alias); + } + + if (@fromlist) { + $attrs->{collapse} = scalar grep { ! $_->[0]{-is_single} } (@fromlist); + } + } + else { + # no joins - no collapse + $attrs->{collapse} = 0; + } + } + # 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 diff --git a/lib/DBIx/Class/ResultSetColumn.pm b/lib/DBIx/Class/ResultSetColumn.pm index e11f868..78ac9ba 100644 --- a/lib/DBIx/Class/ResultSetColumn.pm +++ b/lib/DBIx/Class/ResultSetColumn.pm @@ -44,7 +44,7 @@ sub new { $rs->throw_exception('column must be supplied') unless $column; - my $orig_attrs = $rs->_resolved_attrs; + my $orig_attrs = $rs->_resolved_attrs_copy; # If $column can be found in the 'as' list of the parent resultset, use the # corresponding element of its 'select' list (to keep any custom column @@ -91,7 +91,7 @@ sub new { # {collapse} would mean a has_many join was injected, which in turn means # we need to group *IF WE CAN* (only if the column in question is unique) - if (!$new_attrs->{group_by} && keys %{$orig_attrs->{collapse}}) { + if (!$new_attrs->{group_by} && $orig_attrs->{collapse}) { # scan for a constraint that would contain our column only - that'd be proof # enough it is unique diff --git a/lib/DBIx/Class/ResultSource.pm b/lib/DBIx/Class/ResultSource.pm index 9265d32..c1d9c3c 100644 --- a/lib/DBIx/Class/ResultSource.pm +++ b/lib/DBIx/Class/ResultSource.pm @@ -1378,7 +1378,7 @@ sub _resolve_condition { # in the supplied relationships. sub _resolve_prefetch { - my ($self, $pre, $alias, $alias_map, $order, $collapse, $pref_path) = @_; + my ($self, $pre, $alias, $alias_map, $order, $pref_path) = @_; $pref_path ||= []; if (not defined $pre) { @@ -1386,15 +1386,15 @@ sub _resolve_prefetch { } elsif( ref $pre eq 'ARRAY' ) { return - map { $self->_resolve_prefetch( $_, $alias, $alias_map, $order, $collapse, [ @$pref_path ] ) } + map { $self->_resolve_prefetch( $_, $alias, $alias_map, $order, [ @$pref_path ] ) } @$pre; } elsif( ref $pre eq 'HASH' ) { my @ret = map { - $self->_resolve_prefetch($_, $alias, $alias_map, $order, $collapse, [ @$pref_path ] ), + $self->_resolve_prefetch($_, $alias, $alias_map, $order, [ @$pref_path ] ), $self->related_source($_)->_resolve_prefetch( - $pre->{$_}, "${alias}.$_", $alias_map, $order, $collapse, [ @$pref_path, $_] ) + $pre->{$_}, "${alias}.$_", $alias_map, $order, [ @$pref_path, $_] ) } keys %$pre; return @ret; } @@ -1427,14 +1427,11 @@ sub _resolve_prefetch { #my @col = map { (/^self\.(.+)$/ ? ("${as_prefix}.$1") : ()); } # values %{$rel_info->{cond}}; - $collapse->{".${as_prefix}${pre}"} = [ $rel_source->primary_columns ]; - # action at a distance. prepending the '.' allows simpler code - # in ResultSet->_collapse_result my @key = map { (/^foreign\.(.+)$/ ? ($1) : ()); } keys %{$rel_info->{cond}}; my @ord = (ref($rel_info->{attrs}{order_by}) eq 'ARRAY' ? @{$rel_info->{attrs}{order_by}} - + : (defined $rel_info->{attrs}{order_by} ? ($rel_info->{attrs}{order_by}) : ())); diff --git a/lib/DBIx/Class/Storage/DBI.pm b/lib/DBIx/Class/Storage/DBI.pm index 0030431..b8dafcf 100644 --- a/lib/DBIx/Class/Storage/DBI.pm +++ b/lib/DBIx/Class/Storage/DBI.pm @@ -1793,8 +1793,8 @@ sub _select_args { # see if we need to tear the prefetch apart otherwise delegate the limiting to the # storage, unless software limit was requested if ( - #limited has_many - ( $attrs->{rows} && keys %{$attrs->{collapse}} ) + # limited collapsing has_many + ( $attrs->{rows} && $attrs->{collapse} ) || # limited prefetch with RNO subqueries ( diff --git a/t/90join_torture.t b/t/90join_torture.t index 6eeda5a..90a78b2 100644 --- a/t/90join_torture.t +++ b/t/90join_torture.t @@ -1,37 +1,64 @@ 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( - { - 'producer.name' => 'blah', - 'producer_2.name' => 'foo', - }, - { - 'join' => [ - { cd_to_producer => 'producer' }, - { cd_to_producer => 'producer' }, - ], - 'prefetch' => [ - 'artist', - { cd_to_producer => 'producer' }, - ], - } - ); - - eval { - my @rows = $rs->all(); - }; - is( $@, '' ); - } - +lives_ok (sub { + my $rs = $schema->resultset( 'CD' )->search( + { + 'producer.name' => 'blah', + 'producer_2.name' => 'foo', + }, + { + 'join' => [ + { cd_to_producer => 'producer' }, + { cd_to_producer => 'producer' }, + ], + 'prefetch' => [ + 'artist', + { cd_to_producer => { producer => 'producer_to_cd' } }, + ], + } + ); + + my @executed = $rs->all(); + + is_same_sql_bind ( + $rs->as_query, + '( + SELECT me.cdid, me.artist, me.title, me.year, me.genreid, me.single_track, + artist.artistid, artist.name, artist.rank, artist.charfield, + cd_to_producer.cd, cd_to_producer.producer, cd_to_producer.attribute, + producer.producerid, producer.name, + producer_to_cd.cd, producer_to_cd.producer, producer_to_cd.attribute + FROM cd me + LEFT JOIN cd_to_producer cd_to_producer + ON cd_to_producer.cd = me.cdid + LEFT JOIN producer producer + ON producer.producerid = cd_to_producer.producer + LEFT JOIN cd_to_producer producer_to_cd + ON producer_to_cd.producer = producer.producerid + LEFT JOIN cd_to_producer cd_to_producer_2 + ON cd_to_producer_2.cd = me.cdid + LEFT JOIN producer producer_2 + ON producer_2.producerid = cd_to_producer_2.producer + JOIN artist artist ON artist.artistid = me.artist + WHERE ( ( producer.name = ? AND producer_2.name = ? ) ) + ORDER BY cd_to_producer.cd, producer_to_cd.producer + )', + [ + [ 'producer.name' => 'blah' ], + [ 'producer_2.name' => 'foo' ], + ], + ); + +}, 'Complex join parsed/executed properly'); my @rs1a_results = $schema->resultset("Artist")->search_related('cds', {title => 'Forkful of bees'}, {order_by => 'title'}); is($rs1a_results[0]->title, 'Forkful of bees', "bare field conditions okay after search related"); @@ -125,4 +152,4 @@ 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; +done_testing;