On Jan 14, 2008, at 2:19 PM, Zbigniew Lukasiak wrote:

On Jan 13, 2008 7:20 PM, Matt S Trout <[EMAIL PROTECTED]> 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.

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 PROTECTED] - 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/[EMAIL PROTECTED]

Reply via email to