[Dbix-class] Bug in DBIx::Class::ResultSource::remove_columns ?

[Oleg Pronin]

The patch does not change the functionality of 'remove_columns', just
bugfix, so i suppose no additional tests are required, are they?

2009/1/16 Peter Rabbitson <rabbit+list at rabbit.us <rabbit%2Blist at rabbit.us>>

> Oleg Pronin wrote:
> > Maybe i misunderstood something but i suppose there is a bug in
> > remove_columns.
> >
> >   DB<14> x
> > $this->columns
> >
> > 0  'id'
> > ...
> > 16  'logo_small'
> > 17  'logo_mid'
> > 18  'logo_full'
> > 19  'screenshots'
> > 20  'data'
> > 21  'logo'
> >
> > $this->remove_column($c);     # $c =3D 'logo';
> >
> >   DB<15> x
> > $this->columns
> >
> > 0  'id'
> > ...
> > 16  'logo_mid'
> > 17  'logo_full'
> > 18  'screenshots'
> > 19  'data'
> >
> > It removed 'logo_small' as well.
> >
> > In DBIx::Class::ResultSource:
> >
> > sub remove_columns {
> >   my ($self, @cols) =3D @_;
> >
> >
> >   return unless $self->_ordered_columns;
> >
> >   my $columns =3D $self->_columns;
> >   my @remaining;
> >
> >   foreach my $col (@{$self->_ordered_columns}) {
> >     push @remaining, $col unless grep(/$col/, @cols);
> >
> >   }
> >
> >   foreach (@cols) {
> >     delete $columns->{$_};
> >   };
> >
> >   $self->_ordered_columns(\@remaining);
> > }
> >
> >
> > What is this:  "grep(/$col/, @cols);" and what for?
> >
> > Doc says
> > "$table->remove_columns(qw/col1 col2 col3/);
> > Removes columns from the result source."
> >
> > Not "Removes all columns that looks like" :-)
> >
> > Patch:
> >
> > sub remove_columns {
> >   my ($self, @to_remove) =3D @_;
> >
> >
> >   return unless my @ordered =3D $self->_ordered_columns;
> >
> >   my $columns =3D $self->_columns;
> >   delete $columns->{$_} for @to_remove;
> >
> >   my %to_remove =3D map {$_ =3D> 1} @to_remove;
> >   $self->_ordered_columns([grep {!$to_remove{$_}} @ordered]);
> >
> > }
> >
>
> Looks like a bug. Patch in the next email seems reasonable. However,
> Oleg, you have been around long enough to know the drill - no patches
> without tests.
>
> _______________________________________________
> List: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/dbix-class
> IRC: irc.perl.org#dbix-class
> SVN: http://dev.catalyst.perl.org/repos/bast/DBIx-Class/
> Searchable Archive:
> http://www.grokbase.com/group/dbix-class@lists.scsys.co.uk
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.scsys.co.uk/pipermail/dbix-class/attachments/20090116/af4=
d4f05/attachment.htm