Re: Review: Non-inheritable check constraints

From: Nikhil Sontakke <nikkhils(at)gmail(dot)com>
To: Alvaro Herrera <alvherre(at)commandprompt(dot)com>
Cc: Alex Hunsaker <badalex(at)gmail(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Review: Non-inheritable check constraints
Date: 2011-12-04 07:22:56
Message-ID: CANgU5ZfV36O01nf6yr3nDQg+q6gVPpVrHRSSL-iU5_6OwE743Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: 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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Pavel Stehule 2011-12-04 07:40:31 planner fails on HEAD
Previous Message Tom Lane 2011-12-04 04:19:54 Re: cannot read pg_class without having selected a database / is this a bug?