Make select always equal group by on count subqueries
[dbsrgits/DBIx-Class-Historic.git] / lib / DBIx / Class / ResultSet.pm
index 964d44a..42b100a 100644 (file)
@@ -307,7 +307,7 @@ sub search_rs {
   my $new_attrs = { %{$our_attrs}, %{$attrs} };
 
   # merge new attrs into inherited
-  foreach my $key (qw/join prefetch +select +as/) {
+  foreach my $key (qw/join prefetch +select +as bind/) {
     next unless exists $attrs->{$key};
     $new_attrs->{$key} = $self->_merge_attr($our_attrs->{$key}, $attrs->{$key});
   }
@@ -796,19 +796,16 @@ sub _collapse_query {
   if (ref $query eq 'ARRAY') {
     foreach my $subquery (@$query) {
       next unless ref $subquery;  # -or
-#      warn "ARRAY: " . Dumper $subquery;
       $collapsed = $self->_collapse_query($subquery, $collapsed);
     }
   }
   elsif (ref $query eq 'HASH') {
     if (keys %$query and (keys %$query)[0] eq '-and') {
       foreach my $subquery (@{$query->{-and}}) {
-#        warn "HASH: " . Dumper $subquery;
         $collapsed = $self->_collapse_query($subquery, $collapsed);
       }
     }
     else {
-#      warn "LEAF: " . Dumper $query;
       foreach my $col (keys %$query) {
         my $value = $query->{$col};
         $collapsed->{$col}{$value}++;
@@ -1121,6 +1118,11 @@ An accessor for the class to use when creating row objects. Defaults to
 C<< result_source->result_class >> - which in most cases is the name of the 
 L<"table"|DBIx::Class::Manual::Glossary/"ResultSource"> class.
 
+Note that changing the result_class will also remove any components
+that were originally loaded in the source class via
+L<DBIx::Class::ResultSource/load_components>. Any overloaded methods
+in the original source class will not run.
+
 =cut
 
 sub result_class {
@@ -1146,19 +1148,54 @@ Performs an SQL C<COUNT> with the same query as the resultset was built
 with to find the number of elements. If passed arguments, does a search
 on the resultset and counts the results of that.
 
-Note: When using C<count> with C<group_by>, L<DBIx::Class> emulates C<GROUP BY>
-using C<COUNT( DISTINCT( columns ) )>. Some databases (notably SQLite) do
-not support C<DISTINCT> with multiple columns. If you are using such a
-database, you should only use columns from the main table in your C<group_by>
-clause.
-
 =cut
 
+my @count_via_subq_attrs = qw/join seen_join prefetch group_by/;
 sub count {
   my $self = shift;
   return $self->search(@_)->count if @_ and defined $_[0];
   return scalar @{ $self->get_cache } if $self->get_cache;
-  my $count = $self->_count;
+
+  my @check_attrs = @count_via_subq_attrs;
+
+  # if we are not paged - we are simply asking for a limit
+  if (not $self->{attrs}{page} and not $self->{attrs}{software_limit}) {
+    push @check_attrs, qw/rows offset/;
+  }
+
+  return $self->_has_attr (@check_attrs)
+    ? $self->_count_subq
+    : $self->_count_simple
+}
+
+sub _count_subq {
+  my $self = shift;
+
+  my $attrs = { %{$self->_resolved_attrs} };
+
+  # copy for the subquery, we need to do some adjustments to it too
+  my $sub_attrs = { %$attrs };
+
+  # these can not go in the subquery either
+  delete $sub_attrs->{$_} for qw/prefetch select +select as +as columns +columns/;
+
+  # force a group_by and the same set of columns (most databases require this)
+  $sub_attrs->{columns} = $sub_attrs->{group_by} ||= [ map { "$attrs->{alias}.$_" } ($self->result_source->primary_columns) ];
+
+  $attrs->{from} = [{
+    count_subq => (ref $self)->new ($self->result_source, $sub_attrs )->as_query
+  }];
+
+  # the subquery replaces this
+  delete $attrs->{where};
+
+  return $self->__count ($attrs);
+}
+
+sub _count_simple {
+  my $self = shift;
+
+  my $count = $self->__count;
   return 0 unless $count;
 
   # need to take offset from resolved attrs
@@ -1170,37 +1207,21 @@ sub count {
   return $count;
 }
 
-sub _count { # Separated out so pager can get the full count
-  my $self = shift;
-  my $select = { count => '*' };
+sub __count {
+  my ($self, $attrs) = @_;
 
-  my $attrs = { %{$self->_resolved_attrs} };
-  if (my $group_by = delete $attrs->{group_by}) {
-    delete $attrs->{having};
-    my @distinct = (ref $group_by ?  @$group_by : ($group_by));
-    # todo: try CONCAT for multi-column pk
-    my @pk = $self->result_source->primary_columns;
-    if (@pk == 1) {
-      my $alias = $attrs->{alias};
-      foreach my $column (@distinct) {
-        if ($column =~ qr/^(?:\Q${alias}.\E)?$pk[0]$/) {
-          @distinct = ($column);
-          last;
-        }
-      }
-    }
+  $attrs ||= { %{$self->{attrs}} };
 
-    $select = { count => { distinct => \@distinct } };
-  }
+  # take off any subquery attrs (they'd be incorporated in the subquery),
+  # any column specs, any pagers, record_filter is cdbi, and no point of ordering a count
+  delete $attrs->{$_} for (@count_via_subq_attrs, qw/columns +columns select +select as +as rows offset page pager order_by record_filter/);
 
-  $attrs->{select} = $select;
+  $attrs->{select} = { count => '*' };
   $attrs->{as} = [qw/count/];
 
-  # offset, order by and page are not needed to count. record_filter is cdbi
-  delete $attrs->{$_} for qw/rows offset order_by page pager record_filter/;
-
   my $tmp_rs = (ref $self)->new($self->result_source, $attrs);
   my ($count) = $tmp_rs->cursor->next;
+
   return $count;
 }
 
@@ -1313,6 +1334,18 @@ sub first {
   return $_[0]->reset->next;
 }
 
+
+# _update_delete_via_subq
+#
+# Presence of some rs attributes requires a subquery to reliably 
+# update/deletre
+#
+
+sub _update_delete_via_subq {
+  return $_[0]->_has_attr (qw/join seen_join group_by row offset page/);
+}
+
+
 # _cond_for_update_delete
 #
 # update/delete require the condition to be modified to handle
@@ -1342,11 +1375,9 @@ sub _cond_for_update_delete {
   elsif (ref $full_cond eq 'HASH') {
     if ((keys %{$full_cond})[0] eq '-and') {
       $cond->{-and} = [];
-
       my @cond = @{$full_cond->{-and}};
-      for (my $i = 0; $i < @cond; $i++) {
+       for (my $i = 0; $i < @cond; $i++) {
         my $entry = $cond[$i];
-
         my $hash;
         if (ref $entry eq 'HASH') {
           $hash = $self->_cond_for_update_delete($entry);
@@ -1355,7 +1386,6 @@ sub _cond_for_update_delete {
           $entry =~ /([^.]+)$/;
           $hash->{$1} = $cond[++$i];
         }
-
         push @{$cond->{-and}}, $hash;
       }
     }
@@ -1367,9 +1397,7 @@ sub _cond_for_update_delete {
     }
   }
   else {
-    $self->throw_exception(
-      "Can't update/delete on resultset with condition unless hash or array"
-    );
+    $self->throw_exception("Can't update/delete on resultset with condition unless hash or array");
   }
 
   return $cond;
@@ -1394,16 +1422,16 @@ if no records were updated; exact type of success value is storage-dependent.
 
 sub update {
   my ($self, $values) = @_;
-  $self->throw_exception("Values for update must be a hash")
+  $self->throw_exception('Values for update must be a hash')
     unless ref $values eq 'HASH';
 
-  carp(   'WARNING! Currently $rs->update() does not generate proper SQL'
-        . ' on joined resultsets, and may affect rows well outside of the'
-        . ' contents of $rs. Use at your own risk' )
-    if ( $self->{attrs}{seen_join} );
+  # rs operations with subqueries are Storage dependent - delegate
+  if ($self->_update_delete_via_subq) {
+    return $self->result_source->storage->subq_update_delete($self, 'update', $values);
+  }
 
   my $cond = $self->_cond_for_update_delete;
-   
+
   return $self->result_source->storage->update(
     $self->result_source, $values, $cond
   );
@@ -1426,7 +1454,7 @@ will run DBIC cascade triggers, while L</update> will not.
 
 sub update_all {
   my ($self, $values) = @_;
-  $self->throw_exception("Values for update must be a hash")
+  $self->throw_exception('Values for update_all must be a hash')
     unless ref $values eq 'HASH';
   foreach my $obj ($self->all) {
     $obj->set_columns($values)->update;
@@ -1451,23 +1479,21 @@ to run. See also L<DBIx::Class::Row/delete>.
 delete may not generate correct SQL for a query with joins or a resultset
 chained from a related resultset.  In this case it will generate a warning:-
 
-  WARNING! Currently $rs->delete() does not generate proper SQL on
-  joined resultsets, and may delete rows well outside of the contents
-  of $rs. Use at your own risk
-
 In these cases you may find that delete_all is more appropriate, or you
 need to respecify your query in a way that can be expressed without a join.
 
 =cut
 
 sub delete {
-  my ($self) = @_;
-  $self->throw_exception("Delete should not be passed any arguments")
-    if $_[1];
-  carp(   'WARNING! Currently $rs->delete() does not generate proper SQL'
-        . ' on joined resultsets, and may delete rows well outside of the'
-        . ' contents of $rs. Use at your own risk' )
-    if ( $self->{attrs}{seen_join} );
+  my $self = shift;
+  $self->throw_exception('delete does not accept any arguments')
+    if @_;
+
+  # rs operations with subqueries are Storage dependent - delegate
+  if ($self->_update_delete_via_subq) {
+    return $self->result_source->storage->subq_update_delete($self, 'delete');
+  }
+
   my $cond = $self->_cond_for_update_delete;
 
   $self->result_source->storage->delete($self->result_source, $cond);
@@ -1490,7 +1516,10 @@ will run DBIC cascade triggers, while L</delete> will not.
 =cut
 
 sub delete_all {
-  my ($self) = @_;
+  my $self = shift;
+  $self->throw_exception('delete_all does not accept any arguments')
+    if @_;
+
   $_->delete for $self->all;
   return 1;
 }
@@ -1687,12 +1716,25 @@ C<total_entries> on the L<Data::Page> object.
 
 sub pager {
   my ($self) = @_;
+
+  return $self->{pager} if $self->{pager};
+
   my $attrs = $self->{attrs};
   $self->throw_exception("Can't create pager for non-paged rs")
     unless $self->{attrs}{page};
   $attrs->{rows} ||= 10;
-  return $self->{pager} ||= Data::Page->new(
-    $self->_count, $attrs->{rows}, $self->{attrs}{page});
+
+  # throw away the paging flags and re-run the count (possibly 
+  # with a subselect) to get the real total count
+  my $count_attrs = { %$attrs };
+  delete $count_attrs->{$_} for qw/rows offset page pager/;
+  my $total_count = (ref $self)->new($self->result_source, $count_attrs)->count;
+
+  return $self->{pager} = Data::Page->new(
+    $total_count,
+    $attrs->{rows},
+    $self->{attrs}{page}
+  );
 }
 
 =head2 page
@@ -1796,6 +1838,37 @@ sub _is_deterministic_value {
   return 0;
 }
 
+# _has_attr
+#
+# determines if the resultset defines at least one
+# of the attributes supplied
+#
+# used to determine if a subquery is neccessary
+
+sub _has_attr {
+  my ($self, @attr_names) = @_;
+
+  my $attrs = $self->_resolved_attrs;
+
+  my $join_check_req;
+
+  for my $n (@attr_names) {
+    return 1 if defined $attrs->{$n};
+    ++$join_check_req if $n =~ /join/;
+  }
+
+  # a join can be expressed as a multi-level from
+  return 1 if (
+    $join_check_req
+      and
+    ref $attrs->{from} eq 'ARRAY'
+      and
+    @{$attrs->{from}} > 1 
+  );
+
+  return 0;
+}
+
 # _collapse_cond
 #
 # Recursively collapse the condition.
@@ -1808,19 +1881,16 @@ sub _collapse_cond {
   if (ref $cond eq 'ARRAY') {
     foreach my $subcond (@$cond) {
       next unless ref $subcond;  # -or
-#      warn "ARRAY: " . Dumper $subcond;
       $collapsed = $self->_collapse_cond($subcond, $collapsed);
     }
   }
   elsif (ref $cond eq 'HASH') {
     if (keys %$cond and (keys %$cond)[0] eq '-and') {
       foreach my $subcond (@{$cond->{-and}}) {
-#        warn "HASH: " . Dumper $subcond;
         $collapsed = $self->_collapse_cond($subcond, $collapsed);
       }
     }
     else {
-#      warn "LEAF: " . Dumper $cond;
       foreach my $col (keys %$cond) {
         my $value = $cond->{$col};
         $collapsed->{$col} = $value;
@@ -2266,7 +2336,7 @@ sub related_resultset {
       "search_related: result source '" . $self->result_source->source_name .
         "' has no such relationship $rel")
       unless $rel_obj;
-    
+
     my ($from,$seen) = $self->_resolve_from($rel);
 
     my $join_count = $seen->{$rel};
@@ -2359,28 +2429,33 @@ sub current_source_alias {
   return ($self->{attrs} || {})->{alias} || 'me';
 }
 
+# This code is called by search_related, and makes sure there
+# is clear separation between the joins before, during, and
+# after the relationship. This information is needed later
+# in order to properly resolve prefetch aliases (any alias
+# with a relation_chain_depth less than the depth of the
+# current prefetch is not considered)
 sub _resolve_from {
   my ($self, $extra_join) = @_;
   my $source = $self->result_source;
   my $attrs = $self->{attrs};
-  
+
   my $from = $attrs->{from}
     || [ { $attrs->{alias} => $source->from } ];
     
   my $seen = { %{$attrs->{seen_join}||{}} };
 
-  my $join = ($attrs->{join}
-               ? [ $attrs->{join}, $extra_join ]
-               : $extra_join);
-
   # we need to take the prefetch the attrs into account before we 
   # ->resolve_join as otherwise they get lost - captainL
-  my $merged = $self->_merge_attr( $join, $attrs->{prefetch} );
+  my $merged = $self->_merge_attr( $attrs->{join}, $attrs->{prefetch} );
+
+  push @$from, $source->resolve_join($merged, $attrs->{alias}, $seen) if ($merged);
 
-  $from = [
-    @$from,
-    ($join ? $source->resolve_join($merged, $attrs->{alias}, $seen) : ()),
-  ];
+  ++$seen->{-relation_chain_depth};
+
+  push @$from, $source->resolve_join($extra_join, $attrs->{alias}, $seen);
+
+  ++$seen->{-relation_chain_depth};
 
   return ($from,$seen);
 }
@@ -2399,12 +2474,20 @@ sub _resolved_attrs {
   # build columns (as long as select isn't set) into a set of as/select hashes
   unless ( $attrs->{select} ) {
       @colbits = map {
-          ( ref($_) eq 'HASH' ) ? $_
-            : {
-              (
-                  /^\Q${alias}.\E(.+)$/ ? $1
-                  : $_
-                ) => ( /\./ ? $_ : "${alias}.$_" )
+          ( ref($_) eq 'HASH' )
+              ? $_
+              : {
+                  (
+                    /^\Q${alias}.\E(.+)$/ 
+                      ? "$1"
+                      : "$_"
+                  )
+                => 
+                  (
+                    /\./ 
+                      ? "$_" 
+                      : "${alias}.$_"
+                  )
             }
       } ( ref($attrs->{columns}) eq 'ARRAY' ) ? @{ delete $attrs->{columns}} : (delete $attrs->{columns} || $source->columns );
   }
@@ -2494,12 +2577,12 @@ sub _resolved_attrs {
   if ( my $prefetch = delete $attrs->{prefetch} ) {
     $prefetch = $self->_merge_attr( {}, $prefetch );
     my @pre_order;
-    my $seen = { %{ $attrs->{seen_join} || {} } };
     foreach my $p ( ref $prefetch eq 'ARRAY' ? @$prefetch : ($prefetch) ) {
 
       # bring joins back to level of current class
+      my $join_map = $self->_joinpath_aliases ($attrs->{from}, $attrs->{seen_join});
       my @prefetch =
-        $source->resolve_prefetch( $p, $alias, $seen, \@pre_order, $collapse );
+        $source->resolve_prefetch( $p, $alias, $join_map, \@pre_order, $collapse );
       push( @{ $attrs->{select} }, map { $_->[0] } @prefetch );
       push( @{ $attrs->{as} },     map { $_->[1] } @prefetch );
     }
@@ -2507,14 +2590,32 @@ sub _resolved_attrs {
   }
   $attrs->{collapse} = $collapse;
 
-  if ( $attrs->{page} ) {
-    $attrs->{offset} ||= 0;
-    $attrs->{offset} += ( $attrs->{rows} * ( $attrs->{page} - 1 ) );
+  if ( $attrs->{page} and not defined $attrs->{offset} ) {
+    $attrs->{offset} = ( $attrs->{rows} * ( $attrs->{page} - 1 ) );
   }
 
   return $self->{_attrs} = $attrs;
 }
 
+sub _joinpath_aliases {
+  my ($self, $fromspec, $seen) = @_;
+
+  my $paths = {};
+  return $paths unless ref $fromspec eq 'ARRAY';
+
+  for my $j (@$fromspec) {
+
+    next if ref $j ne 'ARRAY';
+    next if $j->[0]{-relation_chain_depth} < ( $seen->{-relation_chain_depth} || 0);
+
+    my $p = $paths;
+    $p = $p->{$_} ||= {} for @{$j->[0]{-join_path}};
+    push @{$p->{-join_aliases} }, $j->[0]{-join_alias};
+  }
+
+  return $paths;
+}
+
 sub _rollout_attr {
   my ($self, $attr) = @_;