diff --git a/src/backend/catalog/pg_inherits.c b/src/backend/catalog/pg_inherits.c index 402ce4689e..bbba5ae5cb 100644 --- a/src/backend/catalog/pg_inherits.c +++ b/src/backend/catalog/pg_inherits.c @@ -13,7 +13,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/catalog/pg_inherits.c,v 1.5 2010/01/02 16:57:36 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/catalog/pg_inherits.c,v 1.6 2010/02/01 19:28:56 rhaas Exp $ * *------------------------------------------------------------------------- */ @@ -148,6 +148,9 @@ find_inheritance_children(Oid parentrelId, LOCKMODE lockmode) * find_all_inheritors - * Returns a list of relation OIDs including the given rel plus * all relations that inherit from it, directly or indirectly. + * Optionally, it also returns the number of parents found for + * each such relation within the inheritance tree rooted at the + * given rel. * * The specified lock type is acquired on all child relations (but not on the * given rel; caller should already have locked it). If lockmode is NoLock @@ -155,9 +158,9 @@ find_inheritance_children(Oid parentrelId, LOCKMODE lockmode) * against possible DROPs of child relations. */ List * -find_all_inheritors(Oid parentrelId, LOCKMODE lockmode) +find_all_inheritors(Oid parentrelId, LOCKMODE lockmode, List **numparents) { - List *rels_list; + List *rels_list, *rel_numparents; ListCell *l; /* @@ -168,11 +171,13 @@ find_all_inheritors(Oid parentrelId, LOCKMODE lockmode) * doesn't fetch the next list element until the bottom of the loop. */ rels_list = list_make1_oid(parentrelId); + rel_numparents = list_make1_int(0); foreach(l, rels_list) { Oid currentrel = lfirst_oid(l); List *currentchildren; + ListCell *lc; /* Get the direct children of this rel */ currentchildren = find_inheritance_children(currentrel, lockmode); @@ -184,9 +189,37 @@ find_all_inheritors(Oid parentrelId, LOCKMODE lockmode) * loop, though theoretically there can't be any cycles in the * inheritance graph anyway.) */ - rels_list = list_concat_unique_oid(rels_list, currentchildren); + foreach(lc, currentchildren) + { + Oid child_oid = lfirst_oid(lc); + bool found = false; + ListCell *lo; + ListCell *li; + + /* if the rel is already there, bump number-of-parents counter */ + forboth(lo, rels_list, li, rel_numparents) + { + if (lfirst_oid(lo) == child_oid) + { + lfirst_int(li)++; + found = true; + break; + } + } + + /* if it's not there, add it. expect 1 parent, initially. */ + if (!found) + { + rels_list = lappend_oid(rels_list, child_oid); + rel_numparents = lappend_int(rel_numparents, 1); + } + } } + if (numparents) + *numparents = rel_numparents; + else + list_free(rel_numparents); return rels_list; } diff --git a/src/backend/commands/alter.c b/src/backend/commands/alter.c index 8e0fbaa11f..233ac1bc3d 100644 --- a/src/backend/commands/alter.c +++ b/src/backend/commands/alter.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/alter.c,v 1.33 2010/01/02 16:57:36 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/commands/alter.c,v 1.34 2010/02/01 19:28:56 rhaas Exp $ * *------------------------------------------------------------------------- */ @@ -126,7 +126,7 @@ ExecRenameStmt(RenameStmt *stmt) stmt->subname, /* old att name */ stmt->newname, /* new att name */ interpretInhOption(stmt->relation->inhOpt), /* recursive? */ - false); /* recursing already? */ + 0); /* expected inhcount */ break; case OBJECT_TRIGGER: renametrig(relid, diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c index f27a6dad2e..067d375a84 100644 --- a/src/backend/commands/analyze.c +++ b/src/backend/commands/analyze.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/analyze.c,v 1.148 2010/01/22 16:40:18 rhaas Exp $ + * $PostgreSQL: pgsql/src/backend/commands/analyze.c,v 1.149 2010/02/01 19:28:56 rhaas Exp $ * *------------------------------------------------------------------------- */ @@ -1390,7 +1390,8 @@ acquire_inherited_sample_rows(Relation onerel, HeapTuple *rows, int targrows, * Find all members of inheritance set. We only need AccessShareLock on * the children. */ - tableOIDs = find_all_inheritors(RelationGetRelid(onerel), AccessShareLock); + tableOIDs = + find_all_inheritors(RelationGetRelid(onerel), AccessShareLock, NULL); /* * Check that there's at least one descendant, else fail. This could diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 134b217d82..0e0b0de9f9 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.320 2010/01/28 23:21:11 petere Exp $ + * $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.321 2010/02/01 19:28:56 rhaas Exp $ * *------------------------------------------------------------------------- */ @@ -824,7 +824,7 @@ ExecuteTruncate(TruncateStmt *stmt) ListCell *child; List *children; - children = find_all_inheritors(myrelid, AccessExclusiveLock); + children = find_all_inheritors(myrelid, AccessExclusiveLock, NULL); foreach(child, children) { @@ -1943,7 +1943,7 @@ renameatt(Oid myrelid, const char *oldattname, const char *newattname, bool recurse, - bool recursing) + int expected_parents) { Relation targetrelation; Relation attrelation; @@ -1987,24 +1987,31 @@ renameatt(Oid myrelid, */ if (recurse) { - ListCell *child; - List *children; + List *child_oids, *child_numparents; + ListCell *lo, *li; - children = find_all_inheritors(myrelid, AccessExclusiveLock); + /* + * we need the number of parents for each child so that the recursive + * calls to renameatt() can determine whether there are any parents + * outside the inheritance hierarchy being processed. + */ + child_oids = find_all_inheritors(myrelid, AccessExclusiveLock, + &child_numparents); /* * find_all_inheritors does the recursive search of the inheritance * hierarchy, so all we have to do is process all of the relids in the * list that it returns. */ - foreach(child, children) + forboth(lo, child_oids, li, child_numparents) { - Oid childrelid = lfirst_oid(child); + Oid childrelid = lfirst_oid(lo); + int numparents = lfirst_int(li); if (childrelid == myrelid) continue; /* note we need not recurse again */ - renameatt(childrelid, oldattname, newattname, false, true); + renameatt(childrelid, oldattname, newattname, false, numparents); } } else @@ -2012,8 +2019,10 @@ renameatt(Oid myrelid, /* * If we are told not to recurse, there had better not be any child * tables; else the rename would put them out of step. + * + * expected_parents will only be 0 if we are not already recursing. */ - if (!recursing && + if (expected_parents == 0 && find_inheritance_children(myrelid, NoLock) != NIL) ereport(ERROR, (errcode(ERRCODE_INVALID_TABLE_DEFINITION), @@ -2039,10 +2048,15 @@ renameatt(Oid myrelid, oldattname))); /* - * if the attribute is inherited, forbid the renaming, unless we are - * already inside a recursive rename. + * if the attribute is inherited, forbid the renaming. if this is a + * top-level call to renameatt(), then expected_parents will be 0, so the + * effect of this code will be to prohibit the renaming if the attribute + * is inherited at all. if this is a recursive call to renameatt(), + * expected_parents will be the number of parents the current relation has + * within the inheritance hierarchy being processed, so we'll prohibit + * the renaming only if there are additional parents from elsewhere. */ - if (attform->attinhcount > 0 && !recursing) + if (attform->attinhcount > expected_parents) ereport(ERROR, (errcode(ERRCODE_INVALID_TABLE_DEFINITION), errmsg("cannot rename inherited column \"%s\"", @@ -3410,7 +3424,7 @@ ATSimpleRecursion(List **wqueue, Relation rel, ListCell *child; List *children; - children = find_all_inheritors(relid, AccessExclusiveLock); + children = find_all_inheritors(relid, AccessExclusiveLock, NULL); /* * find_all_inheritors does the recursive search of the inheritance @@ -7233,7 +7247,7 @@ ATExecAddInherit(Relation child_rel, RangeVar *parent) * We use weakest lock we can on child's children, namely AccessShareLock. */ children = find_all_inheritors(RelationGetRelid(child_rel), - AccessShareLock); + AccessShareLock, NULL); if (list_member_oid(children, RelationGetRelid(parent_rel))) ereport(ERROR, diff --git a/src/backend/optimizer/prep/prepunion.c b/src/backend/optimizer/prep/prepunion.c index 4eb2859e09..560afaaa24 100644 --- a/src/backend/optimizer/prep/prepunion.c +++ b/src/backend/optimizer/prep/prepunion.c @@ -22,7 +22,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/optimizer/prep/prepunion.c,v 1.179 2010/01/02 16:57:47 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/optimizer/prep/prepunion.c,v 1.180 2010/02/01 19:28:56 rhaas Exp $ * *------------------------------------------------------------------------- */ @@ -1180,7 +1180,7 @@ expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti) lockmode = AccessShareLock; /* Scan for all members of inheritance set, acquire needed locks */ - inhOIDs = find_all_inheritors(parentOID, lockmode); + inhOIDs = find_all_inheritors(parentOID, lockmode, NULL); /* * Check that there's at least one descendant, else treat as no-child diff --git a/src/include/catalog/pg_inherits_fn.h b/src/include/catalog/pg_inherits_fn.h index 20dc534088..e22c497af5 100644 --- a/src/include/catalog/pg_inherits_fn.h +++ b/src/include/catalog/pg_inherits_fn.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2010, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/catalog/pg_inherits_fn.h,v 1.2 2010/01/02 16:58:01 momjian Exp $ + * $PostgreSQL: pgsql/src/include/catalog/pg_inherits_fn.h,v 1.3 2010/02/01 19:28:56 rhaas Exp $ * *------------------------------------------------------------------------- */ @@ -18,7 +18,8 @@ #include "storage/lock.h" extern List *find_inheritance_children(Oid parentrelId, LOCKMODE lockmode); -extern List *find_all_inheritors(Oid parentrelId, LOCKMODE lockmode); +extern List *find_all_inheritors(Oid parentrelId, LOCKMODE lockmode, + List **parents); extern bool has_subclass(Oid relationId); extern bool typeInheritsFrom(Oid subclassTypeId, Oid superclassTypeId); diff --git a/src/include/commands/tablecmds.h b/src/include/commands/tablecmds.h index 13fcf9cbba..45d1835548 100644 --- a/src/include/commands/tablecmds.h +++ b/src/include/commands/tablecmds.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2010, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/commands/tablecmds.h,v 1.45 2010/01/02 16:58:03 momjian Exp $ + * $PostgreSQL: pgsql/src/include/commands/tablecmds.h,v 1.46 2010/02/01 19:28:56 rhaas Exp $ * *------------------------------------------------------------------------- */ @@ -43,7 +43,7 @@ extern void renameatt(Oid myrelid, const char *oldattname, const char *newattname, bool recurse, - bool recursing); + int expected_parents); extern void RenameRelation(Oid myrelid, const char *newrelname, diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out index 9c83a32f93..98c8a79a7e 100644 --- a/src/test/regress/expected/inherit.out +++ b/src/test/regress/expected/inherit.out @@ -1053,3 +1053,97 @@ NOTICE: merging column "a" with inherited definition ERROR: column "a" has a storage parameter conflict DETAIL: MAIN versus EXTENDED DROP TABLE t1, t2, t3, t4, t12_storage, t12_comments, t1_inh, t13_inh, t13_like, t_all; +-- Test for renaming in simple multiple inheritance +CREATE TABLE t1 (a int, b int); +CREATE TABLE s1 (b int, c int); +CREATE TABLE ts (d int) INHERITS (t1, s1); +NOTICE: merging multiple inherited definitions of column "b" +ALTER TABLE t1 RENAME a TO aa; +ALTER TABLE t1 RENAME b TO bb; -- to be failed +ERROR: cannot rename inherited column "b" +ALTER TABLE ts RENAME aa TO aaa; -- to be failed +ERROR: cannot rename inherited column "aa" +ALTER TABLE ts RENAME d TO dd; +\d+ ts + Table "public.ts" + Column | Type | Modifiers | Storage | Description +--------+---------+-----------+---------+------------- + aa | integer | | plain | + b | integer | | plain | + c | integer | | plain | + dd | integer | | plain | +Inherits: t1, + s1 +Has OIDs: no + +DROP TABLE ts; +-- Test for renaming in diamond inheritance +CREATE TABLE t2 (x int) INHERITS (t1); +CREATE TABLE t3 (y int) INHERITS (t1); +CREATE TABLE t4 (z int) INHERITS (t2, t3); +NOTICE: merging multiple inherited definitions of column "aa" +NOTICE: merging multiple inherited definitions of column "b" +ALTER TABLE t1 RENAME aa TO aaa; +\d+ t4 + Table "public.t4" + Column | Type | Modifiers | Storage | Description +--------+---------+-----------+---------+------------- + aaa | integer | | plain | + b | integer | | plain | + x | integer | | plain | + y | integer | | plain | + z | integer | | plain | +Inherits: t2, + t3 +Has OIDs: no + +CREATE TABLE ts (d int) INHERITS (t2, s1); +NOTICE: merging multiple inherited definitions of column "b" +ALTER TABLE t1 RENAME aaa TO aaaa; +ALTER TABLE t1 RENAME b TO bb; -- to be failed +ERROR: cannot rename inherited column "b" +\d+ ts + Table "public.ts" + Column | Type | Modifiers | Storage | Description +--------+---------+-----------+---------+------------- + aaaa | integer | | plain | + b | integer | | plain | + x | integer | | plain | + c | integer | | plain | + d | integer | | plain | +Inherits: t2, + s1 +Has OIDs: no + +WITH RECURSIVE r AS ( + SELECT 't1'::regclass AS inhrelid +UNION ALL + SELECT c.inhrelid FROM pg_inherits c, r WHERE r.inhrelid = c.inhparent +) +SELECT a.attrelid::regclass, a.attname, a.attinhcount, e.expected + FROM (SELECT inhrelid, count(*) AS expected FROM pg_inherits + WHERE inhparent IN (SELECT inhrelid FROM r) GROUP BY inhrelid) e + JOIN pg_attribute a ON e.inhrelid = a.attrelid WHERE NOT attislocal + ORDER BY a.attrelid::regclass::text, a.attnum; + attrelid | attname | attinhcount | expected +----------+---------+-------------+---------- + t2 | aaaa | 1 | 1 + t2 | b | 1 | 1 + t3 | aaaa | 1 | 1 + t3 | b | 1 | 1 + t4 | aaaa | 2 | 2 + t4 | b | 2 | 2 + t4 | x | 1 | 2 + t4 | y | 1 | 2 + ts | aaaa | 1 | 1 + ts | b | 2 | 1 + ts | x | 1 | 1 + ts | c | 1 | 1 +(12 rows) + +DROP TABLE t1, s1 CASCADE; +NOTICE: drop cascades to 4 other objects +DETAIL: drop cascades to table t2 +drop cascades to table ts +drop cascades to table t3 +drop cascades to table t4 diff --git a/src/test/regress/sql/inherit.sql b/src/test/regress/sql/inherit.sql index 192e8604d6..192b3e7334 100644 --- a/src/test/regress/sql/inherit.sql +++ b/src/test/regress/sql/inherit.sql @@ -334,3 +334,42 @@ CREATE TABLE inh_error1 () INHERITS (t1, t4); CREATE TABLE inh_error2 (LIKE t4 INCLUDING STORAGE) INHERITS (t1); DROP TABLE t1, t2, t3, t4, t12_storage, t12_comments, t1_inh, t13_inh, t13_like, t_all; + +-- Test for renaming in simple multiple inheritance +CREATE TABLE t1 (a int, b int); +CREATE TABLE s1 (b int, c int); +CREATE TABLE ts (d int) INHERITS (t1, s1); + +ALTER TABLE t1 RENAME a TO aa; +ALTER TABLE t1 RENAME b TO bb; -- to be failed +ALTER TABLE ts RENAME aa TO aaa; -- to be failed +ALTER TABLE ts RENAME d TO dd; +\d+ ts + +DROP TABLE ts; + +-- Test for renaming in diamond inheritance +CREATE TABLE t2 (x int) INHERITS (t1); +CREATE TABLE t3 (y int) INHERITS (t1); +CREATE TABLE t4 (z int) INHERITS (t2, t3); + +ALTER TABLE t1 RENAME aa TO aaa; +\d+ t4 + +CREATE TABLE ts (d int) INHERITS (t2, s1); +ALTER TABLE t1 RENAME aaa TO aaaa; +ALTER TABLE t1 RENAME b TO bb; -- to be failed +\d+ ts + +WITH RECURSIVE r AS ( + SELECT 't1'::regclass AS inhrelid +UNION ALL + SELECT c.inhrelid FROM pg_inherits c, r WHERE r.inhrelid = c.inhparent +) +SELECT a.attrelid::regclass, a.attname, a.attinhcount, e.expected + FROM (SELECT inhrelid, count(*) AS expected FROM pg_inherits + WHERE inhparent IN (SELECT inhrelid FROM r) GROUP BY inhrelid) e + JOIN pg_attribute a ON e.inhrelid = a.attrelid WHERE NOT attislocal + ORDER BY a.attrelid::regclass::text, a.attnum; + +DROP TABLE t1, s1 CASCADE;