Clarify usage of _resolved_attrs by adding the explicit _resolved_attrs_copy
Peter Rabbitson [Thu, 21 May 2009 07:45:52 +0000 (07:45 +0000)]
Clarify code in ResultSetColumn

lib/DBIx/Class/ResultSet.pm
lib/DBIx/Class/ResultSetColumn.pm

index 5a6b980..cf85311 100644 (file)
@@ -660,7 +660,7 @@ L<DBIx::Class::Cursor> for more information.
 sub cursor {
   my ($self) = @_;
 
-  my $attrs = { %{$self->_resolved_attrs} };
+  my $attrs = $self->_resolved_attrs_copy;
   return $self->{cursor}
     ||= $self->result_source->storage->select($attrs->{from}, $attrs->{select},
           $attrs->{where},$attrs);
@@ -711,7 +711,7 @@ sub single {
       $self->throw_exception('single() only takes search conditions, no attributes. You want ->search( $cond, $attrs )->single()');
   }
 
-  my $attrs = { %{$self->_resolved_attrs} };
+  my $attrs = $self->_resolved_attrs_copy;
   if ($where) {
     if (defined $attrs->{where}) {
       $attrs->{where} = {
@@ -1157,7 +1157,7 @@ sub count {
 sub _count_subq {
   my $self = shift;
 
-  my $attrs = { %{$self->_resolved_attrs} };
+  my $attrs = $self->_resolved_attrs_copy;
 
   # copy for the subquery, we need to do some adjustments to it too
   my $sub_attrs = { %$attrs };
@@ -1197,7 +1197,7 @@ sub _count_simple {
 sub __count {
   my ($self, $attrs) = @_;
 
-  $attrs ||= { %{$self->_resolved_attrs} };
+  $attrs ||= $self->_resolved_attrs_copy;
 
   # take off any column specs, any pagers, record_filter is cdbi, and no point of ordering a count
   delete $attrs->{$_} for (qw/columns +columns select +select as +as rows offset page pager order_by record_filter/); 
@@ -1338,7 +1338,7 @@ sub _rs_update_delete {
   if ($needs_group_by_subq or $needs_subq) {
 
     # make a new $rs selecting only the PKs (that's all we really need)
-    my $attrs = $self->_resolved_attrs;
+    my $attrs = $self->_resolved_attrs_copy;
 
     delete $attrs->{$_} for qw/prefetch collapse select +select as +as columns +columns/;
     $attrs->{columns} = [ map { "$attrs->{alias}.$_" } ($self->result_source->primary_columns) ];
@@ -2496,6 +2496,12 @@ sub _resolve_from {
   return ($from,$seen);
 }
 
+# too many times we have to do $attrs = { %{$self->_resolved_attrs} }
+sub _resolved_attrs_copy {
+  my $self = shift;
+  return { %{$self->_resolved_attrs (@_)} };
+}
+
 sub _resolved_attrs {
   my $self = shift;
   return $self->{_attrs} if $self->{_attrs};
index 2679803..3ed9342 100644 (file)
@@ -36,21 +36,32 @@ passed as params. Used internally by L<DBIx::Class::ResultSet/get_column>.
 sub new {
   my ($class, $rs, $column) = @_;
   $class = ref $class if ref $class;
+
+  $rs->throw_exception("column must be supplied") unless $column;
+
   my $new_parent_rs = $rs->search_rs; # we don't want to mess up the original, so clone it
-  my $attrs = $new_parent_rs->_resolved_attrs;
-  $new_parent_rs->{attrs}->{prefetch} = undef; # prefetch cause additional columns to be fetched
+
+  # prefetch causes additional columns to be fetched, but we can not just make a new
+  # rs via the _resolved_attrs trick - we need to retain the separation between
+  # +select/+as and select/as
+  for my $attr (qw/prefetch collapse/) {
+    for (qw/attrs _attrs/) {
+      delete $new_parent_rs->{$_}{$attr} if ref $new_parent_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 $attrs = $new_parent_rs->_resolved_attrs;
+
   my $as_list = $attrs->{as} || [];
   my $select_list = $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 = bless { _select => $select, _as => $column, _parent_resultset => $new_parent_rs }, $class;
-  $new->throw_exception("column must be supplied") unless $column;
   return $new;
 }