Re: Review: Non-inheritable check constraints - Mailing list pgsql-hackers

From Nikhil Sontakke
Subject Re: Review: Non-inheritable check constraints
Date
Msg-id CANgU5ZfV36O01nf6yr3nDQg+q6gVPpVrHRSSL-iU5_6OwE743Q@mail.gmail.com
Whole thread Raw
In response to Re: Review: Non-inheritable check constraints  (Alvaro Herrera <alvherre@commandprompt.com>)
Responses Re: Review: Non-inheritable check constraints  (Greg Smith <greg@2ndQuadrant.com>)
List pgsql-hackers

 
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:


Thanks for taking a look Alvaro.
 
@@ -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).


Hmmm, good question. IIRC, the patch will pass is_only as true only if it going to be a locally defined, non-inheritable constraint. So I went by the logic that since it was ok to merge and mark a constraint as locally defined, it should be ok to mark it non-inheritable from this moment on with that new local definition?

Regards,
Nikhils


Regards,
Nikhils

pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: cannot read pg_class without having selected a database / is this a bug?
Next
From: Pavel Stehule
Date:
Subject: planner fails on HEAD