X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=blobdiff_plain;f=lib%2FDBIx%2FClass%2FResultSet.pm;h=030f2924bd8bb8b07030b885daf71cb46eaba768;hb=dc7d89911b7bb98c30208cf73af522a99998dcd6;hp=6dbc7caec5207d13571c18d0b5d6e9f8f9c25866;hpb=e50536940adf2ebaef907a0c29ae37fbd5ce95b1;p=dbsrgits%2FDBIx-Class.git diff --git a/lib/DBIx/Class/ResultSet.pm b/lib/DBIx/Class/ResultSet.pm index 6dbc7ca..030f292 100644 --- a/lib/DBIx/Class/ResultSet.pm +++ b/lib/DBIx/Class/ResultSet.pm @@ -4,17 +4,18 @@ use strict; use warnings; use base 'DBIx::Class'; -use mro 'c3'; 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 dump_value - fail_on_internal_wantarray fail_on_internal_call UNRESOLVABLE_CONDITION + dbic_internal_try dbic_internal_catch dump_value emit_loud_diag + fail_on_internal_call UNRESOLVABLE_CONDITION DUMMY_ALIASPAIR ); -use Try::Tiny; +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 @@ -389,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 @@ -654,17 +654,15 @@ sub _stack_cond { (ref $_ eq 'HASH' and ! keys %$_) ) and $_ = undef for ($left, $right); - # either one of the two undef - if ( (defined $left) xor (defined $right) ) { - return defined $left ? $left : $right; - } - # both undef - elsif ( ! defined $left ) { - return undef - } - else { - return $self->result_source->schema->storage->_collapse_cond({ -and => [$left, $right] }); - } + return( + # either one of the two undef + ( (defined $left) xor (defined $right) ) ? ( defined $left ? $left : $right ) + + # both undef + : ( ! defined $left ) ? undef + + : { -and => [$left, $right] } + ); } =head2 search_literal @@ -699,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' ) { @@ -777,9 +777,8 @@ See also L and L. sub find { my $self = shift; - my $attrs = (@_ > 1 && ref $_[$#_] eq 'HASH' ? pop(@_) : {}); + my $attrs = (@_ > 1 && ref $_[-1] eq 'HASH' ? pop(@_) : {}); - my $rsrc = $self->result_source; my $constraint_name; if (exists $attrs->{key}) { @@ -792,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]} }; } @@ -814,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 + ! is_literal_value( $call_cond->{$key} ) and - # implicitly skip has_many's (likely MC) - (ref (my $val = delete $call_cond->{$key}) ne 'ARRAY' ) + # 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} }, + }; } } @@ -886,7 +921,7 @@ sub find { $alias ); } - catch { + dbic_internal_catch { push @fc_exceptions, $_ if $_ =~ /\bFilterColumn\b/; }; } @@ -899,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; @@ -987,7 +1022,7 @@ See also L. =cut -sub search_related { +sub search_related :DBIC_method_is_indirect_sugar { DBIx::Class::_ENV_::ASSERT_NO_INTERNAL_INDIRECT_CALLS and fail_on_internal_call; return shift->related_resultset(shift)->search(@_); } @@ -999,7 +1034,7 @@ it guarantees a resultset, even in list context. =cut -sub search_related_rs { +sub search_related_rs :DBIC_method_is_indirect_sugar { DBIx::Class::_ENV_::ASSERT_NO_INTERNAL_INDIRECT_CALLS and fail_on_internal_call; return shift->related_resultset(shift)->search_rs(@_); } @@ -1157,14 +1192,16 @@ 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.' .' Instead use ->search({ x => { -like => "y%" } })' .' (note the outer pair of {}s - they are important!)' ); - my $attrs = (@_ > 1 && ref $_[$#_] eq 'HASH' ? pop(@_) : {}); + my $attrs = (@_ > 1 && ref $_[-1] eq 'HASH' ? pop(@_) : {}); my $query = ref $_[0] eq 'HASH' ? { %{shift()} }: {@_}; $query->{$_} = { 'like' => $query->{$_} } for keys %$query; return $class->search($query, { %$attrs }); @@ -1188,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; @@ -1572,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 } }; @@ -1620,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 @@ -1747,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'); } @@ -1770,7 +1809,7 @@ with the passed arguments, then L. =cut -sub count_literal { +sub count_literal :DBIC_method_is_indirect_sugar { DBIx::Class::_ENV_::ASSERT_NO_INTERNAL_INDIRECT_CALLS and fail_on_internal_call; shift->search_literal(@_)->count } @@ -1850,7 +1889,7 @@ an object for the first result (or C if the resultset is empty). =cut -sub first { +sub first :DBIC_method_is_indirect_sugar { DBIx::Class::_ENV_::ASSERT_NO_INTERNAL_INDIRECT_CALLS and fail_on_internal_call; return $_[0]->reset->next; } @@ -1978,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; @@ -2223,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); @@ -2231,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 @@ -2241,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; @@ -2291,7 +2331,18 @@ sub populate { or ref $data->[$i][$_->{pos}] eq 'HASH' or - ( defined blessed $data->[$i][$_->{pos}] and $data->[$i][$_->{pos}]->isa('DBIx::Class::Row') ) + ( + defined blessed $data->[$i][$_->{pos}] + and + $data->[$i][$_->{pos}]->isa( + $DBIx::Class::ResultSource::__expected_result_class_isa + || + emit_loud_diag( + confess => 1, + msg => 'Global $DBIx::Class::ResultSource::__expected_result_class_isa unexpectedly unset...' + ) + ) + ) ) and 1 @@ -2299,7 +2350,18 @@ sub populate { # moar sanity check... sigh for ( ref $data->[$i][$_->{pos}] eq 'ARRAY' ? @{$data->[$i][$_->{pos}]} : $data->[$i][$_->{pos}] ) { - if ( defined blessed $_ and $_->isa('DBIx::Class::Row' ) ) { + if ( + defined blessed $_ + and + $_->isa( + $DBIx::Class::ResultSource::__expected_result_class_isa + || + emit_loud_diag( + confess => 1, + msg => 'Global $DBIx::Class::ResultSource::__expected_result_class_isa unexpectedly unset...' + ) + ) + ) { carp_unique("Fast-path populate() with supplied related objects is not possible - falling back to regular create()"); return my $throwaway = $self->populate(@_); } @@ -2341,7 +2403,18 @@ sub populate { or ref $data->[$i]{$_} eq 'HASH' or - ( defined blessed $data->[$i]{$_} and $data->[$i]{$_}->isa('DBIx::Class::Row') ) + ( + defined blessed $data->[$i]{$_} + and + $data->[$i]{$_}->isa( + $DBIx::Class::ResultSource::__expected_result_class_isa + || + emit_loud_diag( + confess => 1, + msg => 'Global $DBIx::Class::ResultSource::__expected_result_class_isa unexpectedly unset...' + ) + ) + ) ) and 1 @@ -2349,7 +2422,18 @@ sub populate { # moar sanity check... sigh for ( ref $data->[$i]{$_} eq 'ARRAY' ? @{$data->[$i]{$_}} : $data->[$i]{$_} ) { - if ( defined blessed $_ and $_->isa('DBIx::Class::Row' ) ) { + if ( + defined blessed $_ + and + $_->isa( + $DBIx::Class::ResultSource::__expected_result_class_isa + || + emit_loud_diag( + confess => 1, + msg => 'Global $DBIx::Class::ResultSource::__expected_result_class_isa unexpectedly unset...' + ) + ) + ) { carp_unique("Fast-path populate() with supplied related objects is not possible - falling back to regular create()"); return my $throwaway = $self->populate(@_); } @@ -2411,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 { @@ -2451,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 } @@ -2619,7 +2705,7 @@ sub _merge_with_rscond { @cols_from_relations = keys %{ $implied_data || {} }; } else { - my $eqs = $self->result_source->schema->storage->_extract_fixed_condition_columns($self->{cond}, 'consider_nulls'); + my $eqs = extract_equality_conditions( $self->{cond}, 'consider_nulls' ); $implied_data = { map { ( ($eqs->{$_}||'') eq UNRESOLVABLE_CONDITION ) ? () : ( $_ => $eqs->{$_} ) } keys %$eqs }; @@ -2779,7 +2865,7 @@ all in the call to C, even when set to C. sub find_or_new { my $self = shift; - my $attrs = (@_ > 1 && ref $_[$#_] eq 'HASH' ? pop(@_) : {}); + my $attrs = (@_ > 1 && ref $_[-1] eq 'HASH' ? pop(@_) : {}); my $hash = ref $_[0] eq 'HASH' ? shift : {@_}; if (keys %$hash and my $row = $self->find($hash, $attrs) ) { return $row; @@ -2868,7 +2954,7 @@ L. =cut -sub create { +sub create :DBIC_method_is_indirect_sugar { #my ($self, $col_data) = @_; DBIx::Class::_ENV_::ASSERT_NO_INTERNAL_INDIRECT_CALLS and fail_on_internal_call; return shift->new_result(shift)->insert; @@ -2948,7 +3034,7 @@ database! sub find_or_create { my $self = shift; - my $attrs = (@_ > 1 && ref $_[$#_] eq 'HASH' ? pop(@_) : {}); + my $attrs = (@_ > 1 && ref $_[-1] eq 'HASH' ? pop(@_) : {}); my $hash = ref $_[0] eq 'HASH' ? shift : {@_}; if (keys %$hash and my $row = $self->find($hash, $attrs) ) { return $row; @@ -3014,7 +3100,7 @@ database! sub update_or_create { my $self = shift; - my $attrs = (@_ > 1 && ref $_[$#_] eq 'HASH' ? pop(@_) : {}); + my $attrs = (@_ > 1 && ref $_[-1] eq 'HASH' ? pop(@_) : {}); my $cond = ref $_[0] eq 'HASH' ? shift : {@_}; my $row = $self->find($cond, $attrs); @@ -3077,7 +3163,7 @@ See also L, L and L. sub update_or_new { my $self = shift; - my $attrs = ( @_ > 1 && ref $_[$#_] eq 'HASH' ? pop(@_) : {} ); + my $attrs = ( @_ > 1 && ref $_[-1] eq 'HASH' ? pop(@_) : {} ); my $cond = ref $_[0] eq 'HASH' ? shift : {@_}; my $row = $self->find( $cond, $attrs ); @@ -3228,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} ); @@ -3242,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}; @@ -3377,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 @@ -3389,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, }); } @@ -3436,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}, @@ -3529,6 +3670,9 @@ sub _resolved_attrs { if ( $attrs->{rows} =~ /[^0-9]/ or $attrs->{rows} <= 0 ); } + # normalize where condition + $attrs->{where} = normalize_sqla_condition( $attrs->{where} ) + if $attrs->{where}; # default selection list $attrs->{columns} = [ $source->columns ]