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