Re: Review: Non-inheritable check constraints

Excerpts from Alex Hunsaker's message of dom oct 09 03:40:36 -0300 2011:
> On Fri, Oct 7, 2011 at 21:30, Nikhil Sontakke <nikkhils(at)gmail(dot)com> wrote:
> > Hi Alex,
> >
> > I guess we both are in agreement with each other :)
> >
> > After sleeping over it, I think that check is indeed dead code with this new
> > non-inheritable check constraints functionality in place. So unless you have
> > some other comments, we can mark this as 'Ready for Commiter'.
> >
> > Again, thanks for the thorough review and subsequent changes!
>
> PFA an updated patch with the check removed and a comment or two added.
>
> I've also marked it ready.

I had a look at this patch today. The pg_dump bits conflict with
another patch I committed a few days ago, so I'm about to merge them.
I have one question which is about this hunk:

@@ -2312,6 +2317,11 @@ MergeWithExistingConstraint(Relation rel, char *ccname, Node *expr,
con->conislocal = true;
else
con->coninhcount++;
+ if (is_only)
+ {
+ Assert(is_local);
+ con->conisonly = true;
+ }
simple_heap_update(conDesc, &tup->t_self, tup);
CatalogUpdateIndexes(conDesc, tup);
break;

Is it okay to modify an existing constraint to mark it as "only", even
if it was originally inheritable? This is not clear to me. Maybe the
safest course of action is to raise an error. Or maybe I'm misreading
what it does (because I haven't compiled it yet).

--
Álvaro Herrera <alvherre(at)commandprompt(dot)com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support