From: Peter Rabbitson Date: Sun, 17 Jan 2010 12:44:00 +0000 (+0000) Subject: Getting warmer X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=539ffe8768e85b2061aa3bb3616da4f848a582f3;p=dbsrgits%2FDBIx-Class-Historic.git Getting warmer --- diff --git a/lib/DBIx/Class/ResultSet.pm b/lib/DBIx/Class/ResultSet.pm index 5827fb4..b0d2e12 100644 --- a/lib/DBIx/Class/ResultSet.pm +++ b/lib/DBIx/Class/ResultSet.pm @@ -1253,16 +1253,15 @@ sub _count_subq_rs { # extra selectors do not go in the subquery and there is no point of ordering it delete $sub_attrs->{$_} for qw/collapse select _prefetch_select as order_by/; - # if we 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 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}} ) { $sub_attrs->{group_by} = [ map { "$attrs->{alias}.$_" } ($rsrc->primary_columns) ] } $sub_attrs->{select} = $rsrc->storage->_subq_count_select ($rsrc, $sub_attrs); # this is so that the query can be simplified e.g. - # * non-limiting joins can be pruned # * ordering can be thrown away in things like Top limit $sub_attrs->{-for_count_only} = 1; @@ -2927,7 +2926,7 @@ sub _joinpath_aliases { my $jpath = $j->[0]{-join_path}; my $p = $paths; - $p = $p->{$_} ||= {} for @{$jpath}[$cur_depth/2 .. $#$jpath]; #only even depths are actual jpath boundaries + $p = $p->{$_} ||= {} for @{$jpath}[ ($cur_depth/2) .. $#$jpath]; #only even depths are actual jpath boundaries push @{$p->{-join_aliases} }, $j->[0]{-alias}; } diff --git a/lib/DBIx/Class/ResultSource.pm b/lib/DBIx/Class/ResultSource.pm index c3b9c74..04b634c 100644 --- a/lib/DBIx/Class/ResultSource.pm +++ b/lib/DBIx/Class/ResultSource.pm @@ -1262,6 +1262,7 @@ sub _resolve_join { : $rel_info->{attrs}{join_type} , -join_path => [@$jpath, $join], + -is_single => (List::Util::first { $rel_info->{attrs}{accessor} eq $_ } (qw/single filter/) ), -alias => $as, -relation_chain_depth => $seen->{-relation_chain_depth} || 0, }, diff --git a/lib/DBIx/Class/Storage/DBI.pm b/lib/DBIx/Class/Storage/DBI.pm index 846decc..2c742a5 100644 --- a/lib/DBIx/Class/Storage/DBI.pm +++ b/lib/DBIx/Class/Storage/DBI.pm @@ -1740,7 +1740,7 @@ sub _select_args { select => $select, from => $ident, where => $where, - $rs_alias + $rs_alias && $alias2source->{$rs_alias} ? ( _source_handle => $alias2source->{$rs_alias}->handle ) : () , @@ -1858,6 +1858,9 @@ sub _select_args { push @limit, $attrs->{rows}, $attrs->{offset}; } + # try to simplify the joinmap further (prune unreferenced type-single joins) + $ident = $self->_prune_unused_joins ($ident, $select, $where, $attrs); + ### # This would be the point to deflate anything found in $where # (and leave $attrs->{bind} intact). Problem is - inflators historically diff --git a/lib/DBIx/Class/Storage/DBIHacks.pm b/lib/DBIx/Class/Storage/DBIHacks.pm index f8dae36..f549204 100644 --- a/lib/DBIx/Class/Storage/DBIHacks.pm +++ b/lib/DBIx/Class/Storage/DBIHacks.pm @@ -17,9 +17,7 @@ use Carp::Clan qw/^DBIx::Class/; # # This code will remove non-selecting/non-restricting joins from -# {from} specs, aiding the RDBMS query optimizer. It will leave any -# unused type-multi joins, if the amount of returned rows is -# important (i.e. count without collapse) +# {from} specs, aiding the RDBMS query optimizer. # sub _prune_unused_joins { my $self = shift; @@ -29,13 +27,17 @@ sub _prune_unused_joins { return $from; # only standard {from} specs are supported } - my $aliastypes = $self->_resolve_aliases_from_select_args($from, @_); + my $aliastypes = $self->_resolve_aliastypes_from_select_args($from, @_); my @newfrom = $from->[0]; # FROM head is always present my %need_joins = (map { %{$_||{}} } (values %$aliastypes) ); for my $j (@{$from}[1..$#$from]) { - push @newfrom, $j if $need_joins{$j->[0]{-alias}}; + push @newfrom, $j if ( + ! $j->[0]{-alias} # legacy crap + || + $need_joins{$j->[0]{-alias}} + ); } return \@newfrom; @@ -92,19 +94,13 @@ sub _adjust_select_args_for_complex_prefetch { # construct the inner $from for the subquery my $inner_from = $self->_prune_unused_joins ($from, $inner_select, $where, $inner_attrs); - # if a multi-type join was needed in the subquery ("multi" is indicated by - # presence in {collapse}) - add a group_by to simulate the collapse in the subq - unless ($inner_attrs->{group_by}) { - for my $alias (map { $_->[0]{-alias} } (@{$inner_from}[1 .. $#$inner_from]) ) { - - # the dot comes from some weirdness in collapse - # remove after the rewrite - if ($attrs->{collapse}{".$alias"}) { - $inner_attrs->{group_by} ||= $inner_select; - last; - } - } - } + # if a multi-type join was needed in the subquery - add a group_by to simulate the + # collapse in the subq + $inner_attrs->{group_by} ||= $inner_select + if List::Util::first + { ! $_->[0]{-is_single} } + (@{$inner_from}[1 .. $#$inner_from]) + ; # generate the subquery my $subq = $self->_select_args_to_query ( @@ -153,7 +149,7 @@ sub _adjust_select_args_for_complex_prefetch { # scan the from spec against different attributes, and see which joins are needed # in what role my $outer_aliastypes = - $self->_resolve_aliases_from_select_args( $from, $outer_select, $where, $outer_attrs ); + $self->_resolve_aliastypes_from_select_args( $from, $outer_select, $where, $outer_attrs ); # see what's left - throw away if not selecting/restricting # also throw in a group_by if restricting to guard against @@ -167,22 +163,7 @@ sub _adjust_select_args_for_complex_prefetch { } elsif ($outer_aliastypes->{restrict}{$alias}) { push @outer_from, $j; - - # FIXME - this should be obviated by SQLA2, as I'll be able to - # have restrict_inner and restrict_outer... or something to that - # effect... I think... - - # FIXME2 - I can't find a clean way to determine if a particular join - # is a multi - instead I am just treating everything as a potential - # explosive join (ribasushi) - # - # if (my $handle = $j->[0]{-source_handle}) { - # my $rsrc = $handle->resolve; - # ... need to bail out of the following if this is not a multi, - # as it will be much easier on the db ... - - $outer_attrs->{group_by} ||= $outer_select; - # } + $outer_attrs->{group_by} ||= $outer_select unless $j->[0]{-is_single}; } } @@ -208,7 +189,7 @@ sub _adjust_select_args_for_complex_prefetch { # happen is for it to fail due to an unqualified column, which in # turn will result in a vocal exception. Qualifying the column will # invariably solve the problem. -sub _resolve_aliases_from_select_args { +sub _resolve_aliastypes_from_select_args { my ( $self, $from, $select, $where, $attrs ) = @_; $self->throw_exception ('Unable to analyze custom {from}') @@ -219,10 +200,15 @@ sub _resolve_aliases_from_select_args { # see what aliases are there to work with my $alias_list; - my @from = @$from; # if I don't copy weird shit happens - for my $j (@from) { + for (@$from) { + my $j = $_; $j = $j->[0] if ref $j eq 'ARRAY'; - $alias_list->{$j->{-alias}} = $j; + my $al = $j->{-alias} + or next; + + $alias_list->{$al} = $j; + $aliases_by_type->{multiplying}{$al} = 1 + unless $j->{-is_single}; } # set up a botched SQLA diff --git a/t/count/count_rs.t b/t/count/count_rs.t index acf696c..0f2a1a0 100644 --- a/t/count/count_rs.t +++ b/t/count/count_rs.t @@ -35,7 +35,6 @@ my $schema = DBICTest->init_schema(); FROM cd me JOIN track tracks ON tracks.cd = me.cdid JOIN cd disc ON disc.cdid = tracks.cd - LEFT JOIN lyrics lyrics ON lyrics.track_id = tracks.trackid WHERE ( ( position = ? OR position = ? ) ) ', [ qw/'1' '2'/ ], @@ -53,7 +52,6 @@ my $schema = DBICTest->init_schema(); FROM cd me JOIN track tracks ON tracks.cd = me.cdid JOIN cd disc ON disc.cdid = tracks.cd - LEFT JOIN lyrics lyrics ON lyrics.track_id = tracks.trackid WHERE ( ( position = ? OR position = ? ) ) LIMIT 3 OFFSET 8 ) count_subq diff --git a/t/count/prefetch.t b/t/count/prefetch.t index ad4b23c..654520b 100644 --- a/t/count/prefetch.t +++ b/t/count/prefetch.t @@ -72,7 +72,7 @@ my $schema = DBICTest->init_schema(); { my $rs = $schema->resultset("CD") ->search_related('tracks', - { position => [1,2] }, + { position => [1,2], 'lyrics.lyric_id' => undef }, { prefetch => [qw/disc lyrics/] }, ); is ($rs->all, 10, 'Correct number of objects'); @@ -88,7 +88,7 @@ my $schema = DBICTest->init_schema(); JOIN track tracks ON tracks.cd = me.cdid JOIN cd disc ON disc.cdid = tracks.cd LEFT JOIN lyrics lyrics ON lyrics.track_id = tracks.trackid - WHERE position = ? OR position = ? + WHERE lyrics.lyric_id IS NULL AND (position = ? OR position = ?) )', [ map { [ position => $_ ] } (1, 2) ], ); diff --git a/t/prefetch/grouped.t b/t/prefetch/grouped.t index a7a16e0..5d0905e 100644 --- a/t/prefetch/grouped.t +++ b/t/prefetch/grouped.t @@ -149,7 +149,6 @@ for ($cd_rs->all) { SELECT me.cdid FROM cd me LEFT JOIN track tracks ON tracks.cd = me.cdid - LEFT JOIN liner_notes liner_notes ON liner_notes.liner_id = me.cdid WHERE ( me.cdid IS NOT NULL ) GROUP BY me.cdid LIMIT 2 diff --git a/t/search/preserve_original_rs.t b/t/search/preserve_original_rs.t index 8ba6d18..8913121 100644 --- a/t/search/preserve_original_rs.t +++ b/t/search/preserve_original_rs.t @@ -89,4 +89,3 @@ for my $s (qw/a2a artw cd artw_back/) { is_same_sql_bind ($rs->as_query, $q{$s}{query}, "$s resultset unmodified (as_query matches)" ); } - diff --git a/t/search/subquery.t b/t/search/subquery.t index 5afc9f3..5d0a255 100644 --- a/t/search/subquery.t +++ b/t/search/subquery.t @@ -76,9 +76,13 @@ my @tests = ( { rs => $art_rs, attrs => { - from => [ { 'me' => 'artist' }, - [ { 'cds' => $cdrs->search({},{ 'select' => [\'me.artist as cds_artist' ]})->as_query }, - { 'me.artistid' => 'cds_artist' } ] ] + from => [ + { 'me' => 'artist' }, + [ + { 'cds' => $cdrs->search({}, { 'select' => [\'me.artist as cds_artist' ]})->as_query }, + { 'me.artistid' => 'cds_artist' } + ] + ] }, sqlbind => \[ "( SELECT me.artistid, me.name, me.rank, me.charfield FROM artist me JOIN (SELECT me.artist as cds_artist FROM cd me) cds ON me.artistid = cds_artist )"