From: Dagfinn Ilmari Mannsåker Date: Tue, 19 Aug 2014 11:10:08 +0000 (+0100) Subject: Delay creation of single-row prefetched resultsets when inflating X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=7dbcc8a6806dcc6f347a942c7484ca2cd5a380a0;hp=74b5397c077a964ed301d18f5ccda72afa91f353;p=dbsrgits%2FDBIx-Class.git Delay creation of single-row prefetched resultsets when inflating The RS is only needed if the user does ->related_resultset($name), and creating it is taking up a significant amount of time when prefetching belongs_to rels on many rows. --- diff --git a/lib/DBIx/Class/Relationship/Base.pm b/lib/DBIx/Class/Relationship/Base.pm index 1587403..ec525c5 100644 --- a/lib/DBIx/Class/Relationship/Base.pm +++ b/lib/DBIx/Class/Relationship/Base.pm @@ -534,71 +534,88 @@ sub related_resultset { }; # keep in mind that the following if() block is part of a do{} - no return()s!!! - if ($is_crosstable and ref $rel_info->{cond} eq 'CODE') { - - # A WHOREIFFIC hack to reinvoke the entire condition resolution - # with the correct alias. Another way of doing this involves a - # lot of state passing around, and the @_ positions are already - # mapped out, making this crap a less icky option. - # - # The point of this exercise is to retain the spirit of the original - # $obj->search_related($rel) where the resulting rset will have the - # root alias as 'me', instead of $rel (as opposed to invoking - # $rs->search_related) - - # make the fake 'me' rel - local $rsrc->{_relationships}{me} = { - %{ $rsrc->{_relationships}{$rel} }, - _original_name => $rel, - }; - - my $obj_table_alias = lc($rsrc->source_name) . '__row'; - $obj_table_alias =~ s/\W+/_/g; - - $rsrc->resultset->search( - $self->ident_condition($obj_table_alias), - { alias => $obj_table_alias }, - )->search_related('me', $query, $attrs) - } - else { - # FIXME - this conditional doesn't seem correct - got to figure out - # at some point what it does. Also the entire UNRESOLVABLE_CONDITION - # business seems shady - we could simply not query *at all* - if ($cond eq UNRESOLVABLE_CONDITION) { - my $reverse = $rsrc->reverse_relationship_info($rel); - foreach my $rev_rel (keys %$reverse) { - if ($reverse->{$rev_rel}{attrs}{accessor} && $reverse->{$rev_rel}{attrs}{accessor} eq 'multi') { - weaken($attrs->{related_objects}{$rev_rel}[0] = $self); - } else { - weaken($attrs->{related_objects}{$rev_rel} = $self); + my $rs = do { + if ($is_crosstable and ref $rel_info->{cond} eq 'CODE') { + + # A WHOREIFFIC hack to reinvoke the entire condition resolution + # with the correct alias. Another way of doing this involves a + # lot of state passing around, and the @_ positions are already + # mapped out, making this crap a less icky option. + # + # The point of this exercise is to retain the spirit of the original + # $obj->search_related($rel) where the resulting rset will have the + # root alias as 'me', instead of $rel (as opposed to invoking + # $rs->search_related) + + # make the fake 'me' rel + local $rsrc->{_relationships}{me} = { + %{ $rsrc->{_relationships}{$rel} }, + _original_name => $rel, + }; + + my $obj_table_alias = lc($rsrc->source_name) . '__row'; + $obj_table_alias =~ s/\W+/_/g; + + $rsrc->resultset->search( + $self->ident_condition($obj_table_alias), + { alias => $obj_table_alias }, + )->search_related('me', $query, $attrs) + } + else { + # FIXME - this conditional doesn't seem correct - got to figure out + # at some point what it does. Also the entire UNRESOLVABLE_CONDITION + # business seems shady - we could simply not query *at all* + if ($cond eq UNRESOLVABLE_CONDITION) { + my $reverse = $rsrc->reverse_relationship_info($rel); + foreach my $rev_rel (keys %$reverse) { + if ($reverse->{$rev_rel}{attrs}{accessor} && $reverse->{$rev_rel}{attrs}{accessor} eq 'multi') { + weaken($attrs->{related_objects}{$rev_rel}[0] = $self); + } else { + weaken($attrs->{related_objects}{$rev_rel} = $self); + } } } - } - elsif (ref $cond eq 'ARRAY') { - $cond = [ map { - if (ref $_ eq 'HASH') { - my $hash; - foreach my $key (keys %$_) { - my $newkey = $key !~ /\./ ? "me.$key" : $key; - $hash->{$newkey} = $_->{$key}; + elsif (ref $cond eq 'ARRAY') { + $cond = [ map { + if (ref $_ eq 'HASH') { + my $hash; + foreach my $key (keys %$_) { + my $newkey = $key !~ /\./ ? "me.$key" : $key; + $hash->{$newkey} = $_->{$key}; + } + $hash; + } else { + $_; } - $hash; - } else { - $_; + } @$cond ]; + } + elsif (ref $cond eq 'HASH') { + foreach my $key (grep { ! /\./ } keys %$cond) { + $cond->{"me.$key"} = delete $cond->{$key}; } - } @$cond ]; - } - elsif (ref $cond eq 'HASH') { - foreach my $key (grep { ! /\./ } keys %$cond) { - $cond->{"me.$key"} = delete $cond->{$key}; } - } - $query = ($query ? { '-and' => [ $cond, $query ] } : $cond); - $rsrc->related_source($rel)->resultset->search( - $query, $attrs - ); + $query = ($query ? { '-and' => [ $cond, $query ] } : $cond); + $rsrc->related_source($rel)->resultset->search( + $query, $attrs + ); + } + }; + if ( + $rel_info->{attrs}{accessor} eq 'single' + and + exists $self->{_relationship_data}{$rel} + ) { + $rs->set_cache([ $self->{_relationship_data}{$rel} || () ]); + } + elsif ( + $rel_info->{attrs}{accessor} eq 'filter' + and + exists $self->{_inflated_column}{$rel} + ) { + $rs->set_cache([ $self->{_inflated_column}{$rel} || () ]); } + $rs; }; } diff --git a/lib/DBIx/Class/Row.pm b/lib/DBIx/Class/Row.pm index 2462b3b..80666b3 100644 --- a/lib/DBIx/Class/Row.pm +++ b/lib/DBIx/Class/Row.pm @@ -711,6 +711,26 @@ sub has_column_loaded { ) ? 1 : 0; } +sub _has_related_resultset_cached { + my ($self, $relname) = @_; + + my $accessor = ($self->relationship_info($relname) || {})->{attrs}{accessor} || ''; + + return (( + $accessor eq 'single' + and + exists $self->{_relationship_data}{$relname} + ) or ( + $accessor eq 'filter' + and + exists $self->{_inflated_column}{$relname} + ) or ( + defined $self->{related_resultsets}{$relname} + and + defined $self->{related_resultsets}{$relname}->get_cache + )) ? 1 : 0; +} + =head2 get_columns my %data = $result->get_columns; @@ -744,9 +764,7 @@ sub get_columns { ) if ( ! $ENV{DBIC_COLUMNS_INCLUDE_FILTER_RELS} and - defined $self->{related_resultsets}{$col} - and - defined $self->{related_resultsets}{$col}->get_cache + $self->_has_related_resultset_cached($col) ); $self->store_column($col, $self->_deflated_column($col, $self->{_inflated_column}{$col})); @@ -858,9 +876,7 @@ sub get_inflated_columns { if ( $loaded_colinfo->{$_}{_inflate_info} and - defined $self->{related_resultsets}{$_} - and - defined $self->{related_resultsets}{$_}->get_cache + $self->_has_related_resultset_cached($_) ) { carp_unique( "Returning prefetched 'filter' rels as part of get_inflated_columns() is deprecated and will " @@ -1247,8 +1263,9 @@ L, see L. sub inflate_result { my ($class, $rsrc, $me, $prefetch) = @_; + # XXX: WTF is $me sometimes undef? my $new = bless - { _column_data => $me, _result_source => $rsrc }, + { _column_data => $me || {}, _result_source => $rsrc }, ref $class || $class ; @@ -1273,7 +1290,7 @@ sub inflate_result { $class->throw_exception("No accessor type declared for prefetched relationship '$rel_name'") unless $relinfo->{attrs}{accessor}; - my $rel_rs = $new->related_resultset($rel_name); + my $rel_rsrc = $rsrc->related_source($rel_name); my @rel_objects; if ( @@ -1283,8 +1300,7 @@ sub inflate_result { ) { if (ref $prefetch->{$rel_name}[0] eq 'ARRAY') { - my $rel_rsrc = $rel_rs->result_source; - my $rel_class = $rel_rs->result_class; + my $rel_class = $rel_rsrc->result_class; my $rel_inflator = $rel_class->can('inflate_result'); @rel_objects = map { $rel_class->$rel_inflator ( $rel_rsrc, @$_ ) } @@ -1292,8 +1308,8 @@ sub inflate_result { ; } else { - @rel_objects = $rel_rs->result_class->inflate_result( - $rel_rs->result_source, @{$prefetch->{$rel_name}} + @rel_objects = $rel_rsrc->result_class->inflate_result( + $rel_rsrc, @{$prefetch->{$rel_name}} ); } } @@ -1304,8 +1320,9 @@ sub inflate_result { elsif ($relinfo->{attrs}{accessor} eq 'filter') { $new->{_inflated_column}{$rel_name} = $rel_objects[0]; } - - $rel_rs->set_cache(\@rel_objects); + else { + $new->related_resultset($rel_name)->set_cache(\@rel_objects); + } } }