From: Peter Rabbitson Date: Mon, 29 Jun 2009 07:46:13 +0000 (+0000) Subject: Support for -select/-as in SQLAHacks field selection X-Git-Tag: v0.08108~37^2~9 X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=83e09b5b5718416258f0db82f8547a7116b08301;p=dbsrgits%2FDBIx-Class.git Support for -select/-as in SQLAHacks field selection --- diff --git a/lib/DBIx/Class/SQLAHacks.pm b/lib/DBIx/Class/SQLAHacks.pm index a454cd5..7c8c725 100644 --- a/lib/DBIx/Class/SQLAHacks.pm +++ b/lib/DBIx/Class/SQLAHacks.pm @@ -240,28 +240,39 @@ sub _recurse_fields { ? ' AS col'.$self->{rownum_hack_count}++ : '') } @$fields); - } elsif ($ref eq 'HASH') { - foreach my $func (keys %$fields) { - if ($func eq 'distinct') { - my $_fields = $fields->{$func}; - if (ref $_fields eq 'ARRAY' && @{$_fields} > 1) { - croak ( - 'The select => { distinct => ... } syntax is not supported for multiple columns.' - .' Instead please use { group_by => [ qw/' . (join ' ', @$_fields) . '/ ] }' - .' or { select => [ qw/' . (join ' ', @$_fields) . '/ ], distinct => 1 }' - ); - } - else { - $_fields = @{$_fields}[0] if ref $_fields eq 'ARRAY'; - carp ( - 'The select => { distinct => ... } syntax will be deprecated in DBIC version 0.09,' - ." please use { group_by => '${_fields}' } or { select => '${_fields}', distinct => 1 }" - ); - } + } + 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 }' + ); } - return $self->_sqlcase($func) - .'( '.$self->_recurse_fields($fields->{$func}).' )'; + $select = sprintf ('%s( %s )', + $self->_sqlcase($func), + $self->_recurse_fields($args) + ); } + + # 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? elsif ( $ref eq 'REF' and ref($$fields) eq 'ARRAY' ) { @@ -279,9 +290,8 @@ sub _order_by { my $ret = ''; - if (defined $arg->{group_by}) { - $ret = $self->_sqlcase(' group by ') - .$self->_recurse_fields($arg->{group_by}, { no_rownum_hack => 1 }); + if (my $g = $self->_recurse_fields($arg->{group_by}, { no_rownum_hack => 1 }) ) { + $ret = $self->_sqlcase(' group by ') . $g; } if (defined $arg->{having}) { diff --git a/t/95sql_maker_quote.t b/t/95sql_maker_quote.t index a7687f8..6b27c7b 100644 --- a/t/95sql_maker_quote.t +++ b/t/95sql_maker_quote.t @@ -35,12 +35,23 @@ my ($sql, @bind) = $sql_maker->select( { 'artist.artistid' => 'me.artist' } - ] + ], + [ + { + 'tracks' => 'tracks', + '-join_type' => 'left' + }, + { + 'tracks.cd' => 'me.cdid' + } + ], ], [ - { - 'count' => '*' - } + 'me.cdid', + { count => 'tracks.cd' }, + { -select => 'me.artist' }, + { -select => 'me.title', -as => 'name' }, + { -select => { min => 'me.year' }, -as => 'me.minyear' }, ], { 'artist.name' => 'Caterwauler McCrae', @@ -53,8 +64,15 @@ my ($sql, @bind) = $sql_maker->select( 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] ], - 'got correct SQL and bind parameters for count query with quoting' + q/ + SELECT `me`.`cdid`, COUNT( `tracks`.`cd` ), `me`.`artist`, `me`.`title` AS `name`, 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` ) + WHERE ( `artist`.`name` = ? AND `me`.`year` = ? ) + /, + [ ['artist.name' => 'Caterwauler McCrae'], ['me.year' => 2001] ], + 'got correct SQL and bind parameters for complex select query with quoting' ); diff --git a/t/count/distinct.t b/t/count/distinct.t index 00ee411..6df9ed0 100644 --- a/t/count/distinct.t +++ b/t/count/distinct.t @@ -11,7 +11,7 @@ use DBIC::SqlMakerTest; my $schema = DBICTest->init_schema(); -plan tests => 58; +plan tests => 56; # The tag Blue is assigned to cds 1 2 3 and 5 # The tag Cheesy is assigned to cds 2 4 and 5 @@ -80,17 +80,6 @@ for my $get_count ( is($get_count->($rs), 3, 'Count by distinct function result as select literal'); } -eval { - my @warnings; - local $SIG{__WARN__} = sub { $_[0] =~ /The select => { distinct => ... } syntax will be deprecated/ - ? push @warnings, @_ - : warn @_ - }; - my $row = $schema->resultset('Tag')->search({}, { select => { distinct => 'tag' } })->first; - is (@warnings, 1, 'Warned about deprecated distinct') if $DBIx::Class::VERSION < 0.09; -}; -ok ($@, 'Exception on deprecated distinct usage thrown') if $DBIx::Class::VERSION >= 0.09; - throws_ok( sub { my $row = $schema->resultset('Tag')->search({}, { select => { distinct => [qw/tag cd/] } })->first }, qr/select => { distinct => \.\.\. } syntax is not supported for multiple columns/, @@ -99,4 +88,3 @@ throws_ok( # These two rely on the database to throw an exception. This might not be the case one day. Please revise. dies_ok(sub { my $count = $schema->resultset('Tag')->search({}, { '+select' => \'tagid AS tag_id', distinct => 1 })->count }, 'expecting to die'); -dies_ok(sub { my $count = $schema->resultset('Tag')->search({}, { select => { length => 'tag' }, distinct => 1 })->count }, 'expecting to die');