From: Peter Rabbitson Date: Fri, 5 Feb 2010 16:00:12 +0000 (+0000) Subject: Looks like RSC is finally (halfway) fixed X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=472d7df3682bdb73a76af18c6c78e4596cb8aa73;p=dbsrgits%2FDBIx-Class-Historic.git Looks like RSC is finally (halfway) fixed --- diff --git a/lib/DBIx/Class/ResultSetColumn.pm b/lib/DBIx/Class/ResultSetColumn.pm index 14b35f7..57fa227 100644 --- a/lib/DBIx/Class/ResultSetColumn.pm +++ b/lib/DBIx/Class/ResultSetColumn.pm @@ -45,9 +45,41 @@ sub new { $rs->throw_exception('column must be supplied') unless $column; my $orig_attrs = $rs->_resolved_attrs; - my $new_parent_rs = $rs->search_rs; + + # If $column can be found in the 'as' list of the parent resultset, use the + # corresponding element of its 'select' list (to keep any custom column + # definition set up with 'select' or '+select' attrs), otherwise use $column + # (to create a new column definition on-the-fly). + my $as_list = $orig_attrs->{as} || []; + my $select_list = $orig_attrs->{select} || []; + my $as_index = List::Util::first { ($as_list->[$_] || "") eq $column } 0..$#$as_list; + my $select = defined $as_index ? $select_list->[$as_index] : $column; + + my $new_parent_rs; + # analyze the order_by, and see if it is done over a function/nonexistentcolumn + # if this is the case we will need to wrap a subquery since the result of RSC + # *must* be a single column select + my %collist = map { $_ => 1 } ($rs->result_source->columns, $column); + if ( + scalar grep + { ! $collist{$_} } + ( $rs->result_source->schema->storage->_parse_order_by ($orig_attrs->{order_by} ) ) + ) { + my $alias = $rs->current_source_alias; + $new_parent_rs = $rs->result_source->resultset->search ( {}, { + alias => $alias, + from => [{ + $alias => $rs->as_query, + -alias => $alias, + -source_handle => $rs->result_source->handle, + }] + }); + } + + $new_parent_rs ||= $rs->search_rs; my $new_attrs = $new_parent_rs->{attrs} ||= {}; + # FIXME - this should go away when the chaining branch is merged # since what we do is actually chain to the original resultset, we need to throw # away all selectors (otherwise they'll chain) delete $new_attrs->{$_} for (qw/columns +columns select +select as +as cols include_columns/); @@ -58,15 +90,6 @@ sub new { # prefetch would otherwise generate. $new_attrs->{join} = $rs->_merge_attr( delete $new_attrs->{join}, delete $new_attrs->{prefetch} ); - # If $column can be found in the 'as' list of the parent resultset, use the - # corresponding element of its 'select' list (to keep any custom column - # definition set up with 'select' or '+select' attrs), otherwise use $column - # (to create a new column definition on-the-fly). - my $as_list = $orig_attrs->{as} || []; - my $select_list = $orig_attrs->{select} || []; - my $as_index = List::Util::first { ($as_list->[$_] || "") eq $column } 0..$#$as_list; - my $select = defined $as_index ? $select_list->[$as_index] : $column; - # {collapse} would mean a has_many join was injected, which in turn means # we need to group *IF WE CAN* (only if the column in question is unique) if (!$new_attrs->{group_by} && keys %{$orig_attrs->{collapse}}) { diff --git a/t/88result_set_column.t b/t/88result_set_column.t index f3d31d4..847483a 100644 --- a/t/88result_set_column.t +++ b/t/88result_set_column.t @@ -55,11 +55,11 @@ is_deeply ( # test +select/+as for single column my $psrs = $schema->resultset('CD')->search({}, { - '+select' => \'COUNT(*)', - '+as' => 'count' + '+select' => \'MAX(year)', + '+as' => 'last_year' } ); -lives_ok(sub { $psrs->get_column('count')->next }, '+select/+as additional column "count" present (scalar)'); +lives_ok(sub { $psrs->get_column('last_year')->next }, '+select/+as additional column "last_year" present (scalar)'); dies_ok(sub { $psrs->get_column('noSuchColumn')->next }, '+select/+as nonexistent column throws exception'); # test +select/+as for overriding a column @@ -75,11 +75,11 @@ is($psrs->get_column('title')->next, 'The Final Countdown', '+select/+as overrid # test +select/+as for multiple columns $psrs = $schema->resultset('CD')->search({}, { - '+select' => [ \'COUNT(*)', 'title' ], - '+as' => [ 'count', 'addedtitle' ] + '+select' => [ \'LENGTH(title) AS title_length', 'title' ], + '+as' => [ 'tlength', 'addedtitle' ] } ); -lives_ok(sub { $psrs->get_column('count')->next }, '+select/+as multiple additional columns, "count" column present'); +lives_ok(sub { $psrs->get_column('tlength')->next }, '+select/+as multiple additional columns, "tlength" column present'); lives_ok(sub { $psrs->get_column('addedtitle')->next }, '+select/+as multiple additional columns, "addedtitle" column present'); # test that +select/+as specs do not leak @@ -98,13 +98,28 @@ is_same_sql_bind ( ); is_same_sql_bind ( - $psrs->get_column('count')->as_query, - '(SELECT COUNT(*) FROM cd me)', + $psrs->get_column('tlength')->as_query, + '(SELECT LENGTH(title) AS title_length FROM cd me)', [], 'Correct SQL for get_column/+as func' ); - +# test that order_by over a function forces a subquery +lives_ok ( sub { + is_deeply ( + [ $psrs->search ({}, { order_by => { -desc => 'title_length' } })->get_column ('title')->all ], + [ + "Generic Manufactured Singles", + "Come Be Depressed With Us", + "Caterwaulin' Blues", + "Spoonful of bees", + "Forkful of bees", + ], + 'Subquery count induced by aliased ordering function', + ); +}); + +# test for prefetch not leaking { my $rs = $schema->resultset("CD")->search({}, { prefetch => 'artist' }); my $rsc = $rs->get_column('year');