From: Peter Rabbitson <ribasushi@cpan.org>
Date: Mon, 29 Jun 2009 07:46:13 +0000 (+0000)
Subject: Support for -select/-as in SQLAHacks field selection
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=83e09b5b5718416258f0db82f8547a7116b08301;p=dbsrgits%2FDBIx-Class-Historic.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');