From: Peter Rabbitson Date: Mon, 17 Aug 2009 09:09:39 +0000 (+0000) Subject: Allow select AS specification for functions only via the -as hash-key (no pod yet) X-Git-Tag: v0.08109~8 X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=50136dd9129a2dfd3350e1a8b9ddd9af73664145;p=dbsrgits%2FDBIx-Class.git Allow select AS specification for functions only via the -as hash-key (no pod yet) --- diff --git a/lib/DBIx/Class/SQLAHacks.pm b/lib/DBIx/Class/SQLAHacks.pm index 61ff365..edd73fb 100644 --- a/lib/DBIx/Class/SQLAHacks.pm +++ b/lib/DBIx/Class/SQLAHacks.pm @@ -405,35 +405,33 @@ sub _recurse_fields { } elsif ($ref eq 'HASH') { my %hash = %$fields; - my ($select, $as); - if ($hash{-select}) { - $select = $self->_recurse_fields (delete $hash{-select}); - $as = $self->_quote (delete $hash{-as}); - } - else { - my ($func, $args) = each %hash; - delete $hash{$func}; - - if (lc ($func) eq 'distinct' && ref $args eq 'ARRAY' && @$args > 1) { - croak ( - 'The select => { distinct => ... } syntax is not supported for multiple columns.' - .' Instead please use { group_by => [ qw/' . (join ' ', @$args) . '/ ] }' - .' or { select => [ qw/' . (join ' ', @$args) . '/ ], distinct => 1 }' - ); - } - $select = sprintf ('%s( %s )', - $self->_sqlcase($func), - $self->_recurse_fields($args) + my $as = delete $hash{-as}; # if supplied + + my ($func, $args) = each %hash; + delete $hash{$func}; + + if (lc ($func) eq 'distinct' && ref $args eq 'ARRAY' && @$args > 1) { + croak ( + 'The select => { distinct => ... } syntax is not supported for multiple columns.' + .' Instead please use { group_by => [ qw/' . (join ' ', @$args) . '/ ] }' + .' or { select => [ qw/' . (join ' ', @$args) . '/ ], distinct => 1 }' ); } + my $select = sprintf ('%s( %s )%s', + $self->_sqlcase($func), + $self->_recurse_fields($args), + $as + ? join (' ', $self->_sqlcase('as'), $as) + : '' + ); + # there should be nothing left if (keys %hash) { croak "Malformed select argument - too many keys in hash: " . join (',', keys %$fields ); } - $select .= " AS $as" if $as; return $select; } # Is the second check absolutely necessary? diff --git a/lib/DBIx/Class/Storage/DBI.pm b/lib/DBIx/Class/Storage/DBI.pm index 333bf19..b8f3ebb 100644 --- a/lib/DBIx/Class/Storage/DBI.pm +++ b/lib/DBIx/Class/Storage/DBI.pm @@ -1601,9 +1601,9 @@ sub _adjust_select_args_for_complex_prefetch { # alias any functions to the dbic-side 'as' label # adjust the outer select accordingly - if (ref $sel eq 'HASH' && !$sel->{-select}) { - $sel = { -select => $sel, -as => $attrs->{as}[$i] }; - $select->[$i] = join ('.', $attrs->{alias}, ($attrs->{as}[$i] || "select_$i") ); + if (ref $sel eq 'HASH' ) { + $sel->{-as} ||= $attrs->{as}[$i]; + $select->[$i] = join ('.', $attrs->{alias}, ($sel->{-as} || "select_$i") ); } push @$sub_select, $sel; diff --git a/t/95sql_maker_quote.t b/t/95sql_maker_quote.t index 6b27c7b..e7fbb60 100644 --- a/t/95sql_maker_quote.t +++ b/t/95sql_maker_quote.t @@ -49,9 +49,7 @@ my ($sql, @bind) = $sql_maker->select( [ 'me.cdid', { count => 'tracks.cd' }, - { -select => 'me.artist' }, - { -select => 'me.title', -as => 'name' }, - { -select => { min => 'me.year' }, -as => 'me.minyear' }, + { min => 'me.year', -as => 'me.minyear' }, ], { 'artist.name' => 'Caterwauler McCrae', @@ -65,7 +63,7 @@ my ($sql, @bind) = $sql_maker->select( is_same_sql_bind( $sql, \@bind, q/ - SELECT `me`.`cdid`, COUNT( `tracks`.`cd` ), `me`.`artist`, `me`.`title` AS `name`, MIN( `me`.`year` ) AS `me`.`minyear` + SELECT `me`.`cdid`, COUNT( `tracks`.`cd` ), MIN( `me`.`year` ) AS `me`.`minyear` FROM `cd` `me` JOIN `artist` `artist` ON ( `artist`.`artistid` = `me`.`artist` ) LEFT JOIN `tracks` `tracks` ON ( `tracks`.`cd` = `me`.`cdid` ) @@ -294,7 +292,13 @@ $sql_maker->quote_char([qw/[ ]/]); ], [ { - 'count' => '*' + max => 'rank', + -as => 'max_rank', + }, + 'rank', + { + 'count' => '*', + -as => 'cnt', } ], { @@ -308,7 +312,7 @@ $sql_maker->quote_char([qw/[ ]/]); is_same_sql_bind( $sql, \@bind, - q/SELECT COUNT( * ) FROM [cd] [me] JOIN [artist] [artist] ON ( [artist].[artistid] = [me].[artist] ) WHERE ( [artist].[name] = ? AND [me].[year] = ? )/, [ ['artist.name' => 'Caterwauler McCrae'], ['me.year' => 2001] ], + q/SELECT MAX ( [rank] ) AS [max_rank], [rank], COUNT( * ) AS [cnt] FROM [cd] [me] JOIN [artist] [artist] ON ( [artist].[artistid] = [me].[artist] ) WHERE ( [artist].[name] = ? AND [me].[year] = ? )/, [ ['artist.name' => 'Caterwauler McCrae'], ['me.year' => 2001] ], 'got correct SQL and bind parameters for count query with bracket quoting' ); diff --git a/t/prefetch/grouped.t b/t/prefetch/grouped.t index 7ee4e7c..8e07612 100644 --- a/t/prefetch/grouped.t +++ b/t/prefetch/grouped.t @@ -133,10 +133,10 @@ for ($cd_rs->all) { }, { prefetch => [qw/tracks liner_notes/], - select => ['me.cdid', { count => 'tracks.trackid' } ], - as => [qw/cdid track_count/], + select => ['me.cdid', { count => 'tracks.trackid' }, { max => 'tracks.trackid', -as => 'maxtr'} ], + as => [qw/cdid track_count max_track_id/], group_by => 'me.cdid', - order_by => { -desc => 'track_count' }, + order_by => [ { -desc => 'track_count' }, { -asc => 'maxtr' } ], rows => 2, } ); @@ -162,22 +162,22 @@ for ($cd_rs->all) { is_same_sql_bind ( $most_tracks_rs->as_query, '( - SELECT me.cdid, me.track_count, + SELECT me.cdid, me.track_count, me.maxtr, tracks.trackid, tracks.cd, tracks.position, tracks.title, tracks.last_updated_on, tracks.last_updated_at, tracks.small_dt, liner_notes.liner_id, liner_notes.notes FROM ( - SELECT me.cdid, COUNT( tracks.trackid ) AS track_count + SELECT me.cdid, COUNT( tracks.trackid ) AS track_count, MAX( tracks.trackid ) AS maxtr, FROM cd me LEFT JOIN track tracks ON tracks.cd = me.cdid WHERE ( me.cdid IS NOT NULL ) GROUP BY me.cdid - ORDER BY track_count DESC + ORDER BY track_count DESC, maxtr ASC LIMIT 2 ) me LEFT JOIN track tracks ON tracks.cd = me.cdid LEFT JOIN liner_notes liner_notes ON liner_notes.liner_id = me.cdid WHERE ( me.cdid IS NOT NULL ) - ORDER BY track_count DESC, tracks.cd + ORDER BY track_count DESC, maxtr ASC, tracks.cd )', [], 'next() query generated expected SQL',