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

On Sun, 2011-01-23 at 19:50 +0200, Marko Tiikkaja wrote:
> 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

Thanks for the review.

> , 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.

Thanks. Will fix.

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

Should it?
What command do you think needs changing?

> 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?

Yes, the consensus was that DDL was required, not a function. Function
was my preferred approach originally.

That now appears to be an additional request from a couple of people. At
present, its easy enough to write the SQL statement yourself, so that's
non-essential, and maybe/likely won't make this release (not sure,
depends upon how other aspects go).

There is no option to invoke this yet from pg_restore, which seems
likely to top the list of priorities. Would you agree?

-- Simon Riggs           http://www.2ndQuadrant.com/books/PostgreSQL Development, 24x7 Support, Training and Services