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