Hi,
> Yeah, but I think we need to take that chance. At the very least, we
>> need to support the equivalent of a non-inherited CHECK (false) on
>> parent tables.
>>
>
> Indeed. I usually enforce that with a trigger that raises an exception, but
> of course that doesn't help at all with constraint exclusion, and I saw a
> result just a few weeks ago (I forget the exact details) where it appeared
> that the plan chosen was significantly worse because the parent table wasn't
> excluded, so there's a non-trivial downside from having this restriction.
>
>
The downside appears to be non-trivial indeed! I cooked up the attached
patch to try to allow ALTER...ONLY...CHECK(false) on parent tables.
If this approach looks acceptable, I can provide a complete patch later with
some documentation changes (I think we ought to tell about this special case
in the documentation) and a minor test case along with it (if the need be
felt for the test case).
Although partitioning ought to be looked at from a different angle
completely, maybe this small patch can help out a bit in the current scheme
of things, although this is indeed a unusual special casing... Thoughts?
Regards,
Nikhils
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 82bb756..5340402 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -5433,6 +5433,7 @@ ATAddCheckConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
ListCell *lcon;
List *children;
ListCell *child;
+ bool skip_children = false;
/* At top level, permission check was done in ATPrepCmd, else do it */
if (recursing)
@@ -5502,9 +5503,31 @@ ATAddCheckConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
* tables; else the addition would put them out of step.
*/
if (children && !recurse)
- ereport(ERROR,
+ {
+ /*
+ * Try a bit harder and check if this is a CHECK(FALSE) kinda
+ * constraint. Allow if so, otherwise error out
+ */
+ if (list_length(newcons) == 1)
+ {
+ CookedConstraint *cooked = linitial(newcons);
+
+ if (cooked->contype == CONSTR_CHECK && cooked->expr)
+ {
+ Node *expr = cooked->expr;
+ if (IsA(expr, Const) && ((Const *)expr)->consttype == BOOLOID &&
+ ((Const *)expr)->constvalue == 0)
+ {
+ skip_children = true;
+ }
+ }
+ }
+
+ if (!skip_children)
+ ereport(ERROR,
(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
errmsg("constraint must be added to child tables too")));
+ }
foreach(child, children)
{
@@ -5512,6 +5535,13 @@ ATAddCheckConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
Relation childrel;
AlteredTableInfo *childtab;
+ /*
+ * Skipping the constraint should be good enough for the special case.
+ * No need to even release the locks on the children immediately..
+ */
+ if (skip_children)
+ break;
+
/* find_inheritance_children already got lock */
childrel = heap_open(childrelid, NoLock);
CheckTableNotInUse(childrel, "ALTER TABLE");
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
