Skip site navigation (1) Skip section navigation (2)

Re: [PATCH] Support for Array ELEMENT Foreign Keys

From: Noah Misch <noah(at)leadboat(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>,Andres Freund <andres(at)2ndquadrant(dot)com>,pgsql-hackers(at)postgresql(dot)org, Andrew Dunstan <andrew(at)dunslane(dot)net>,Marco Nenciarini <marco(dot)nenciarini(at)2ndquadrant(dot)it>
Subject: Re: [PATCH] Support for Array ELEMENT Foreign Keys
Date: 2012-11-01 00:46:03
Message-ID: 20121101004603.GA3006@tornado.leadboat.com (view raw or flat)
Thread:
Lists: pgsql-hackers
On Wed, Oct 24, 2012 at 12:06:35PM -0400, Tom Lane wrote:
> Noah Misch <noah(at)leadboat(dot)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


In response to

pgsql-hackers by date

Next:From: Peter van HardenbergDate: 2012-11-01 01:39:20
Subject: Synchronous commit not... synchronous?
Previous:From: Daniel FarinaDate: 2012-10-31 23:37:41
Subject: Re: Creating indexes in the background

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group