pgsql-fix-inherit-rename.3.patch

Index: base/src/test/regress/sql/inherit.sql
===================================================================
*** base/src/test/regress/sql/inherit.sql	(revision 2538)
--- base/src/test/regress/sql/inherit.sql	(working copy)
*************** CREATE TABLE inh_error1 () INHERITS (t1,
*** 334,336 ****
--- 334,348 ----
  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
+ CREATE TABLE t1 (a int, b int);
+ CREATE TABLE t2 (b int, c int);
+ CREATE TABLE t3 (d int) INHERITS(t1, t2);
+ ALTER TABLE t1 RENAME a TO x;
+ ALTER TABLE t1 RENAME b TO y;		-- to be failed
+ ALTER TABLE t3 RENAME d TO z;
+ SELECT * FROM t3;
+ DROP TABLE t3;
+ DROP TABLE t2;
+ DROP TABLE t1;
Index: base/src/test/regress/expected/inherit.out
===================================================================
*** base/src/test/regress/expected/inherit.out	(revision 2538)
--- base/src/test/regress/expected/inherit.out	(working copy)
*************** NOTICE:  merging column "a" with inherit
*** 1053,1055 ****
--- 1053,1072 ----
  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
+ CREATE TABLE t1 (a int, b int);
+ CREATE TABLE t2 (b int, c int);
+ CREATE TABLE t3 (d int) INHERITS(t1, t2);
+ NOTICE:  merging multiple inherited definitions of column "b"
+ ALTER TABLE t1 RENAME a TO x;
+ ALTER TABLE t1 RENAME b TO y;		-- to be failed
+ ERROR:  cannot rename multiple inherited column "b"
+ ALTER TABLE t3 RENAME d TO z;
+ SELECT * FROM t3;
+  x | b | c | z 
+ ---+---+---+---
+ (0 rows)
+ 
+ DROP TABLE t3;
+ DROP TABLE t2;
+ DROP TABLE t1;
Index: base/src/include/commands/tablecmds.h
===================================================================
*** base/src/include/commands/tablecmds.h	(revision 2538)
--- base/src/include/commands/tablecmds.h	(working copy)
*************** extern void ExecuteTruncate(TruncateStmt
*** 42,47 ****
--- 42,48 ----
  extern void renameatt(Oid myrelid,
  		  const char *oldattname,
  		  const char *newattname,
+ 		  int expected_inhcount,
  		  bool recurse,
  		  bool recursing);
  
Index: base/src/include/catalog/pg_inherits_fn.h
===================================================================
*** base/src/include/catalog/pg_inherits_fn.h	(revision 2538)
--- base/src/include/catalog/pg_inherits_fn.h	(working copy)
***************
*** 19,24 ****
--- 19,28 ----
  
  extern List *find_inheritance_children(Oid parentrelId, LOCKMODE lockmode);
  extern List *find_all_inheritors(Oid parentrelId, LOCKMODE lockmode);
+ extern void find_all_inheritors_with_inhcount(Oid parentrelId,
+ 											  LOCKMODE lockmode,
+ 											  List **child_oids,
+ 											  List **child_inhs);
  extern bool has_subclass(Oid relationId);
  extern bool typeInheritsFrom(Oid subclassTypeId, Oid superclassTypeId);
  
Index: base/src/backend/commands/tablecmds.c
===================================================================
*** base/src/backend/commands/tablecmds.c	(revision 2538)
--- base/src/backend/commands/tablecmds.c	(working copy)
*************** void
*** 1908,1913 ****
--- 1908,1914 ----
  renameatt(Oid myrelid,
  		  const char *oldattname,
  		  const char *newattname,
+ 		  int expected_inhcount,
  		  bool recurse,
  		  bool recursing)
  {
*************** renameatt(Oid myrelid,
*** 1948,1971 ****
  	 */
  	if (recurse)
  	{
! 		ListCell   *child;
! 		List	   *children;
! 
! 		children = find_all_inheritors(myrelid, AccessExclusiveLock);
  
  		/*
! 		 * 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)
  		{
! 			Oid			childrelid = lfirst_oid(child);
  
! 			if (childrelid == myrelid)
  				continue;
  			/* note we need not recurse again */
! 			renameatt(childrelid, oldattname, newattname, false, true);
  		}
  	}
  	else
--- 1949,1998 ----
  	 */
  	if (recurse)
  	{
! 		List	   *child_oids, *child_inhs;
! 		ListCell   *lo, *li;
  
  		/*
! 		 * If we cound find an expected inhcount larger than 1,
! 		 * t means the relation has diamond inheritance tree.
! 		 *
! 		 * If an attribute to be renamed is inherited from multiple
! 		 * parent relations independently defined then merged, it
! 		 * is problematic to rename the attribute, because anohter
! 		 * inheritance tree assumes the child relation still has the
! 		 * inherited attributes, but it will be incorrect.
! 		 * So, we have to forbide to rename an attribute which is
! 		 * inherited from multiple parent relations.
! 		 *
! 		 * However, we have an exception case when the attribute is
! 		 * inherited from multiple parent relations, but these ones
! 		 * eventually have same origion.
! 		 * For example, when TBL_A is inherited by TBL_X and TBL_Y,
! 		 * these tables are inherited by TBL_XY, the TBL_XY indeed
! 		 * has multiple parent relations and its attributes was
! 		 * merged. But, these have same origin, and we have no reason
! 		 * to prevent renaming them.
! 		 *
! 		 * So, we check whether the child relation has diamond
! 		 * inheritance, or not, here. If pg_attribute.attinhcount is
! 		 * larger than expected inhcount, it means the attribute was
! 		 * merged with any other attributes come from other parent
! 		 * relations.
  		 */
! 		find_all_inheritors_with_inhcount(myrelid, AccessExclusiveLock,
! 										  &child_oids, &child_inhs);
! 
! 		forboth (lo, child_oids, li, child_inhs)
  		{
! 			Oid		child_relid = lfirst_oid(lo);
! 			int		child_inhcount = lfirst_int(li);
  
! 			if (child_relid == myrelid)
  				continue;
+ 
  			/* note we need not recurse again */
! 			renameatt(child_relid, oldattname, newattname,
! 					  child_inhcount, false, true);
  		}
  	}
  	else
*************** renameatt(Oid myrelid,
*** 2009,2014 ****
--- 2036,2052 ----
  				 errmsg("cannot rename inherited column \"%s\"",
  						oldattname)));
  
+ 	/*
+ 	 * if the attribute is multiple inherited, forbid the renaming,
+ 	 * even if we are already inside a recursive rename, because we
+ 	 * have no reasonable way to keep its integrity.
+ 	 */
+ 	if (attform->attinhcount > expected_inhcount)
+ 		ereport(ERROR,
+ 				(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
+ 				 (errmsg("cannot rename multiple inherited column \"%s\"",
+ 						 oldattname))));
+ 
  	/* new name should not already exist */
  
  	/* this test is deliberately not attisdropped-aware */
Index: base/src/backend/commands/alter.c
===================================================================
*** base/src/backend/commands/alter.c	(revision 2538)
--- base/src/backend/commands/alter.c	(working copy)
*************** ExecRenameStmt(RenameStmt *stmt)
*** 125,130 ****
--- 125,131 ----
  						renameatt(relid,
  								  stmt->subname,		/* old att name */
  								  stmt->newname,		/* new att name */
+ 								  1,			/* expected inhcount */
  								  interpretInhOption(stmt->relation->inhOpt),	/* recursive? */
  								  false);		/* recursing already? */
  						break;
Index: base/src/backend/catalog/pg_inherits.c
===================================================================
*** base/src/backend/catalog/pg_inherits.c	(revision 2538)
--- base/src/backend/catalog/pg_inherits.c	(working copy)
*************** find_all_inheritors(Oid parentrelId, LOC
*** 190,195 ****
--- 190,256 ----
  	return rels_list;
  }
  
+ /*
+  * find_all_inheritors_with_inhcount
+  *
+  * It is same as find_all_inheritors, except for it returns an excepted
+  * inhcount for each children.
+  */
+ void
+ find_all_inheritors_with_inhcount(Oid parentrelId, LOCKMODE lockmode,
+ 								  List **child_oids, List **child_inhs)
+ {
+ 	List	   *children, *rels_oids, *rels_inhs;
+ 	ListCell   *lc, *lo, *li;
+ 
+ 	/*
+ 	 * The root of the inheritance tree always has 1 as an expected
+ 	 * inhcount.
+ 	 */
+ 	rels_oids = list_make1_oid(parentrelId);
+ 	rels_inhs = list_make1_int(1);
+ 
+ 	children = find_inheritance_children(parentrelId, lockmode);
+ 	foreach (lc, children)
+ 	{
+ 		Oid		cur_relid = lfirst_oid(lc);
+ 		List   *cur_children;
+ 		bool	found = false;
+ 
+ 		forboth (lo, rels_oids, li, rels_inhs)
+ 		{
+ 			/*
+ 			 * If we found a duplicated relation within a certain inheritance
+ 			 * tree, it means a diamond-inheritance. So, we increment its
+ 			 * expected inhcount without appending an element to the list.
+ 			 */
+ 			if (lfirst_oid(lo) == cur_relid)
+ 			{
+ 				lfirst_int(li)++;
+ 				found = true;
+ 				break;
+ 			}
+ 		}
+ 
+ 		if (!found)
+ 		{
+ 			rels_oids = lappend_oid(rels_oids, cur_relid);
+ 			rels_inhs = lappend_int(rels_inhs, 1);
+ 		}
+ 
+ 		/*
+ 		 * Unlike find_all_inheritors(), we need to walk on child relations
+ 		 * that have diamond inheritance tree, because this function has to
+ 		 * return correct expected inhecount to the caller.
+ 		 */
+ 		cur_children = find_inheritance_children(cur_relid, lockmode);
+ 
+ 		children = list_concat(children, cur_children);
+ 	}
+ 
+ 	*child_oids = rels_oids;
+ 	*child_inhs = rels_inhs;
+ }
  
  /*
   * has_subclass - does this relation have any children?