[Dbix-class] patch for ResultSet::find_or_new

Tue Jan 15 08:54:26 GMT 2008

On Jan 15, 2008 9:40 AM, Moritz Onken <onken at houseofdesign.de> wrote:
> BTW:
>
> http://search.cpan.org/~ash/DBIx-Class-0.08008/lib/DBIx/Class/ResultSet.pm#ATTRIBUTES
> doesn't contain the "key" attribute.

Thanks for backing your arguments with links to the documentation. So
I'll do this as well:

"Also takes an optional key attribute, to search by a specific key or
unique constraint. For example:"

from: http://search.cpan.org/~ash/DBIx-Class-0.08008/lib/DBIx/Class/ResultSet.pm#find_or_create

My understanding is that 'find_or_new' and 'find_or_create' use the
same attributes - this is both logical and is supported by the source
code.  This 'key' attribute comes from 'find':
http://search.cpan.org/~ash/DBIx-Class-0.08008/lib/DBIx/Class/ResultSet.pm#find

Cheers,
Zbigniew

>
>
>
> Am 15.01.2008 um 08:00 schrieb Zbigniew Lukasiak:
>
>
> > On Jan 14, 2008 10:59 PM, Jason Kohles <email at jasonkohles.com> wrote:
> >>
> >> On Jan 14, 2008, at 2:19 PM, Zbigniew Lukasiak wrote:
> >>
> >>> 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?
> >>>
> >>
> >> The results you are seeing are *exactly* the way it is expected to
> >> work.  In this example you are doing:
> >>
> >> $rs->find_or_new( { id => undef } )
> >>
> >> and getting back an object where id is undefined.  How is that
> >> mysterious or incorrect behaviour?  You told it 'find me a row in the
> >> database where id is undef, and if you can't find one, create a new
> >> object where id is undef' and that's exactly what it did.
> >>
> >> The big difference between ->find_or_new and ->find_or_create, is
> >> that
> >> ->find_or_new doesn't attempt to do the insert yet, and so is not
> >> guaranteed to return to you an object that even *can* be inserted.
> >> What I would do in this case is either not use ->find_or_new when you
> >> don't have a valid primary key, or do something like this:
> >>
> >> my $obj = $rs->find_or_new( $id ? { id => $id } : {} );
> >>
> >> or even...
> >>
> >> my $obj = $rs->find_or_new( { id => $id } );
> >> if ( ! defined $obj->id ) { $obj->id( 'foo' ) }
> >>
> >>
> >>>> 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.
> >>>
> >> You've just created an even more bizarre situation, what you have now
> >> is essentially
> >>
> >> my $rs = $schema->resultset( 'PageViews' );
> >> my $stats = $rs->find( {} ) || $rs->new( { views => 0 } );
> >>
> >>
> >> The documentation for find_or_create says "Tries to find a record
> >> based on its primary key or unique constraint; if none is found,
> >> creates one and returns that instead."  What you are trying to do is
> >> use it without providing a primary key or a unique constraint, and
> >> the
> >> behavior you are seeing is exactly what I would expect to happen in
> >> that situation, since ->find can't match anything without a primary
> >> key or a unique constraint, you get back a new object instead.
> >
> > No - that is not what would happen.  No new record will be created if
> > you have another record that have views == 0.
> > This is very important - the behaviour is not what is documented - but
> > it is intentional - as the comment in the code shows.
> >
> > --
> > Zbigniew
> >
> >
> >>> 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'.
> >>>
> >>
> >> I would say it's working exactly as intended, you are abusing find
> >> and
> >> it's doing the best it can to accomodate you, by creating new objects
> >> when your ->find fails to find a matching row.  The only
> >> alternative I
> >> can see would be for it to throw an exception when you try and call
> >> find_or_create without the
> >>
> >> --
> >> Jason Kohles, RHCA RHCDS RHCE
> >> email at jasonkohles.com - http://www.jasonkohles.com/
> >> "A witty saying proves nothing."  -- Voltaire
> >>
> >>
> >>
> >>
> >> _______________________________________________
> >> 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.rawmode.org
> >>
> >
> >
> >
> > --
> > Zbigniew Lukasiak
> > http://brudnopis.blogspot.com/
> >
> > _______________________________________________
> > 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.rawmode.org
>
>
> _______________________________________________
> 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.rawmode.org
>

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