Bill Moseley wrote:
On Sat, Oct 16, 2010 at 1:57 AM, Peter Rabbitson <[email protected]
<mailto:rabbit%[email protected]>> wrote:
I should have worded it differently, I meant "architectural
correctness".
But I respectfully disagree with the part of your post about "violating
principle of least surprise". All mature APIs have warts, and DBIC is by
far no exception. While breaking compat to correct glaring deficiencies
in operation is crucial, breaking compat *just* for the sake of removing
such "the api is ugly" warts is unacceptable.
What started this thread wasn't a codepath failure, it was a blatant
documentation failure, and this is being rectified asap.
Hi Peter,
We have perhaps a dozen people that may touch the app code, and the sad
truth is not all, no few, read the docs well enough. That's in no way
DBIC's fault but it's what often happens in a busy office. Clearly, we
don't want to break the existing API in anyway -- surprises due to an
upgrade is never good. But, providing a way for people (managers) to
tighten the API can be useful.
Indeed, read on.
(quoting from previous message)
Do you think that a stronger warning about multiple rows and non
unique constraints used in find() with, say, an option "strict_find
=> 1" that could be added to enforce it with the goal to make it the
default in later versions would still break DBIC for many existing
users?
I went to look at the docs and realized that they do not actually
talk about this at all, so we started with different initial info.
This option had existed since the very beginning and is called
'key', which basically means "you either satisfy this particular
unique constraint or you don't find anything". Without a 'key'
argument, find() becomes a best-effort wrapper around search() with
some extra juice.
I'm aware of the "key" option but have relied on the "automatic"
approach find uses if not provided a key to determine a unique
constraint. My concern is I doubt that it would be used consistently
w/o some type of enforcement. I wonder how common its usage is.
Also, I IIRC, it's possible to specify a key but not provide any or all
of the key's columns which results in a non-unique query. (Of course
there's nothing to prevent calling $rs->find w/o any keys either.)
This has now been fixed, thank you for making me look at the code close
enough to realize how bad it really was :)
http://git.shadowcat.co.uk/gitweb/gitweb.cgi?p=dbsrgits/DBIx-Class.git;a=commit;h=95d028fecb626f3a90438c51dfdac98bdaf9eab3
Again, it's pretty easy for me to add code to report on non-unique
constraints and when duplicate rows are returned (as we had because a
column was added as a unique constraint in the db but not updated in
DBIC). I've now disabled it as we fix code and clean up the database
(as it overwhelmed the logs). But, it would have been great to have it
from the start of this project to catch early on. I suspect others
could easily fall into the same situation.
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).
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.
Also please offer your comments on the new documentation:
diff:
http://git.shadowcat.co.uk/gitweb/gitweb.cgi?p=dbsrgits/DBIx-Class.git;a=commitdiff;h=8e0a1115f5fbe517102444fd52d51d7d2a610795
result:
http://git.shadowcat.co.uk/gitweb/gitweb.cgi?p=dbsrgits/DBIx-Class.git;a=blob;f=lib/DBIx/Class/ResultSet.pm;hb=8e0a1115f5fbe517102444fd52d51d7d2a610795
Thanks for persisting :)
Cheers!
_______________________________________________
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]
