Make renamerel take an OID, not a RangeVar, to identify the relation

to rename.  Avoids some corner-case bugs in cluster.c, improves
consistency with renameatt.

Author: tglsfdc

src/backend/commands/cluster.c

@@ -15,7 +15,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/commands/cluster.c,v 1.76 2002/03/31 06:26:30 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/commands/cluster.c,v 1.77 2002/03/31 07:49:30 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -37,8 +37,9 @@
 #include "utils/syscache.h"
 
 
-static Oid	copy_heap(Oid OIDOldHeap, char *NewName);
-static void copy_index(Oid OIDOldIndex, Oid OIDNewHeap, char *NewIndexName);
+static Oid	copy_heap(Oid OIDOldHeap, const char *NewName);
+static Oid	copy_index(Oid OIDOldIndex, Oid OIDNewHeap,
+					   const char *NewIndexName);
 static void rebuildheap(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex);
 
 /*
@@ -58,13 +59,12 @@ cluster(RangeVar *oldrelation, char *oldindexname)
 {
 	Oid			OIDOldHeap,
 				OIDOldIndex,
-				OIDNewHeap;
+				OIDNewHeap,
+				OIDNewIndex;
 	Relation	OldHeap,
 				OldIndex;
 	char		NewHeapName[NAMEDATALEN];
 	char		NewIndexName[NAMEDATALEN];
-	RangeVar   *NewHeap;
-	RangeVar   *NewIndex;
 
 	/*
 	 * We grab exclusive access to the target rel and index for the
@@ -115,7 +115,7 @@ cluster(RangeVar *oldrelation, char *oldindexname)
 	/* Create new index over the tuples of the new heap. */
 	snprintf(NewIndexName, NAMEDATALEN, "temp_%u", OIDOldIndex);
 
-	copy_index(OIDOldIndex, OIDNewHeap, NewIndexName);
+	OIDNewIndex = copy_index(OIDOldIndex, OIDNewHeap, NewIndexName);
 
 	CommandCounterIncrement();
 
@@ -124,23 +124,16 @@ cluster(RangeVar *oldrelation, char *oldindexname)
 
 	CommandCounterIncrement();
 
-	/* XXX ugly, and possibly wrong in the presence of schemas... */
-	/* would be better to pass OIDs to renamerel. */
-	NewHeap = copyObject(oldrelation);
-	NewHeap->relname = NewHeapName;
-	NewIndex = copyObject(oldrelation);
-	NewIndex->relname = NewIndexName;
-	
-	renamerel(NewHeap, oldrelation->relname);
+	renamerel(OIDNewHeap, oldrelation->relname);
 
 	/* This one might be unnecessary, but let's be safe. */
 	CommandCounterIncrement();
 
-	renamerel(NewIndex, oldindexname);
+	renamerel(OIDNewIndex, oldindexname);
 }
 
 static Oid
-copy_heap(Oid OIDOldHeap, char *NewName)
+copy_heap(Oid OIDOldHeap, const char *NewName)
 {
 	TupleDesc	OldHeapDesc,
 				tupdesc;
@@ -181,9 +174,10 @@ copy_heap(Oid OIDOldHeap, char *NewName)
 	return OIDNewHeap;
 }
 
-static void
-copy_index(Oid OIDOldIndex, Oid OIDNewHeap, char *NewIndexName)
+static Oid
+copy_index(Oid OIDOldIndex, Oid OIDNewHeap, const char *NewIndexName)
 {
+	Oid			OIDNewIndex;
 	Relation	OldIndex,
 				NewHeap;
 	IndexInfo  *indexInfo;
@@ -198,19 +192,21 @@ copy_index(Oid OIDOldIndex, Oid OIDNewHeap, char *NewIndexName)
 	 */
 	indexInfo = BuildIndexInfo(OldIndex->rd_index);
 
-	index_create(OIDNewHeap,
-				 NewIndexName,
-				 indexInfo,
-				 OldIndex->rd_rel->relam,
-				 OldIndex->rd_index->indclass,
-				 OldIndex->rd_index->indisprimary,
-				 allowSystemTableMods);
+	OIDNewIndex = index_create(OIDNewHeap,
+							   NewIndexName,
+							   indexInfo,
+							   OldIndex->rd_rel->relam,
+							   OldIndex->rd_index->indclass,
+							   OldIndex->rd_index->indisprimary,
+							   allowSystemTableMods);
 
 	setRelhasindex(OIDNewHeap, true,
 				   OldIndex->rd_index->indisprimary, InvalidOid);
 
 	index_close(OldIndex);
 	heap_close(NewHeap, NoLock);
+
+	return OIDNewIndex;
 }
 
 

src/backend/commands/rename.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/commands/Attic/rename.c,v 1.67 2002/03/31 06:26:30 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/commands/Attic/rename.c,v 1.68 2002/03/31 07:49:30 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -69,8 +69,8 @@ static void update_ri_trigger_args(Oid relid,
  */
 void
 renameatt(Oid relid,
-		  char *oldattname,
-		  char *newattname,
+		  const char *oldattname,
+		  const char *newattname,
 		  bool recurse)
 {
 	Relation	targetrelation;
@@ -250,51 +250,36 @@ renameatt(Oid relid,
  *		renamerel		- change the name of a relation
  */
 void
-renamerel(const RangeVar *relation, const char *newrelname)
+renamerel(Oid relid, const char *newrelname)
 {
 	Relation	targetrelation;
 	Relation	relrelation;	/* for RELATION relation */
 	HeapTuple	reltup;
 	Oid			namespaceId;
-	Oid			reloid;
 	char		relkind;
 	bool		relhastriggers;
 	Relation	irelations[Num_pg_class_indices];
 
-	if (!allowSystemTableMods && IsSystemRelationName(relation->relname))
-		elog(ERROR, "renamerel: system relation \"%s\" may not be renamed",
-			 relation->relname);
-
-	if (!allowSystemTableMods && IsSystemRelationName(newrelname))
-		elog(ERROR, "renamerel: Illegal class name: \"%s\" -- pg_ is reserved for system catalogs",
-			 newrelname);
-
 	/*
 	 * Grab an exclusive lock on the target table or index, which we will
 	 * NOT release until end of transaction.
 	 */
-	targetrelation = relation_openrv(relation, AccessExclusiveLock);
+	targetrelation = relation_open(relid, AccessExclusiveLock);
 
 	namespaceId = RelationGetNamespace(targetrelation);
-	reloid = RelationGetRelid(targetrelation);
-	relkind = targetrelation->rd_rel->relkind;
-	relhastriggers = (targetrelation->rd_rel->reltriggers > 0);
 
-	/*
-	 * Close rel, but keep exclusive lock!
-	 */
-	relation_close(targetrelation, NoLock);
+	/* Validity checks */
+	if (!allowSystemTableMods &&
+		IsSystemRelationName(RelationGetRelationName(targetrelation)))
+		elog(ERROR, "renamerel: system relation \"%s\" may not be renamed",
+			 RelationGetRelationName(targetrelation));
 
-	/*
-	 * Flush the relcache entry (easier than trying to change it at
-	 * exactly the right instant).	It'll get rebuilt on next access to
-	 * relation.
-	 *
-	 * XXX What if relation is myxactonly?
-	 *
-	 * XXX this is probably not necessary anymore?
-	 */
-	RelationIdInvalidateRelationCacheByRelationId(reloid);
+	if (!allowSystemTableMods && IsSystemRelationName(newrelname))
+		elog(ERROR, "renamerel: Illegal class name: \"%s\" -- pg_ is reserved for system catalogs",
+			 newrelname);
+
+	relkind = targetrelation->rd_rel->relkind;
+	relhastriggers = (targetrelation->rd_rel->reltriggers > 0);
 
 	/*
 	 * Find relation's pg_class tuple, and make sure newrelname isn't in
@@ -303,11 +288,11 @@ renamerel(const RangeVar *relation, const char *newrelname)
 	relrelation = heap_openr(RelationRelationName, RowExclusiveLock);
 
 	reltup = SearchSysCacheCopy(RELOID,
-								PointerGetDatum(reloid),
+								PointerGetDatum(relid),
 								0, 0, 0);
 	if (!HeapTupleIsValid(reltup))
 		elog(ERROR, "renamerel: relation \"%s\" does not exist",
-			 relation->relname);
+			 RelationGetRelationName(targetrelation));
 
 	if (get_relname_relid(newrelname, namespaceId) != InvalidOid)
 		elog(ERROR, "renamerel: relation \"%s\" exists", newrelname);
@@ -332,7 +317,8 @@ renamerel(const RangeVar *relation, const char *newrelname)
 	 * Also rename the associated type, if any.
 	 */
 	if (relkind != RELKIND_INDEX)
-		TypeRename(relation->relname, namespaceId, newrelname);
+		TypeRename(RelationGetRelationName(targetrelation), namespaceId,
+				   newrelname);
 
 	/*
 	 * If it's a view, must also rename the associated ON SELECT rule.
@@ -342,7 +328,7 @@ renamerel(const RangeVar *relation, const char *newrelname)
 		char	   *oldrulename,
 				   *newrulename;
 
-		oldrulename = MakeRetrieveViewRuleName(relation->relname);
+		oldrulename = MakeRetrieveViewRuleName(RelationGetRelationName(targetrelation));
 		newrulename = MakeRetrieveViewRuleName(newrelname);
 		RenameRewriteRule(oldrulename, newrulename);
 	}
@@ -353,14 +339,21 @@ renamerel(const RangeVar *relation, const char *newrelname)
 	if (relhastriggers)
 	{
 		/* update tgargs where relname is primary key */
-		update_ri_trigger_args(reloid,
-							   relation->relname, newrelname,
+		update_ri_trigger_args(relid,
+							   RelationGetRelationName(targetrelation),
+							   newrelname,
 							   false, true);
 		/* update tgargs where relname is foreign key */
-		update_ri_trigger_args(reloid,
-							   relation->relname, newrelname,
+		update_ri_trigger_args(relid,
+							   RelationGetRelationName(targetrelation),
+							   newrelname,
 							   true, true);
 	}
+
+	/*
+	 * Close rel, but keep exclusive lock!
+	 */
+	relation_close(targetrelation, NoLock);
 }
 
 /*

src/backend/tcop/utility.c

@@ -10,7 +10,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/tcop/utility.c,v 1.143 2002/03/31 06:26:31 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/tcop/utility.c,v 1.144 2002/03/31 07:49:30 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -385,32 +385,20 @@ ProcessUtility(Node *parsetree,
 
 				CheckOwnership(stmt->relation, true);
 
-				/* ----------------
-				 *	XXX using len == 3 to tell the difference
-				 *		between "rename rel to newrel" and
-				 *		"rename att in rel to newatt" will not
-				 *		work soon because "rename type/operator/rule"
-				 *		stuff is being added. - cim 10/24/90
-				 * ----------------
-				 * [another piece of amuzing but useless anecdote -- ay]
-				 */
 				if (stmt->column == NULL)
 				{
 					/*
 					 * rename relation
-					 *
-					 * Note: we also rename the "type" tuple corresponding to
-					 * the relation.
 					 */
-					renamerel(stmt->relation,	/* old relation */
-							  stmt->newname);	/* new name */
+					renamerel(RangeVarGetRelid(stmt->relation, false),
+							  stmt->newname);
 				}
 				else
 				{
 					/*
 					 * rename attribute
 					 */
-					renameatt(RangeVarGetRelid(stmt->relation, false),	/* relation */
+					renameatt(RangeVarGetRelid(stmt->relation, false),
 							  stmt->column,		/* old att name */
 							  stmt->newname,	/* new att name */
 							  interpretInhOption(stmt->relation->inhOpt));		/* recursive? */

src/include/commands/rename.h

@@ -7,19 +7,19 @@
  * Portions Copyright (c) 1996-2001, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: rename.h,v 1.15 2002/03/29 19:06:22 tgl Exp $
+ * $Id: rename.h,v 1.16 2002/03/31 07:49:30 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
 #ifndef RENAME_H
 #define RENAME_H
 
 extern void renameatt(Oid relid,
-		  char *oldattname,
-		  char *newattname,
+		  const char *oldattname,
+		  const char *newattname,
 		  bool recurse);
 
-extern void renamerel(const RangeVar *relation,
+extern void renamerel(Oid relid,
 		  const char *newrelname);
 
 #endif   /* RENAME_H */