On Wed, Oct 24, 2012 at 12:06:35PM -0400, Tom Lane wrote:
> Noah Misch <n...@leadboat.com> writes:
> > For FKs, we currently document that "The referenced columns must be the
> > columns of a non-deferrable unique or primary key constraint in the 
> > referenced
> > table."  Taking that literally, one might imagine that bare UNIQUE indexes 
> > do
> > not qualify.  However, transformFkeyCheckAttrs() does accept them, including
> > indexes with non-default operator classes:
> 
> Indeed, and considerable sweat was spilled to make that happen.  I'm
> pretty unimpressed with any proposal that we should just blow that off
> for array keys.  Now, I concede that cross-type FKs are a corner case to
> begin with, and we may well end up concluding that it's just too much
> work to handle it for arrays because of the lack of infrastructure for
> applying non-default comparison operators to arrays.  But I don't want
> that to happen just because we failed to even think about it.

Sure, it's important to raise for discussion.  The way I see it, it's the
existing functions and operators for arrays that blew off non-default element
operator classes.  This patch is just dealing with those building blocks on
their own terms.  I would hesitate to give up cross-type support, but support
for a non-default operator class on the PK side seems expendable.  Given the
limitations that I mentioned for the corresponding feature of ordinary foreign
keys, I'm skeptical of its importance for ELEMENT foreign keys.

On further reflection, we could stop short of preemptively forbidding
non-default operator classes and just teach transformFkeyCheckAttrs() to
select an affected index only as a last resort.  The
equality_ops_are_compatible() check in ATAddForeignKeyConstraint() may proceed
to reject the index.  A text_pattern_ops UNIQUE index uses the same equality
operator as a UNIQUE constraint, and it would continue to be rightly accepted.

> However, I'm about to bounce this patch back for rework anyway, because
> I've just noticed that it has fatal performance issues.  If you issue
> any UPDATE or DELETE against the PK table, you get a query like this
> (shorn of some uninteresting syntactic details) for checking to see
> if the RI constraint would be violated:
> 
> SELECT 1 FROM ONLY fktable x WHERE $1 = ANY (fkcol) FOR SHARE OF x;
> 
> It is impossible to implement this query except with a full-table
> seqscan on the FK table.  You can put a GIN index on the array fkcol,
> but that won't help, because "something = ANY (indexedcol)" isn't an
> indexable condition.  I don't think we can ship a feature that's
> unusable for anything except toy-sized tables, and that's what this is
> right now.
> 
> One way we could consider making this GIN-indexable is to change it to
> 
> SELECT 1 FROM ONLY fktable x WHERE ARRAY[$1] <@ fkcol FOR SHARE OF x;
> 
> However, that puts us right back into the problem that we have no
> control over the specific comparison semantics that <@ uses.
> 
> Or we could try to teach PG to make "something = ANY (indexedcol)"
> indexable.  That would likely be a pretty substantial amount of work
> though.  In particular, matching such a query to a GIN index would
> require knowing whether the "=" operator corresponds to the idea of
> equality embodied in the GIN opclass's key compare() method, and that
> isn't information that's available from the current opclass API.

Perhaps, then, excluding all cross-type ELEMENT FKs is the right start.  With
that and the operator compatibility check already appearing in the patch, we
can prove that <@ has the right semantics.  Doing better adds either large
subprojects or seemingly-ad-hoc limitations.  It's ugly, but only in a manner
comparable to "ARRAY[0.0] < ARRAY[0]" finding no operator.

Thanks,
nm


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to