X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=blobdiff_plain;f=lib%2FDBIx%2FClass%2FResultSet.pm;h=030f2924bd8bb8b07030b885daf71cb46eaba768;hb=d8cf3aa31fb3d6ff7813f021fcc002663725fc41;hp=bf7e88f03a02567474ca7a889646d6b8bee4a622;hpb=c6ec79000b160e7491d9ab9d95d6e69c473b0862;p=dbsrgits%2FDBIx-Class.git diff --git a/lib/DBIx/Class/ResultSet.pm b/lib/DBIx/Class/ResultSet.pm index bf7e88f..030f292 100644 --- a/lib/DBIx/Class/ResultSet.pm +++ b/lib/DBIx/Class/ResultSet.pm @@ -9,11 +9,13 @@ use DBIx::Class::Carp; use DBIx::Class::ResultSetColumn; use DBIx::Class::ResultClass::HashRefInflator; use Scalar::Util qw( blessed reftype ); +use SQL::Abstract 'is_literal_value'; use DBIx::Class::_Util qw( dbic_internal_try dbic_internal_catch dump_value emit_loud_diag - fail_on_internal_wantarray fail_on_internal_call UNRESOLVABLE_CONDITION + fail_on_internal_call UNRESOLVABLE_CONDITION DUMMY_ALIASPAIR ); use DBIx::Class::SQLMaker::Util qw( normalize_sqla_condition extract_equality_conditions ); +use DBIx::Class::ResultSource::FromSpec::Util 'find_join_path_to_alias'; BEGIN { # De-duplication in _merge_attr() is disabled, but left in for reference @@ -388,28 +390,27 @@ L. =cut -sub search { - my $self = shift; - my $rs = $self->search_rs( @_ ); +sub search :DBIC_method_is_indirect_sugar { + DBIx::Class::_ENV_::ASSERT_NO_INTERNAL_INDIRECT_CALLS and fail_on_internal_call; - if (wantarray) { - DBIx::Class::_ENV_::ASSERT_NO_INTERNAL_WANTARRAY and my $sog = fail_on_internal_wantarray; - return $rs->all; - } - elsif (defined wantarray) { - return $rs; - } - else { - # we can be called by a relationship helper, which in - # turn may be called in void context due to some braindead - # overload or whatever else the user decided to be clever - # at this particular day. Thus limit the exception to - # external code calls only - $self->throw_exception ('->search is *not* a mutator, calling it in void context makes no sense') - if (caller)[0] !~ /^\QDBIx::Class::/; - - return (); - } + my $rs = shift->search_rs( @_ ); + + return $rs->all + if wantarray; + + return $rs + if defined wantarray; + + # we can be called by a relationship helper, which in + # turn may be called in void context due to some braindead + # overload or whatever else the user decided to be clever + # at this particular day. Thus limit the exception to + # external code calls only + $rs->throw_exception ('->search is *not* a mutator, calling it in void context makes no sense') + if (caller)[0] !~ /^\QDBIx::Class::/; + + # we are in void ctx here, but just in case + return (); } =head2 search_rs @@ -696,7 +697,9 @@ Example of how to use C instead of C =cut -sub search_literal { +sub search_literal :DBIC_method_is_indirect_sugar { + DBIx::Class::_ENV_::ASSERT_NO_INTERNAL_INDIRECT_CALLS and fail_on_internal_call; + my ($self, $sql, @bind) = @_; my $attr; if ( @bind && ref($bind[-1]) eq 'HASH' ) { @@ -776,7 +779,6 @@ sub find { my $self = shift; my $attrs = (@_ > 1 && ref $_[-1] eq 'HASH' ? pop(@_) : {}); - my $rsrc = $self->result_source; my $constraint_name; if (exists $attrs->{key}) { @@ -789,6 +791,8 @@ sub find { # Parse out the condition from input my $call_cond; + my $rsrc = $self->result_source; + if (ref $_[0] eq 'HASH') { $call_cond = { %{$_[0]} }; } @@ -811,25 +815,59 @@ sub find { } # process relationship data if any + my $rel_list; + for my $key (keys %$call_cond) { if ( + # either a structure or a result-ish object length ref($call_cond->{$key}) and - my $relinfo = $rsrc->relationship_info($key) + ( $rel_list ||= { map { $_ => 1 } $rsrc->relationships } ) + ->{$key} and - # implicitly skip has_many's (likely MC) - ( ref( my $val = delete $call_cond->{$key} ) ne 'ARRAY' ) + ! is_literal_value( $call_cond->{$key} ) + and + # implicitly skip has_many's (likely MC), via the delete() + ( ref( my $foreign_val = delete $call_cond->{$key} ) ne 'ARRAY' ) ) { - my ($rel_cond, $crosstable) = $rsrc->_resolve_condition( - $relinfo->{cond}, $val, $key, $key - ); - $self->throw_exception("Complex condition via relationship '$key' is unsupported in find()") - if $crosstable or ref($rel_cond) ne 'HASH'; + # FIXME: it seems wrong that relationship conditions take precedence...? + $call_cond = { + %$call_cond, + + %{ $rsrc->resolve_relationship_condition( + require_join_free_values => 1, + rel_name => $key, + foreign_values => ( + (! defined blessed $foreign_val) ? $foreign_val : do { + + my $f_result_class = $rsrc->related_source($key)->result_class; + + unless( $foreign_val->isa($f_result_class) ) { + + $self->throw_exception( + 'Objects supplied to find() must inherit from ' + . "'$DBIx::Class::ResultSource::__expected_result_class_isa'" + ) unless $foreign_val->isa( + $DBIx::Class::ResultSource::__expected_result_class_isa + ); + + carp_unique( + "Objects supplied to find() via '$key' usually should inherit from " + . "the related ResultClass ('$f_result_class'), perhaps you've made " + . 'a mistake?' + ); + } + + +{ $foreign_val->get_columns }; + } + ), - # supplement condition - # relationship conditions take precedence (?) - @{$call_cond}{keys %$rel_cond} = values %$rel_cond; + # an API where these are optional would be too cumbersome, + # instead always pass in some dummy values + DUMMY_ALIASPAIR, + )->{join_free_values} }, + }; } } @@ -896,7 +934,7 @@ sub find { } # Run the query, passing the result_class since it should propagate for find - my $rs = $self->search ($final_cond, {result_class => $self->result_class, %$attrs}); + my $rs = $self->search_rs( $final_cond, {result_class => $self->result_class, %$attrs} ); if ($rs->_resolved_attrs->{collapse}) { my $row = $rs->next; carp "Query returned more than one row" if $rs->next; @@ -1154,7 +1192,9 @@ instead. An example conversion is: =cut -sub search_like { +sub search_like :DBIC_method_is_indirect_sugar { + DBIx::Class::_ENV_::ASSERT_NO_INTERNAL_INDIRECT_CALLS and fail_on_internal_call; + my $class = shift; carp_unique ( 'search_like() is deprecated and will be removed in DBIC version 0.09.' @@ -1185,7 +1225,9 @@ three records, call: =cut -sub slice { +sub slice :DBIC_method_is_indirect_sugar { + DBIx::Class::_ENV_::ASSERT_NO_INTERNAL_INDIRECT_CALLS and fail_on_internal_call; + my ($self, $min, $max) = @_; my $attrs = {}; # = { %{ $self->{attrs} || {} } }; $attrs->{offset} = $self->{attrs}{offset} || 0; @@ -1569,7 +1611,7 @@ C<< $rs->search ($cond, \%attrs)->count >> sub count { my $self = shift; - return $self->search(@_)->count if @_ and defined $_[0]; + return $self->search_rs(@_)->count if @_ and defined $_[0]; return scalar @{ $self->get_cache } if $self->get_cache; my $attrs = { %{ $self->_resolved_attrs } }; @@ -1617,7 +1659,7 @@ the same single value obtainable via L. sub count_rs { my $self = shift; - return $self->search(@_)->count_rs if @_; + return $self->search_rs(@_)->count_rs if @_; # this may look like a lack of abstraction (count() does about the same) # but in fact an _rs *must* use a subquery for the limits, as the @@ -1744,7 +1786,7 @@ sub _count_subq_rs { return $rsrc->resultset_class ->new ($rsrc, $sub_attrs) ->as_subselect_rs - ->search ({}, { columns => { count => $rsrc->schema->storage->_count_select ($rsrc, $attrs) } }) + ->search_rs ({}, { columns => { count => $rsrc->schema->storage->_count_select ($rsrc, $attrs) } }) ->get_column ('count'); } @@ -1975,7 +2017,7 @@ sub _rs_update_delete { } } - $subrs = $subrs->search({}, { group_by => $attrs->{columns} }); + $subrs = $subrs->search_rs({}, { group_by => $attrs->{columns} }); } $guard = $storage->txn_scope_guard; @@ -2220,6 +2262,7 @@ sub populate { # At this point assume either hashes or arrays my $rsrc = $self->result_source; + my $storage = $rsrc->schema->storage; if(defined wantarray) { my (@results, $guard); @@ -2228,7 +2271,7 @@ sub populate { # column names only, nothing to do return if @$data == 1; - $guard = $rsrc->schema->storage->txn_scope_guard + $guard = $storage->txn_scope_guard if @$data > 2; @results = map @@ -2238,7 +2281,7 @@ sub populate { } else { - $guard = $rsrc->schema->storage->txn_scope_guard + $guard = $storage->txn_scope_guard if @$data > 1; @results = map { $self->new_result($_)->insert } @$data; @@ -2452,13 +2495,13 @@ sub populate { ### start work my $guard; - $guard = $rsrc->schema->storage->txn_scope_guard + $guard = $storage->txn_scope_guard if $slices_with_rels; ### main source data # FIXME - need to switch entirely to a coderef-based thing, # so that large sets aren't copied several times... I think - $rsrc->schema->storage->_insert_bulk( + $storage->_insert_bulk( $rsrc, [ @$colnames, sort keys %$rs_data ], [ map { @@ -2492,18 +2535,20 @@ sub populate { $colinfo->{$rel}{rs} = $rsrc->related_source($rel)->resultset; - $colinfo->{$rel}{fk_map} = { reverse %{ $rsrc->_resolve_relationship_condition( + $colinfo->{$rel}{fk_map} = { reverse %{ $rsrc->resolve_relationship_condition( rel_name => $rel, - self_alias => "\xFE", # irrelevant - foreign_alias => "\xFF", # irrelevant + + # an API where these are optional would be too cumbersome, + # instead always pass in some dummy values + DUMMY_ALIASPAIR, )->{identity_map} || {} } }; } - $colinfo->{$rel}{rs}->search({ map # only so that we inherit them values properly, no actual search + $colinfo->{$rel}{rs}->search_rs({ map # only so that we inherit them values properly, no actual search { $_ => { '=' => - ( $main_proto_rs ||= $rsrc->resultset->search($main_proto) ) + ( $main_proto_rs ||= $rsrc->resultset->search_rs($main_proto) ) ->get_column( $colinfo->{$rel}{fk_map}{$_} ) ->as_query } @@ -3269,13 +3314,11 @@ sub related_resultset { my $attrs = $self->_chain_relationship($rel); - my $storage = $rsrc->schema->storage; - # Previously this atribute was deleted (instead of being set as it is now) # Doing so seems to be harmless in all available test permutations # See also 01d59a6a6 and mst's comment below # - $attrs->{alias} = $storage->relname_to_table_alias( + $attrs->{alias} = $rsrc->schema->storage->relname_to_table_alias( $rel, $attrs->{seen_join}{$rel} ); @@ -3283,8 +3326,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 = find_join_path_to_alias( + $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}; @@ -3418,9 +3508,19 @@ but because we isolated the group by into a subselect the above works. =cut sub as_subselect_rs { + + # FIXME - remove at some point in the future (2018-ish) + wantarray + and + carp_unique( + 'Starting with DBIC@0.082900 as_subselect_rs() always returns a ResultSet ' + . 'instance regardless of calling context. Please force scalar() context to ' + . 'silence this warning' + ); + my $self = shift; - my $attrs = $self->_resolved_attrs; + my $alias = $self->current_source_alias; my $fresh_rs = (ref $self)->new ( $self->result_source @@ -3430,13 +3530,13 @@ sub as_subselect_rs { delete $fresh_rs->{cond}; delete @{$fresh_rs->{attrs}}{qw/where bind/}; - return $fresh_rs->search( {}, { + $fresh_rs->search_rs( {}, { from => [{ - $attrs->{alias} => $self->as_query, - -alias => $attrs->{alias}, + $alias => $self->as_query, + -alias => $alias, -rsrc => $self->result_source, }], - alias => $attrs->{alias}, + alias => $alias, }); } @@ -3477,7 +3577,7 @@ sub _chain_relationship { # Nuke the prefetch (if any) before the new $rs attrs # are resolved (prefetch is useless - we are wrapping # a subquery anyway). - my $rs_copy = $self->search; + my $rs_copy = $self->search_rs; $rs_copy->{attrs}{join} = $self->_merge_joinpref_attr ( $rs_copy->{attrs}{join}, delete $rs_copy->{attrs}{prefetch},