[Dbix-class] find_or_create and unique constraints

[Bill Moseley]

On Sat, Oct 16, 2010 at 4:28 PM, Peter Rabbitson
<rabbit+dbic at rabbit.us<rabbit%2Bdbic at rabbit.us>
> wrote:

>
> So all you have to do now is override find() in your base resultset class
> to croak if no key attribute has been supplied and there you have it -
> fully tightened unique find for the entire app (which is also pretty hard
> to circumvent, at least by accident).
>

Hi Peter,

I only had a few minutes to review the diffs but I think it looks good.
 Thanks for spending so much time on this.  A few comments:

Maybe move this up after the paragraph describing the behavior if no key is
found because it's related:

 447 If the resulting query produces more than one row, a warning to the
effect is
 448 issued, though only the first row is constructed and returned as
C<$row_object>


as the only reason to get multiple rows is if you don't specify a unique key
(or if your unique constraint on the Result class doesn't match the database
-- one of my cases).

I wonder if it would be useful to add a note about making sure correct
unique constraints are defined in the result classes.  Another one of our
mistakes was not having a unique constraint defined so find() fell back to
doing the general search() which returned no rows, but then on create we hit
duplicate key errors.

Finally, did you intend to remove the example of the list of key values?

  my $cd =3D $schema->resultset('CD')->find('Massive Attack', 'Mezzanine', {
    key =3D> 'cd_artist_title'
  });



As for overriding find(), I'm not sure I can just croak if no "key" is
provided quite yet.  It would take quite a bit of work to find all places
find is called directly or indirectly and add a "key".  I wonder how often
people specify a "key".  My guess is not very often -- especially since the
normal case is to only have one unique constraint.

Maybe I can override find() and if "key" is not provided attempt to
determine a unique constraint and set it -- and warn or croak if a unique
constraint cannot be determined.

That was a bit easier to do previously as I wrapped (the private)
_unique_queries. And since that was only called when no "key" was provided I
could just check if _unique_queries returned any columns (at least when
additional search criteria was provided to find() ) and warn/croak if no
unique queries could be found.  Not sure how to do that now.


> That said I am starting to ponder the idea of an app-wide strict switch,
> however this is not something that is likely to happen this month.
>

A switch might be a good way to offer more strict-ness.  For  now, what
about just abstracting out two methods? Make a public method that handles
the no-"key" else block in find() like _unique_queries did before, and
another method to generate the warning about multiple rows.  Then those can
be overridden to enforce more strict behavior.  The difficulty there is the
unique query might just be on the result set "cond" so it's not as that
simple to determine if a unique constraint was found in the combination of
the passed in criteria and resultset condition just by looking at the output
from a _unique_queries type of method.  Maybe you have a better idea?


Thanks,



-- =

Bill Moseley
moseley at hank.org
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.scsys.co.uk/pipermail/dbix-class/attachments/20101017/635=
293cf/attachment.htm