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