On Jan 21, 2008 11:27 AM, Matt S Trout <[EMAIL PROTECTED]> wrote:
> On Fri, Jan 18, 2008 at 01:05:30PM +0100, Zbigniew Lukasiak wrote:
> > I think I have it well documented in the "rs->find finds by all
> > columns" thread that find behaviour is rather not reliable when it is
> > not fed a unique constraint. By the way here is another question -
> > what should happen when you call find with just one of the two fields
> > in a composed primary key? Currently it would return just first
> > record with that one field set to the searched value, I don't know if
> > that is a bug - but it is surprising isn't it?
> >
> > In fact all of this is not that bad - because it can also be viewed as
> > an error on the application side to try to find something without
> > supplying a unique condition.
> >
> > The problem is that update_or_create relies on calling find without a
> > unique constraint. And more precisely it needs to do that if you want
> > to write a generic handler for updating or creating rows that would
> > work for all databases. The reason for that is that $rs->create( {
> > primary_key => undef, ... } ) will not work for Pg so when you call
> > update_or_create and you have no primary key you need to delete it
> > from the query.
>
> I don't understand that. If you don't have a PK value, don't pass the
> column. This works perfectly and is orthogonal to your proposed find changes.
Thanks for finding time to answer. The problem with just removing the
PK column is that this query without PK will be passed to find - and
find has many bugs that appear in that case (I have documented them in
the other emails). The reason for that is that finding without a
unique condition is a special case - it's not really obvious what it
should return - it could even be a run time error. Find is based on
search, normally when you remove a condition from search it should
return more rows - but here when you remove a condition from find, to
cooperate with the find_or_* methods, it should return no rows at all.
We can force it to do that - but this is quite complex, because you
can have the conditions not only in the query but also in the
resultset itself. This needs to be done very carefully.
But I am OK with that solution - in fact I have sent a patch that
worked for most of the cases that I imagined, but I am sure it would
benefit from your review. I am attaching it here once again.
Cheers,
Zbigniew
http://perlalchemy.blogspot.com/
Index: t/61findnot.t
===================================================================
--- t/61findnot.t (revision 3948)
+++ t/61findnot.t (working copy)
@@ -7,7 +7,7 @@
my $schema = DBICTest->init_schema();
-plan tests => 17;
+plan tests => 20;
my $art = $schema->resultset("Artist")->find(4);
ok(!defined($art), 'Find on primary id: artist not found');
@@ -44,3 +44,15 @@
@cd = $cd->single;
cmp_ok(@cd, '==', 1, 'Return something even in array context');
ok(@cd && !defined($cd[0]), 'Array contains an undef as only element');
+
+my $random_girl = $schema->resultset("Artist")->create({ name => 'Random Girl Band'});
+$art = $schema->resultset("Artist")->find({name => 'Random Girl Band'}, {dont_fallback => 1});
+ok(!defined($art), 'Find on primary key with no key provided: artist not found');
+
+my $random_again = $schema->resultset("Artist")->search( artistid => $random_girl->id )->find({}, {dont_fallback => 1});
+ok(defined($random_again), 'Find on primary key with no key provided on a narrow resultset: record found');
+
+$cd = $schema->resultset("CD")->first;
+$art = $cd->find_related( 'artist', {}, {dont_fallback => 1} );
+ok( defined $art, 'find_related for a belongs_to relation traversed' );
+
Index: lib/DBIx/Class/ResultSet.pm
===================================================================
--- lib/DBIx/Class/ResultSet.pm (revision 3948)
+++ lib/DBIx/Class/ResultSet.pm (working copy)
@@ -321,6 +321,11 @@
If no C<key> is specified, it searches on all unique constraints defined on the
source, including the primary key.
+If you specify columns explicitely and there are no columns with unique constraints
+among them then C<find> currently will fall back to search by all other columns.
+Don't depend on that functionality - as it can change in the future. To not fallback
+set the 'dont_fallback' attribute to 1;
+
If your table does not have a primary key, you B<must> provide a value for the
C<key> attribute matching one of the unique constraints on the source.
@@ -381,10 +386,24 @@
# 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);
-
+ my $query;
+ if ( ! @unique_queries ){
+ if ( delete $attrs->{dont_fallback} ) {
+ if( $self->_has_uniq_cond ){
+ $query = {};
+ }
+ else {
+ return undef;
+ }
+ }
+ else{
+ carp "No unique fields found in query. Falling back to search by all query fields. This usage is deprecated. For not falling back use 'dont_fallback' attribute";
+ $query = $self->_add_alias($input_query, $alias);
+ }
+ }
+ else{
+ $query = [ map { $self->_add_alias($_, $alias) } @unique_queries ];
+ }
# Run the query
if (keys %$attrs) {
my $rs = $self->search($query, $attrs);
@@ -397,6 +416,21 @@
}
}
+# _has_uniq_cond
+#
+# Check if the resultset condition identifies one row
+
+sub _has_uniq_cond {
+ my ( $self ) = @_;
+ my @condition_keys = keys %{ $self->{cond} };
+ my $source = $self->result_source;
+ for my $key ( $source->primary_columns ){
+ return 0 if not grep { $key } @condition_keys;
+ }
+ return 1;
+}
+
+
# _add_alias
#
# Add the specified alias to the specified query hash. A copy is made so the
_______________________________________________
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]
