From: Peter Rabbitson <ribasushi@cpan.org>
Date: Fri, 5 Feb 2010 16:00:12 +0000 (+0000)
Subject: Looks like RSC is finally (halfway) fixed
X-Git-Tag: v0.08116~3
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=472d7df3682bdb73a76af18c6c78e4596cb8aa73;p=dbsrgits%2FDBIx-Class.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');