From: Peter Rabbitson Date: Sun, 21 Aug 2016 08:07:17 +0000 (+0200) Subject: With time couple DBIHacks methods became single-callsite only X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?p=dbsrgits%2FDBIx-Class.git;a=commitdiff_plain;h=1e8d85b39753dff2cd42b1f7b6342e145105feca With time couple DBIHacks methods became single-callsite only Remove _inner_join_to_node and _resolve_ident_sources from the callchain entirely --- diff --git a/lib/DBIx/Class/ResultSet.pm b/lib/DBIx/Class/ResultSet.pm index bf7e88f..920c713 100644 --- a/lib/DBIx/Class/ResultSet.pm +++ b/lib/DBIx/Class/ResultSet.pm @@ -3283,8 +3283,55 @@ sub related_resultset { # since this is search_related, and we already slid the select window inwards # (the select/as attrs were deleted in the beginning), we need to flip all # left joins to inner, so we get the expected results - # read the comment on top of the actual function to see what this does - $attrs->{from} = $storage->_inner_join_to_node( $attrs->{from}, $attrs->{alias} ); + # + # The DBIC relationship chaining implementation is pretty simple - every + # new related_relationship is pushed onto the {from} stack, and the {select} + # window simply slides further in. This means that when we count somewhere + # in the middle, we got to make sure that everything in the join chain is an + # actual inner join, otherwise the count will come back with unpredictable + # results (a resultset may be generated with _some_ rows regardless of if + # the relation which the $rs currently selects has rows or not). E.g. + # $artist_rs->cds->count - normally generates: + # SELECT COUNT( * ) FROM artist me LEFT JOIN cd cds ON cds.artist = me.artistid + # which actually returns the number of artists * (number of cds || 1) + # + # So what we do here is crawl {from}, determine if the current alias is at + # the top of the stack, and if not - make sure the chain is inner-joined down + # to the root. + # + my $switch_branch = $storage->_find_join_path_to_node( + $attrs->{from}, + $attrs->{alias}, + ); + + if ( @{ $switch_branch || [] } ) { + + # So it looks like we will have to switch some stuff around. + # local() is useless here as we will be leaving the scope + # anyway, and deep cloning is just too fucking expensive + # So replace the first hashref in the node arrayref manually + my @new_from = $attrs->{from}[0]; + my $sw_idx = { map { (values %$_), 1 } @$switch_branch }; #there's one k/v per join-path + + for my $j ( @{$attrs->{from}}[ 1 .. $#{$attrs->{from}} ] ) { + my $jalias = $j->[0]{-alias}; + + if ($sw_idx->{$jalias}) { + my %attrs = %{$j->[0]}; + delete $attrs{-join_type}; + push @new_from, [ + \%attrs, + @{$j}[ 1 .. $#$j ], + ]; + } + else { + push @new_from, $j; + } + } + + $attrs->{from} = \@new_from; + } + #XXX - temp fix for result_class bug. There likely is a more elegant fix -groditi delete $attrs->{result_class}; diff --git a/lib/DBIx/Class/Storage/DBI.pm b/lib/DBIx/Class/Storage/DBI.pm index 99a895e..8d704bd 100644 --- a/lib/DBIx/Class/Storage/DBI.pm +++ b/lib/DBIx/Class/Storage/DBI.pm @@ -2642,8 +2642,6 @@ sub _select_args { $orig_attrs->{_last_sqlmaker_alias_map} = $attrs->{_aliastypes}; ### - # my $alias2source = $self->_resolve_ident_sources ($ident); - # # This would be the point to deflate anything found in $attrs->{where} # (and leave $attrs->{bind} intact). Problem is - inflators historically # expect a result object. And all we have is a resultsource (it is trivial diff --git a/lib/DBIx/Class/Storage/DBIHacks.pm b/lib/DBIx/Class/Storage/DBIHacks.pm index 317dbd8..e9cbdd6 100644 --- a/lib/DBIx/Class/Storage/DBIHacks.pm +++ b/lib/DBIx/Class/Storage/DBIHacks.pm @@ -769,36 +769,6 @@ sub _minmax_operator_for_datatype { $_[2] ? 'MAX' : 'MIN'; } -sub _resolve_ident_sources { - my ($self, $ident) = @_; - - my $alias2source = {}; - - # the reason this is so contrived is that $ident may be a {from} - # structure, specifying multiple tables to join - if ( blessed $ident && $ident->isa("DBIx::Class::ResultSource") ) { - # this is compat mode for insert/update/delete which do not deal with aliases - $alias2source->{me} = $ident; - } - elsif (ref $ident eq 'ARRAY') { - - for (@$ident) { - my $tabinfo; - if (ref $_ eq 'HASH') { - $tabinfo = $_; - } - if (ref $_ eq 'ARRAY' and ref $_->[0] eq 'HASH') { - $tabinfo = $_->[0]; - } - - $alias2source->{$tabinfo->{-alias}} = $tabinfo->{-rsrc} - if ($tabinfo->{-rsrc}); - } - } - - return $alias2source; -} - # Takes $ident, \@column_names # # returns { $column_name => \%column_info, ... } @@ -811,7 +781,34 @@ sub _resolve_column_info { return {} if $colnames and ! @$colnames; - my $sources = $self->_resolve_ident_sources($ident); + my $sources = ( + # this is compat mode for insert/update/delete which do not deal with aliases + ( + blessed($ident) + and + $ident->isa('DBIx::Class::ResultSource') + ) ? +{ me => $ident } + + # not a known fromspec - no columns to resolve: return directly + : ref($ident) ne 'ARRAY' ? return +{} + + : +{ + # otherwise decompose into alias/rsrc pairs + map + { + ( $_->{-rsrc} and $_->{-alias} ) + ? ( @{$_}{qw( -alias -rsrc )} ) + : () + } + map + { + ( ref $_ eq 'ARRAY' and ref $_->[0] eq 'HASH' ) ? $_->[0] + : ( ref $_ eq 'HASH' ) ? $_ + : () + } + @$ident + } + ); $_ = { rsrc => $_, colinfos => $_->columns_info } for values %$sources; @@ -873,54 +870,6 @@ sub _resolve_column_info { return \%return; } -# The DBIC relationship chaining implementation is pretty simple - every -# new related_relationship is pushed onto the {from} stack, and the {select} -# window simply slides further in. This means that when we count somewhere -# in the middle, we got to make sure that everything in the join chain is an -# actual inner join, otherwise the count will come back with unpredictable -# results (a resultset may be generated with _some_ rows regardless of if -# the relation which the $rs currently selects has rows or not). E.g. -# $artist_rs->cds->count - normally generates: -# SELECT COUNT( * ) FROM artist me LEFT JOIN cd cds ON cds.artist = me.artistid -# which actually returns the number of artists * (number of cds || 1) -# -# So what we do here is crawl {from}, determine if the current alias is at -# the top of the stack, and if not - make sure the chain is inner-joined down -# to the root. -# -sub _inner_join_to_node { - my ($self, $from, $alias) = @_; - - my $switch_branch = $self->_find_join_path_to_node($from, $alias); - - return $from unless @{$switch_branch||[]}; - - # So it looks like we will have to switch some stuff around. - # local() is useless here as we will be leaving the scope - # anyway, and deep cloning is just too fucking expensive - # So replace the first hashref in the node arrayref manually - my @new_from = ($from->[0]); - my $sw_idx = { map { (values %$_), 1 } @$switch_branch }; #there's one k/v per join-path - - for my $j (@{$from}[1 .. $#$from]) { - my $jalias = $j->[0]{-alias}; - - if ($sw_idx->{$jalias}) { - my %attrs = %{$j->[0]}; - delete $attrs{-join_type}; - push @new_from, [ - \%attrs, - @{$j}[ 1 .. $#$j ], - ]; - } - else { - push @new_from, $j; - } - } - - return \@new_from; -} - sub _find_join_path_to_node { my ($self, $from, $target_alias) = @_; @@ -1099,4 +1048,18 @@ sub _extract_fixed_condition_columns :DBIC_method_is_indirect_sugar { extract_equality_conditions(@_); } +sub _resolve_ident_sources :DBIC_method_is_indirect_sugar { + DBIx::Class::Exception->throw( + '_resolve_ident_sources() has been removed with no replacement, ' + . 'ask for advice on IRC if this affected you' + ); +} + +sub _inner_join_to_node :DBIC_method_is_indirect_sugar { + DBIx::Class::Exception->throw( + '_inner_join_to_node() has been removed with no replacement, ' + . 'ask for advice on IRC if this affected you' + ); +} + 1;