REVIEW: ALTER TABLE ... ADD FOREIGN KEY ... NOT ENFORCED

From: Marko Tiikkaja <marko(dot)tiikkaja(at)cs(dot)helsinki(dot)fi>
To: Simon Riggs <simon(at)2ndQuadrant(dot)com>
Cc: Peter Geoghegan <peter(dot)geoghegan86(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Dimitri Fontaine <dimitri(at)2ndquadrant(dot)fr>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers(at)postgresql(dot)org
Subject: REVIEW: ALTER TABLE ... ADD FOREIGN KEY ... NOT ENFORCED
Date: 2011-01-23 17:50:45
Message-ID: 4D3C6A75.9050204@cs.helsinki.fi
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Simon,

On 1/14/2011 1:15 PM, Simon Riggs wrote:
> Patch to implement the proposed feature attached, for CFJan2011.

Overall, I think the patch looks good, but I found some problems with
it. In tablecmds.c you have:

+ if (found && con->contype == CONSTR_FOREIGN && !con->convalidated)

which I don't think is correct, and my tests seem to agree; the actual
validation doesn't happen at all. Changing that to CONSTRAINT_FOREIGN
makes the validation part work, but then I get:

ERROR: cache lookup failed for constraint 16419

when trying to drop the table and the regression tests fail because of
this. Also having a regression test where the validation fails seems
like a good idea.

Another problem I found is that psql doesn't indicate in any way that a
FOREIGN KEY constraint is not validated yet.

I also think that having the function for getting a list of values that
violate the constraint would be helpful. Any particular reason why you
decided to omit it from this patch?

Regards,
Marko Tiikkaja

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Simon Riggs 2011-01-23 18:23:52 Re: REVIEW: ALTER TABLE ... ADD FOREIGN KEY ... NOT ENFORCED
Previous Message Robert Haas 2011-01-23 17:39:10 Re: Bug in pg_describe_object, patch v2