[Dbix-class] Adding UNIQUE constraint support

[Brandon Black]

On 12/16/05, Daniel Westermann-Clark <daniel at acceleration.net> wrote:
> In many of my applications, I use UNIQUE constraints.  To avoid
> violating these constraints, a lot of the code ends up looking
> something like:
>
> my $a = DBIC::Artist->find_or_create({ ... UNIQUE columns ... });
> $a->set_columns({ ... other columns ... });
> $a->update;
>
> This gets a little tedious, so I thought I could override
> find_or_create to abstract this away.  The result is
> DBIx::Class::Constraint::Unique, which you can find at:
>
> http://svn.westermann-clark.com/projects/DBIx-Class-Constraint-Unique/trunk/
>
> Thoughts?
>

I have a few thoughts, but take them with a grain of salt, I haven't
thought it all through as well as I'd like yet.

There's been discussion in PostgreSQL of having a native operation for
this, which will probably end up being called either "UPSERT"
(UPdate/inSERT) or "REPLACE" or be a part of the MERGE functionality -
it's kinda still up in the air at the moment.  But it's such a
commonly-wanted thing that it may become a standard feature as a
single combined statement, at least for PostgreSQL (I think either the
latest SQL standard or the next one coming down the pipe has one or
more of these keywords in it for these purposes, but I'm not sure how
soon other vendors might support it).

Anyways, my opinions on the module:

First, I think this is a very useful peice of functionality, and might
be better served ultimately as a patch to DBIx::Class::Table rather
than a seperate module, although putting it in a seperate module for
now is fine.

Second, I don't think what you're doing is really the same as
->find_or_create() anymore.  I would call it something more like
->update_or_create(), or ->create_or_update(), and leave the original
alone as it was.  The new function never just finds an existing record
and returns it intact, it either finds an existing record and updates
it to contain new data fields, or creates one with those same keys +
data fields.

Thirdly, the UNIQUE issue.  Your create/update function seems to
consider UNIQUE keys you have defined for the table, but not the PK
itself, which is logically a subset of the UNIQUE keys on a table, but
is already defined elsewhere with ->set_primary_key().  It would make
more sense for your function to consider the PK as part of the set of
UNIQUE keys as well.  And the api for defining UNIQUE keys should
probably mirror that of PK's as much as possible.  I'd call them
"Alternate Keys" or "Secondary Keys" instead, since that's more of a
generic relational theory term, whereas UNIQUE is sql-specific.

So to sum up my suggestions (not all of which anyone else on the
planet might agree with):

1) rename to your custom find_or_create() to update_or_create() or
something similar, instead of overriding find_or_create().
2) rename ->add_unique_constriants() to ->add_alternate_key() or
->add_secondary_key(), which gets called once per UNIQUE constraint,
with as close an interface to set_primary_key() as possible, and at
some point support for multi-value keys like the pk stuff does (you
can apply a unique constraint to a pair of columns together, like one
does for a multi-part PK).
3) make sure your find_or_create() (or update_or_create(), or whatever
it ultimately gets called) treats PKs on level footing with UKs.

In any case it's a definite improvement over the current situation
even if you choose to keep everything the way it is.  Don't take my
long critique as a bad thing :)

-- Brandon