[Dbix-class] patch for ResultSet::find_or_new

[Zbigniew Lukasiak]

On Jan 13, 2008 7:20 PM, Matt S Trout <dbix-class at trout.me.uk> wrote:
> On Sun, Jan 13, 2008 at 10:41:43AM +0100, Patrick Weemeeuw wrote:
> > There is a small bug in find_or_new: when the find part
> > fails, it calls new_result with the hash containing the
> > values that were used for the search. It should use no
> > values at all instead.
>
> This isn't a bug. If that's the behaviour you want, do
>
> my $o = $rs->find({ id => $id }) || $rs->new({});
>
> > Example buggy case: $o = $...->find_or_new( { id => $id } )
> > with id a not null primary key. When $id is undefined, there
> > is obviously no row in the DB, and a new result object is
> > returned. However, the object returned contains the column
> > id => NULL, which (1) is invalid for this kind of object,
> > and (2) prevents in some backends (e.g. Pg) that the
> > sequence is used to generate a unique id.
>
> So don't pass id if it isn't a valid value. Passing undef there is a bug
> in your code, not in DBIx::Class.
>
> The usual use of find_or_new is to pass a unique key plus additional
> attributes to be used for object creation (which are ignored in the find()
> by specifying the key attr as well). Consider for example
>
> my $stats = $schema->resultset('PageViews')->find_or_new(
>               { page => $page, views => 0 },
>               { key => 'page' }
>             );
>

How can we excercise the _or_new part of find_or_new?

>From the above I conclude that we should omit 'page' in the request
(instead of setting it to 'undef'):

my $stats = $schema->resultset('PageViews')->find_or_new(
               { views => 0 },
               { key => 'page' }
             );
but then if we have another record with views == 0 it will be found
and no new row would be created.

This is because find falls back on finding by all columns in the query
(ie by 'views' in this case)
if the key is not represented in the query - this is not documented,
but it is commented in the source:

# @unique_queries = () if there is no 'key' in $input_query

# Build the final query: Default to the disjunction of the unique queries,
# but allow the input query in case the ResultSet defines the query or the
# user is abusing find
my $alias = exists $attrs->{alias} ? $attrs->{alias} : $self->{attrs}{alias};
my $query = @unique_queries
    ? [ map { $self->_add_alias($_, $alias) } @unique_queries ]
    : $self->_add_alias($input_query, $alias);

I would say - we should get rid of that 'special feature'.

-- 
Zbigniew Lukasiak
http://brudnopis.blogspot.com/