Re: Review: Non-inheritable check constraints

From: Nikhil Sontakke <nikkhils(at)gmail(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Alvaro Herrera <alvherre(at)commandprompt(dot)com>, Greg Smith <greg(at)2ndquadrant(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Review: Non-inheritable check constraints
Date: 2011-12-19 17:07:02
Message-ID: CANgU5ZdMeVHj_gVAP+JhauC8TtNmrMDtH1n4=F0jMb+UE6oEaA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> It seems hard to believe that ATExecDropConstraint() doesn't need any
> adjustment.
>
>
Yeah, I think we could return early on for "only" type of constraints.

Also, shouldn't the systable scan break out of the while loop after a
matching constraint name has been found? As of now, it's doing a full scan
of all the constraints for the given relation.

> @@ -6755,6 +6765,7 @@ ATExecDropConstraint(Relation rel, const char
*constrName,
> HeapTuple tuple;
> bool found = false;
> bool is_check_constraint = false;
> + bool is_only_constraint = false;
>
> /* At top level, permission check was done in ATPrepCmd, else do it
*/
> if (recursing)
> @@ -6791,6 +6802,12 @@ ATExecDropConstraint(Relation rel, const char
*constrName,
> /* Right now only CHECK constraints can be inherited */
> if (con->contype == CONSTRAINT_CHECK)
> is_check_constraint = true;
> +
> + if (con->conisonly)
> + {
> + Assert(is_check_constraint);
> + is_only_constraint = true;
> + }
>
> /*
> * Perform the actual constraint deletion
> @@ -6802,6 +6819,9 @@ ATExecDropConstraint(Relation rel, const char
*constrName,
> performDeletion(&conobj, behavior);
>
> found = true;
> +
> + /* constraint found - break from the while loop now */
> + break;
> }
>
> systable_endscan(scan);
> @@ -6830,7 +6850,7 @@ ATExecDropConstraint(Relation rel, const char
*constrName,
> * routines, we have to do this one level of recursion at a time; we
can't
> * use find_all_inheritors to do it in one pass.
> */
> - if (is_check_constraint)
> + if (is_check_constraint && !is_only_constraint)
> children = find_inheritance_children(RelationGetRelid(rel),
lockmode);
> else
> children = NIL;

PFA, revised version containing the above changes based on Alvaro's v4
patch.

Regards,
Nikhils

Attachment Content-Type Size
non_inh_check_v5.0.patch application/octet-stream 30.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Marti Raudsepp 2011-12-19 17:08:23 Re: Postgres 9.1: Adding rows to table causing too much latency in other queries
Previous Message David Fetter 2011-12-19 17:07:00 Re: Page Checksums