This works:

    my $rs = $schema->resultset('CD')->search(
        { artist => 1 },
        { order_by => 'id' },
    );

Oddly, so does this:

    my $rs = $schema->resultset('CD')->search(
        { artist => 1 },
        undef,
        { order_by => 'id' },
    );

I'm finding it hard to imagine that's deliberate.  Attached is a
patch (against 0.08/trunk) which (a) tests that this code throws an
exception, and (b) throws such an exception.  All other tests seem to
pass.

If the consensus is that the current behaviour is correct, I'll
happily concoct an alternative patch with appropriate documentation
and tests.

(Or if you'd like me to regenerate this patch against an different
branch, that's also fine.)

I've also attached a second patch which refactors some surrounding
code in a way that I find much easier to understand.  It should apply
to 0.08/trunk either before or after the behaviour-changing patch.
Again, all tests still pass.

-- 
Aaron Crane ** http://aaroncrane.co.uk/
Index: t/60core.t
===================================================================
--- t/60core.t	(revision 4387)
+++ t/60core.t	(working copy)
@@ -7,7 +7,7 @@
 
 my $schema = DBICTest->init_schema();
 
-plan tests => 78;
+plan tests => 79;
 
 eval { require DateTime::Format::MySQL };
 my $NO_DTFM = $@ ? 1 : 0;
@@ -277,6 +277,11 @@
   ok($schema->source('ArtistSubclass'), 'ArtistSubclass exists');
 }
 
+{
+  my $rs = eval { $schema->resultset('CD')->search({artist => 1}, undef, {order_by => 'me.id'}) };
+  like($@, qr/excess/i, 'Excess search parameters throw exception');
+}
+
 my $newbook = $schema->resultset( 'Bookmark' )->find(1);
 
 $@ = '';
Index: lib/DBIx/Class/ResultSet.pm
===================================================================
--- lib/DBIx/Class/ResultSet.pm	(revision 4387)
+++ lib/DBIx/Class/ResultSet.pm	(working copy)
@@ -193,6 +193,9 @@
     $rows = $self->get_cache;
   }
 
+  $self->throw_exception('Excess arguments to search')
+    if ref $_[0] eq 'HASH' && @_ > 1;
+
   my $new_attrs = { %{$our_attrs}, %{$attrs} };
 
   # merge new attrs into inherited
Index: lib/DBIx/Class/ResultSet.pm
===================================================================
--- lib/DBIx/Class/ResultSet.pm	(revision 4387)
+++ lib/DBIx/Class/ResultSet.pm	(working copy)
@@ -183,12 +183,8 @@
 
   my %safe = (alias => 1, cache => 1);
 
-  unless (
-    (@_ && defined($_[0])) # @_ == () or (undef)
-    || 
-    (keys %$attrs # empty attrs or only 'safe' attrs
-    && List::Util::first { !$safe{$_} } keys %$attrs)
-  ) {
+  # no arg list (or 0th arg just undef), and no attrs (or only 'safe' attrs)
+  if (!defined($_[0]) && !List::Util::first { !$safe{$_} } keys %$attrs) {
     # no search, effectively just a clone
     $rows = $self->get_cache;
   }
@@ -201,25 +197,12 @@
     $new_attrs->{$key} = $self->_merge_attr($our_attrs->{$key}, $attrs->{$key});
   }
 
-  my $cond = (@_
-    ? (
-        (@_ == 1 || ref $_[0] eq "HASH")
-          ? (
-              (ref $_[0] eq 'HASH')
-                ? (
-                    (keys %{ $_[0] }  > 0)
-                      ? shift
-                      : undef
-                   )
-                :  shift
-             )
-          : (
-              (@_ % 2)
-                ? $self->throw_exception("Odd number of arguments to search")
-                : [EMAIL PROTECTED]
-             )
-      )
-    : undef
+  my $cond = (
+    [EMAIL PROTECTED]                 ? undef
+  : ref $_[0] eq 'HASH' ? (keys %{ $_[0] } > 0 ? shift : undef)
+  : @_ == 1             ? shift
+  : @_ % 2              ? $self->throw_exception('Odd number of arguments to search')
+  :                       { @_ }
   );
 
   if (defined $where) {
_______________________________________________
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