Hi,

We ran into a problem on 9.0beta3 with check constraints using table
inheritance in a multi-level hierarchy with multiple inheritance.

A test script is provided below and a proposed patch is attached to this
email.

Regards,

Henk Enting, Yeb Havinga
MGRID B.V.
http://www.mgrid.net





/*

First, create a local inheritance structure:

level_0_parent
level_0_child inherits level_0_parent

This structure is the base level. The table definition and also check
constraints are defined on this level.

Add two levels that inherit this structure:

level_1_parent inherits level_0_parent
level_1_child inherits level_1_parent, level_0_child

level_2_parent inherits level_1_parent
level_2_child inherits level_2_parent, level_1_child

BTW: there is a reason that we want e.g. level_1_child to inherit from
both level_1_parent and level_0_child:
we want the data of level_1_child to be visible in both level_0_child
and level_1_parent

*/
DROP SCHEMA IF EXISTS test_inheritance CASCADE;
CREATE SCHEMA test_inheritance;
SET search_path TO test_inheritance;

CREATE TABLE level_0_parent (i int);
CREATE TABLE level_0_child (a text) INHERITS (level_0_parent);

CREATE TABLE level_1_parent() INHERITS (level_0_parent);
CREATE TABLE level_1_child() INHERITS (level_0_child, level_1_parent);

CREATE TABLE level_2_parent() INHERITS (level_1_parent);
CREATE TABLE level_2_child() INHERITS (level_1_child, level_2_parent);


-- Now add a check constraint on the top level table:
ALTER TABLE level_0_parent ADD CONSTRAINT a_check_constraint CHECK (i IN
(0,1));


/*
Check the "coninhcount" attribute of pg_constraint

Doxygen says this about the parameter:
coninhcount: Number of times inherited from direct parent relation(s)

On our machine (running 9.0beta3) the query below returns a
coninhcount of 3 for the level_2_child table.

This doesn't seem correct because the table only has two direct
parents.
*/


SELECT t.oid, t.relname, c.coninhcount
FROM pg_class t
JOIN pg_constraint c ON (c.conrelid = t.oid)
JOIN pg_namespace n ON (t.relnamespace = n.oid)
WHERE n.nspname = 'test_inheritance'
ORDER BY t.oid;

-- Next, drop the constraint on the top level table

ALTER TABLE level_0_parent DROP CONSTRAINT a_check_constraint;

/*

The constraint should now be dropped from all the tables in the
hierarchy, but the constraint hasn't been dropped on the level_2_child
table. It is still there and has a coninhcount of 1.

*/

SELECT t.oid, t.relname, c.conname, c.coninhcount
FROM pg_class t
JOIN pg_constraint c ON (c.conrelid = t.oid)
JOIN pg_namespace n ON (t.relnamespace = n.oid)
WHERE n.nspname = 'test_inheritance'
ORDER BY t.oid;

/*
Trying to drop this constraint that shouldn't be there anymore won't work.

The "drop constraint" statement below returns:
ERROR:  cannot drop inherited constraint "a_check_constraint" of
relation "level_2_child"

NB after fixing this bug, the statement should return
"constraint does not exist"
*/

ALTER TABLE level_2_child DROP CONSTRAINT a_check_constraint;

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 5f6fe41..d23dcdc 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -96,6 +96,17 @@ typedef struct OnCommitItem
        SubTransactionId deleting_subid;
 } OnCommitItem;
 
+/*
+ * Visit information needed to prevent redundant constraint merges. This
+ * structure is needed to prevent faulty increments of coninhcount in the case
+ * of a multiple inheritance tree that has multiple paths to a parent.
+ */
+typedef struct ParentVisit
+{
+       Oid   parent;
+       Oid   child;
+} ParentVisit;
+
 static List *on_commits = NIL;
 
 
@@ -300,7 +311,7 @@ static void ATExecAddConstraint(List **wqueue,
 static void ATAddCheckConstraint(List **wqueue,
                                         AlteredTableInfo *tab, Relation rel,
                                         Constraint *constr,
-                                        bool recurse, bool recursing);
+                                        bool recurse, bool recursing, List 
*visited);
 static void ATAddForeignKeyConstraint(AlteredTableInfo *tab, Relation rel,
                                                  Constraint *fkconstraint);
 static void ATExecDropConstraint(Relation rel, const char *constrName,
@@ -4584,7 +4595,7 @@ ATExecAddConstraint(List **wqueue, AlteredTableInfo *tab, 
Relation rel,
        {
                case CONSTR_CHECK:
                        ATAddCheckConstraint(wqueue, tab, rel,
-                                                                newConstraint, 
recurse, false);
+                                                                newConstraint, 
recurse, false, NIL);
                        break;
 
                case CONSTR_FOREIGN:
@@ -4639,7 +4650,7 @@ ATExecAddConstraint(List **wqueue, AlteredTableInfo *tab, 
Relation rel,
  */
 static void
 ATAddCheckConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
-                                        Constraint *constr, bool recurse, bool 
recursing)
+                                        Constraint *constr, bool recurse, bool 
recursing, List *visited)
 {
        List       *newcons;
        ListCell   *lcon;
@@ -4711,6 +4722,30 @@ ATAddCheckConstraint(List **wqueue, AlteredTableInfo 
*tab, Relation rel,
                Oid                     childrelid = lfirst_oid(child);
                Relation        childrel;
                AlteredTableInfo *childtab;
+               ListCell   *lv;
+               bool        skip = false;
+               ParentVisit *visit;
+
+               /* Skip children that were visited from the same direct parent. 
*/
+               foreach(lv, visited)
+               {
+                       visit = lfirst(lv);
+
+                       if ((visit->parent == RelationGetRelid(rel)) &&
+                               (visit->child == childrelid))
+                       {
+                               skip = true;
+                               break;
+                       }
+               }
+
+               if (skip)
+                       continue;
+
+               visit = (ParentVisit *) palloc(sizeof(ParentVisit));
+               visit->parent = RelationGetRelid(rel);
+               visit->child = childrelid;
+               visited = lappend(visited, visit);
 
                /* find_inheritance_children already got lock */
                childrel = heap_open(childrelid, NoLock);
@@ -4721,7 +4756,7 @@ ATAddCheckConstraint(List **wqueue, AlteredTableInfo 
*tab, Relation rel,
 
                /* Recurse to child */
                ATAddCheckConstraint(wqueue, childtab, childrel,
-                                                        constr, recurse, true);
+                                                        constr, recurse, true, 
visited);
 
                heap_close(childrel, NoLock);
        }

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to