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.
I was lead to this because the mysql error was due to the fact the line:
carp "Query returned more than one row" if $sth->fetchrow_array;
is going to die when the $sth is exhausted (which was generating 'fetch without
execute' errors). I could fix this to work properly, but I really feel it's
not such a good idea to change the API behavior like this without some sort of
deprecation cycle. In addition I am not certain why should change it. I can
see a need for a quick, "get me the first something you find" function.
However at the very least we should update the docs to reflect clarified
semantices between $resultset->first, ->single, and ->find.
So what do you all think? Our options are:
-1- Revert ->select_single to the old behavior but deprecate $resultset->single
so that we can eventually remove it from the API at some point, freeing us to
constraint ->select_single when it's no longer part of the public API
-2- Revert ->select_single to the old behavior and deprecate $resultset->single
for SQL that returns multiple rows. Change the carp to some sort of warn and
fix the 'fetch without execute' bug.
-3- Revert ->select_single to the old behavior and properly document the
existing behavior, in particular to clarify the differences between ->first,
->single, and ->find. This is basically reverting to the 0.08010 behavior,
documenting it, adding a test, etc.
-4-> Push this change through. Fix the 'fetch without excute bug' and tell
everyone they need to grep/ack through their code and replace ->single with
either ->first or ->find for the cases the underlying SQL could return more
than one row.
To be honest I prefer option 3, since that's the easiest thing for me and my
deployment. I really don't see much of a use for $resultset->single that only
is useful against SQL that is guaranteed to return a single row. Seems to me
that is what ->find is for. However I do see value in a cursorless form of
->first, since sometimes you just want to grab the first thing as fast as
possible.
Certainly at the minimum we should clarify the docs. I think we should push
people away from ->single if what they really want in ->first.
I'm ready to make these changes, fix up the docs, add a test. Let's just
decide so it's part of this next release.
--John
____________________________________________________________________________________
Be a better friend, newshound, and
know-it-all with Yahoo! Mobile. Try it now.
http://mobile.yahoo.com/;_ylt=Ahu06i62sR8HDtDypao8Wcj9tAcJ
_______________________________________________
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/[EMAIL PROTECTED]
