[Dbix-class] RFC: What to do with $resultset->single returning multiple rows?

[Matt S Trout]

On Mon, May 05, 2008 at 05:19:01PM -0700, John Napiorkowski wrote:
> Hi,
> 
> While trying to track down a bug in .08 trunk that was causing an error on mysql I discovered that the problem was due to a change in the way DBIx::Class::Storage::DBI->select_single handled results that returned more than a single row.  In 0.08010 and earlier, if ->select_single was run against SQL that returned more than a single row, only the first row was returned and the remaining ones silently discarded.  On DBIC trunk, a bit of code was added that would carp out if more rows existed:
> 
> sub select_single {
>   my $self = shift;
>   my ($rv, $sth, @bind) = $self->_select(@_);
>   my @row = $sth->fetchrow_array;
>   carp "Query returned more than one row" if $sth->fetchrow_array;
>   # Need to call finish() to work round broken DBDs
>   $sth->finish();
>   return @row;
> }
> 
> 
> Now, from what I can see, the entire point of this method is to help optimise $resultset->find by just returning a single row without creating a whole cursor.  However, DBIC::Storage::DBI->select_single is also publicly exposed as $resultset->single and is documented as a optimised select for a single row.  Nothing in the docs or previous versions warned users that >single should only be run against SQL that returns a single row.  In fact, one would think that ->find is reserved for this purpose.  So by adding this additional constraint at that point I am very certain we are going to be breaking peoples code.  I know in fact $resultset->single is used in a few spots in our codebase in places that will explode if the above carp is kept.

carp is a warning.

It's warning you that your code is broken.

Fix the bug, keep the warning.

There's no incompatiblity here at all, just that we're warning people with
broken code that they have broken code.

If your query doesn't return a single result, using single() makes no
sense.

-- 
      Matt S Trout       Need help with your Catalyst or DBIx::Class project?
   Technical Director                    http://www.shadowcat.co.uk/catalyst/
 Shadowcat Systems Ltd.  Want a managed development or deployment platform?
http://chainsawblues.vox.com/            http://www.shadowcat.co.uk/servers/