Make DROP TABLE rollback-able: postpone physical file delete until commit.

(WAL logging for this is not done yet, however.)  Clean up a number of really
crufty things that are no longer needed now that DROP behaves nicely.  Make
temp table mapper do the right things when drop or rename affecting a temp
table is rolled back.  Also, remove "relation modified while in use" error
check, in favor of locking tables at first reference and holding that lock
throughout the statement.

7 files changed, 149 insertions, 213 deletions

diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index 5c176254d66..c02bafc3227 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -15,7 +15,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/commands/cluster.c,v 1.58 2000/07/14 22:17:42 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/commands/cluster.c,v 1.59 2000/11/08 22:09:57 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -34,20 +34,14 @@
 #include "utils/builtins.h"
 #include "utils/syscache.h"
 
-static Relation copy_heap(Oid OIDOldHeap);
-static void copy_index(Oid OIDOldIndex, Oid OIDNewHeap);
+static Oid copy_heap(Oid OIDOldHeap, char *NewName);
+static void copy_index(Oid OIDOldIndex, Oid OIDNewHeap, char *NewIndexName);
 static void rebuildheap(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex);
 
 /*
  * cluster
  *
- *	 Check that the relation is a relation in the appropriate user
- *	 ACL. I will use the same security that limits users on the
- *	 renamerel() function.
- *
- *	 Check that the index specified is appropriate for the task
- *	 ( ie it's an index over this relation ). This is trickier.
- *
+ * STILL TO DO:
  *	 Create a list of all the other indicies on this relation. Because
  *	 the cluster will wreck all the tids, I'll need to destroy bogus
  *	 indicies. The user will have to re-create them. Not nice, but
@@ -55,14 +49,6 @@ static void rebuildheap(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex);
  *	 destroy re-build. This may be possible. I'll check out what the
  *	 index create functiond want in the way of paramaters. On the other
  *	 hand, re-creating n indicies may blow out the space.
- *
- *	 Create new (temporary) relations for the base heap and the new
- *	 index.
- *
- *	 Exclusively lock the relations.
- *
- *	 Create new clustered index and base heap relation.
- *
  */
 void
 cluster(char *oldrelname, char *oldindexname)
@@ -70,101 +56,93 @@ cluster(char *oldrelname, char *oldindexname)
 	Oid			OIDOldHeap,
 				OIDOldIndex,
 				OIDNewHeap;
-
 	Relation	OldHeap,
 				OldIndex;
-	Relation	NewHeap;
-
-	char		NewIndexName[NAMEDATALEN];
+	HeapTuple	tuple;
 	char		NewHeapName[NAMEDATALEN];
+	char		NewIndexName[NAMEDATALEN];
 	char		saveoldrelname[NAMEDATALEN];
 	char		saveoldindexname[NAMEDATALEN];
 
 	/*
-	 * Copy the arguments into local storage, because they are probably
-	 * in palloc'd storage that will go away when we commit a transaction.
+	 * Copy the arguments into local storage, just to be safe.
 	 */
-	strcpy(saveoldrelname, oldrelname);
-	strcpy(saveoldindexname, oldindexname);
+	StrNCpy(saveoldrelname, oldrelname, NAMEDATALEN);
+	StrNCpy(saveoldindexname, oldindexname, NAMEDATALEN);
 
 	/*
-	 * Like vacuum, cluster spans transactions, so I'm going to handle it
-	 * in the same way: commit and restart transactions where needed.
-	 *
 	 * We grab exclusive access to the target rel and index for the duration
-	 * of the initial transaction.
+	 * of the transaction.
 	 */
-
 	OldHeap = heap_openr(saveoldrelname, AccessExclusiveLock);
 	OIDOldHeap = RelationGetRelid(OldHeap);
 
-	OldIndex = index_openr(saveoldindexname); /* Open old index relation	*/
+	OldIndex = index_openr(saveoldindexname);
 	LockRelation(OldIndex, AccessExclusiveLock);
 	OIDOldIndex = RelationGetRelid(OldIndex);
 
 	/*
-	 * XXX Should check that index is in fact an index on this relation?
+	 * Check that index is in fact an index on the given relation
 	 */
-
-	heap_close(OldHeap, NoLock);/* do NOT give up the locks */
+	tuple = SearchSysCacheTuple(INDEXRELID,
+								ObjectIdGetDatum(OIDOldIndex),
+								0, 0, 0);
+	if (!HeapTupleIsValid(tuple))
+		elog(ERROR, "CLUSTER: no pg_index entry for index %u",
+			 OIDOldIndex);
+	if (((Form_pg_index) GETSTRUCT(tuple))->indrelid != OIDOldHeap)
+		elog(ERROR, "CLUSTER: \"%s\" is not an index for table \"%s\"",
+			 saveoldindexname, saveoldrelname);
+
+	/* Drop relcache refcnts, but do NOT give up the locks */
+	heap_close(OldHeap, NoLock);
 	index_close(OldIndex);
 
 	/*
-	 * I need to build the copies of the heap and the index. The Commit()
-	 * between here is *very* bogus. If someone is appending stuff, they
-	 * will get the lock after being blocked and add rows which won't be
-	 * present in the new table. Bleagh! I'd be best to try and ensure
-	 * that no-one's in the tables for the entire duration of this process
-	 * with a pg_vlock.  XXX Isn't the above comment now invalid?
+	 * Create the new heap with a temporary name.
 	 */
-	NewHeap = copy_heap(OIDOldHeap);
-	OIDNewHeap = RelationGetRelid(NewHeap);
-	strcpy(NewHeapName, RelationGetRelationName(NewHeap));
+	snprintf(NewHeapName, NAMEDATALEN, "temp_%u", OIDOldHeap);
+
+	OIDNewHeap = copy_heap(OIDOldHeap, NewHeapName);
 
 	/* To make the new heap visible (which is until now empty). */
 	CommandCounterIncrement();
 
+	/*
+	 * Copy the heap data into the new table in the desired order.
+	 */
 	rebuildheap(OIDNewHeap, OIDOldHeap, OIDOldIndex);
 
-	/* To flush the filled new heap (and the statistics about it). */
+	/* To make the new heap's data visible. */
 	CommandCounterIncrement();
 
 	/* Create new index over the tuples of the new heap. */
-	copy_index(OIDOldIndex, OIDNewHeap);
-	snprintf(NewIndexName, NAMEDATALEN, "temp_%x", OIDOldIndex);
+	snprintf(NewIndexName, NAMEDATALEN, "temp_%u", OIDOldIndex);
 
-	/*
-	 * make this really happen. Flush all the buffers. (Believe me, it is
-	 * necessary ... ended up in a mess without it.)
-	 */
-	CommitTransactionCommand();
-	StartTransactionCommand();
+	copy_index(OIDOldIndex, OIDNewHeap, NewIndexName);
+
+	CommandCounterIncrement();
 
 	/* Destroy old heap (along with its index) and rename new. */
 	heap_drop_with_catalog(saveoldrelname, allowSystemTableMods);
 
-	CommitTransactionCommand();
-	StartTransactionCommand();
+	CommandCounterIncrement();
 
 	renamerel(NewHeapName, saveoldrelname);
+
+	/* This one might be unnecessary, but let's be safe. */
+	CommandCounterIncrement();
+
 	renamerel(NewIndexName, saveoldindexname);
 }
 
-static Relation
-copy_heap(Oid OIDOldHeap)
+static Oid
+copy_heap(Oid OIDOldHeap, char *NewName)
 {
-	char		NewName[NAMEDATALEN];
 	TupleDesc	OldHeapDesc,
 				tupdesc;
 	Oid			OIDNewHeap;
-	Relation	NewHeap,
-				OldHeap;
-
-	/*
-	 * Create a new heap relation with a temporary name, which has the
-	 * same tuple description as the old one.
-	 */
-	snprintf(NewName, NAMEDATALEN, "temp_%x", OIDOldHeap);
+	Relation	OldHeap;
 
 	OldHeap = heap_open(OIDOldHeap, AccessExclusiveLock);
 	OldHeapDesc = RelationGetDescr(OldHeap);
@@ -173,7 +151,6 @@ copy_heap(Oid OIDOldHeap)
 	 * Need to make a copy of the tuple descriptor,
 	 * heap_create_with_catalog modifies it.
 	 */
-
 	tupdesc = CreateTupleDescCopy(OldHeapDesc);
 
 	OIDNewHeap = heap_create_with_catalog(NewName, tupdesc,
@@ -181,19 +158,15 @@ copy_heap(Oid OIDOldHeap)
 										  allowSystemTableMods);
 
 	if (!OidIsValid(OIDNewHeap))
-		elog(ERROR, "clusterheap: cannot create temporary heap relation\n");
+		elog(ERROR, "copy_heap: cannot create temporary heap relation");
 
-	/* XXX why are we bothering to do this: */
-	NewHeap = heap_open(OIDNewHeap, AccessExclusiveLock);
-
-	heap_close(NewHeap, AccessExclusiveLock);
-	heap_close(OldHeap, AccessExclusiveLock);
+	heap_close(OldHeap, NoLock);
 
-	return NewHeap;
+	return OIDNewHeap;
 }
 
 static void
-copy_index(Oid OIDOldIndex, Oid OIDNewHeap)
+copy_index(Oid OIDOldIndex, Oid OIDNewHeap, char *NewIndexName)
 {
 	Relation	OldIndex,
 				NewHeap;
@@ -202,18 +175,17 @@ copy_index(Oid OIDOldIndex, Oid OIDNewHeap)
 	Form_pg_index Old_pg_index_Form;
 	Form_pg_class Old_pg_index_relation_Form;
 	IndexInfo  *indexInfo;
-	char	   *NewIndexName;
 
 	NewHeap = heap_open(OIDNewHeap, AccessExclusiveLock);
 	OldIndex = index_open(OIDOldIndex);
 
 	/*
-	 * OK. Create a new (temporary) index for the one that's already here.
+	 * Create a new (temporary) index like the one that's already here.
 	 * To do this I get the info from pg_index, and add a new index with
 	 * a temporary name.
 	 */
 	Old_pg_index_Tuple = SearchSysCacheTupleCopy(INDEXRELID,
-							ObjectIdGetDatum(RelationGetRelid(OldIndex)),
+												 ObjectIdGetDatum(OIDOldIndex),
 												 0, 0, 0);
 	Assert(Old_pg_index_Tuple);
 	Old_pg_index_Form = (Form_pg_index) GETSTRUCT(Old_pg_index_Tuple);
@@ -221,15 +193,11 @@ copy_index(Oid OIDOldIndex, Oid OIDNewHeap)
 	indexInfo = BuildIndexInfo(Old_pg_index_Tuple);
 
 	Old_pg_index_relation_Tuple = SearchSysCacheTupleCopy(RELOID,
-							ObjectIdGetDatum(RelationGetRelid(OldIndex)),
+														  ObjectIdGetDatum(OIDOldIndex),
 														  0, 0, 0);
 	Assert(Old_pg_index_relation_Tuple);
 	Old_pg_index_relation_Form = (Form_pg_class) GETSTRUCT(Old_pg_index_relation_Tuple);
 
-	/* Set the name. */
-	NewIndexName = palloc(NAMEDATALEN); /* XXX */
-	snprintf(NewIndexName, NAMEDATALEN, "temp_%x", OIDOldIndex);
-
 	index_create(RelationGetRelationName(NewHeap),
 				 NewIndexName,
 				 indexInfo,
@@ -239,10 +207,10 @@ copy_index(Oid OIDOldIndex, Oid OIDNewHeap)
 				 Old_pg_index_Form->indisprimary,
 				 allowSystemTableMods);
 
-	setRelhasindexInplace(OIDNewHeap, true, false);
+	setRelhasindex(OIDNewHeap, true);
 
 	index_close(OldIndex);
-	heap_close(NewHeap, AccessExclusiveLock);
+	heap_close(NewHeap, NoLock);
 }
 
 
@@ -294,6 +262,6 @@ rebuildheap(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex)
 	index_endscan(ScanDesc);
 
 	index_close(LocalOldIndex);
-	heap_close(LocalOldHeap, AccessExclusiveLock);
-	heap_close(LocalNewHeap, AccessExclusiveLock);
+	heap_close(LocalOldHeap, NoLock);
+	heap_close(LocalNewHeap, NoLock);
 }
diff --git a/src/backend/commands/command.c b/src/backend/commands/command.c
index 4446c9f5cb5..54b913dcac1 100644
--- a/src/backend/commands/command.c
+++ b/src/backend/commands/command.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/commands/Attic/command.c,v 1.108 2000/10/26 21:34:44 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/commands/Attic/command.c,v 1.109 2000/11/08 22:09:57 tgl Exp $
  *
  * NOTES
  *	  The PerformAddAttribute() code, like most of the relation
@@ -1661,9 +1661,13 @@ AlterTableCreateToastTable(const char *relationName, bool silent)
 
 	/*
 	 * Update toast rel's pg_class entry to show that it has an index.
-	 * NOTE this also does CommandCounterIncrement() to make index visible.
 	 */
-	setRelhasindexInplace(toast_relid, true, false);
+	setRelhasindex(toast_relid, true);
+
+	/*
+	 * Make index visible
+	 */
+	CommandCounterIncrement();
 
 	/*
 	 * Get the OID of the newly created index
diff --git a/src/backend/commands/comment.c b/src/backend/commands/comment.c
index 6dd3c4dfab8..bff2b897c6e 100644
--- a/src/backend/commands/comment.c
+++ b/src/backend/commands/comment.c
@@ -356,10 +356,8 @@ CommentAttribute(char *relname, char *attrname, char *comment)
 	attrtuple = SearchSysCacheTuple(ATTNAME, ObjectIdGetDatum(relation->rd_id),
 									PointerGetDatum(attrname), 0, 0);
 	if (!HeapTupleIsValid(attrtuple))
-	{
 		elog(ERROR, "'%s' is not an attribute of class '%s'",
 			 attrname, relname);
-	}
 	oid = attrtuple->t_data->t_oid;
 
 	/*** Call CreateComments() to create/drop the comments ***/
@@ -368,8 +366,7 @@ CommentAttribute(char *relname, char *attrname, char *comment)
 
 	/*** Now, close the heap relation and return ***/
 
-	heap_close(relation, AccessShareLock);
-
+	heap_close(relation, NoLock);
 }
 
 /*------------------------------------------------------------------
@@ -840,6 +837,5 @@ CommentTrigger(char *trigger, char *relname, char *comment)
 
 	heap_endscan(scan);
 	heap_close(pg_trigger, AccessShareLock);
-	heap_close(relation, AccessShareLock);
-
+	heap_close(relation, NoLock);
 }
diff --git a/src/backend/commands/creatinh.c b/src/backend/commands/creatinh.c
index b6485850eb3..75fd0473922 100644
--- a/src/backend/commands/creatinh.c
+++ b/src/backend/commands/creatinh.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/commands/Attic/creatinh.c,v 1.64 2000/09/12 21:06:47 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/commands/Attic/creatinh.c,v 1.65 2000/11/08 22:09:57 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -150,6 +150,20 @@ DefineRelation(CreateStmt *stmt, char relkind)
 	StoreCatalogInheritance(relationId, inheritList);
 
 	/*
+	 * We must bump the command counter to make the newly-created relation
+	 * tuple visible for opening.
+	 */
+	CommandCounterIncrement();
+
+	/*
+	 * Open the new relation and acquire exclusive lock on it.  This isn't
+	 * really necessary for locking out other backends (since they can't
+	 * see the new rel anyway until we commit), but it keeps the lock manager
+	 * from complaining about deadlock risks.
+	 */
+	rel = heap_openr(relname, AccessExclusiveLock);
+
+	/*
 	 * Now add any newly specified column default values and CHECK
 	 * constraints to the new relation.  These are passed to us in the
 	 * form of raw parsetrees; we need to transform them to executable
@@ -181,25 +195,11 @@ DefineRelation(CreateStmt *stmt, char relkind)
 		rawDefaults = lappend(rawDefaults, rawEnt);
 	}
 
-	/* If no raw defaults and no constraints, nothing to do. */
-	if (rawDefaults == NIL && stmt->constraints == NIL)
-		return;
-
-	/*
-	 * We must bump the command counter to make the newly-created relation
-	 * tuple visible for opening.
-	 */
-	CommandCounterIncrement();
-
-	/*
-	 * Open the new relation.
-	 */
-	rel = heap_openr(relname, AccessExclusiveLock);
-
 	/*
-	 * Parse and add the defaults/constraints.
+	 * Parse and add the defaults/constraints, if any.
 	 */
-	AddRelationRawConstraints(rel, rawDefaults, stmt->constraints);
+	if (rawDefaults || stmt->constraints)
+		AddRelationRawConstraints(rel, rawDefaults, stmt->constraints);
 
 	/*
 	 * Clean up.  We keep lock on new relation (although it shouldn't be
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index f9d1a92c758..fff6d569753 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/commands/indexcmds.c,v 1.39 2000/10/22 23:32:39 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/commands/indexcmds.c,v 1.40 2000/11/08 22:09:57 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -214,7 +214,7 @@ DefineIndex(char *heapRelationName,
 	 * backends to flush their relcache entries and in particular their
 	 * cached lists of the indexes for this relation.
 	 */
-	setRelhasindexInplace(relationId, true, false);
+	setRelhasindex(relationId, true);
 }
 
 
@@ -635,6 +635,15 @@ ReindexIndex(const char *name, bool force /* currently unused */ )
 {
 	HeapTuple	tuple;
 
+	/* ----------------
+	 *	REINDEX within a transaction block is dangerous, because
+	 *	if the transaction is later rolled back we have no way to
+	 *	undo truncation of the index's physical file.  Disallow it.
+	 * ----------------
+	 */
+	if (IsTransactionBlock())
+		elog(ERROR, "REINDEX cannot run inside a BEGIN/END block");
+
 	tuple = SearchSysCacheTuple(RELNAME,
 								PointerGetDatum(name),
 								0, 0, 0);
@@ -666,6 +675,15 @@ ReindexTable(const char *name, bool force)
 {
 	HeapTuple	tuple;
 
+	/* ----------------
+	 *	REINDEX within a transaction block is dangerous, because
+	 *	if the transaction is later rolled back we have no way to
+	 *	undo truncation of the index's physical file.  Disallow it.
+	 * ----------------
+	 */
+	if (IsTransactionBlock())
+		elog(ERROR, "REINDEX cannot run inside a BEGIN/END block");
+
 	tuple = SearchSysCacheTuple(RELNAME,
 								PointerGetDatum(name),
 								0, 0, 0);
diff --git a/src/backend/commands/rename.c b/src/backend/commands/rename.c
index 0f41cac1dc7..6a9de4abf00 100644
--- a/src/backend/commands/rename.c
+++ b/src/backend/commands/rename.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/commands/Attic/rename.c,v 1.51 2000/10/22 23:32:39 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/commands/Attic/rename.c,v 1.52 2000/11/08 22:09:57 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -183,16 +183,9 @@ renamerel(const char *oldrelname, const char *newrelname)
 	Oid			reloid;
 	char		relkind;
 	Relation	irelations[Num_pg_class_indices];
-#ifdef OLD_FILE_NAMING
-	int			i;
-	char		oldpath[MAXPGPATH],
-				newpath[MAXPGPATH],
-				toldpath[MAXPGPATH + 10],
-				tnewpath[MAXPGPATH + 10];
-#endif
 
 	if (!allowSystemTableMods && IsSystemRelationName(oldrelname))
-		elog(ERROR, "renamerel: system relation \"%s\" not renamed",
+		elog(ERROR, "renamerel: system relation \"%s\" may not be renamed",
 			 oldrelname);
 
 	if (!allowSystemTableMods && IsSystemRelationName(newrelname))
@@ -201,7 +194,7 @@ renamerel(const char *oldrelname, const char *newrelname)
 
 	/*
 	 * Check for renaming a temp table, which only requires altering
-	 * the temp-table mapping, not the physical table.
+	 * the temp-table mapping, not the underlying table.
 	 */
 	if (rename_temp_relation(oldrelname, newrelname))
 		return;					/* all done... */
@@ -213,7 +206,7 @@ renamerel(const char *oldrelname, const char *newrelname)
 	targetrelation = RelationNameGetRelation(oldrelname);
 
 	if (!RelationIsValid(targetrelation))
-		elog(ERROR, "Relation '%s' does not exist", oldrelname);
+		elog(ERROR, "Relation \"%s\" does not exist", oldrelname);
 
 	/*
 	 * Grab an exclusive lock on the target table, which we will NOT
@@ -221,47 +214,10 @@ renamerel(const char *oldrelname, const char *newrelname)
 	 */
 	LockRelation(targetrelation, AccessExclusiveLock);
 
-	/* ----------------
-	 *	RENAME TABLE within a transaction block is dangerous, because
-	 *	if the transaction is later rolled back we have no way to
-	 *	undo the rename of the relation's physical file.  For now, allow it
-	 *	but emit a warning message.
-	 *	Someday we might want to consider postponing the physical rename
-	 *	until transaction commit, but that's a lot of work...
-	 *	The only case that actually works right is for relations created
-	 *	in the current transaction, since the post-abort state would be that
-	 *	they don't exist anyway.  So, no warning in that case.
-	 * ----------------
-	 */
-	if (IsTransactionBlock() && !targetrelation->rd_myxactonly)
-		elog(NOTICE, "Caution: RENAME TABLE cannot be rolled back, so don't abort now");
-
 	reloid = RelationGetRelid(targetrelation);
 	relkind = targetrelation->rd_rel->relkind;
 
 	/*
-	 * Flush all blocks of the relation out of the buffer pool.  We need
-	 * this because the blocks are marked with the relation's name as well
-	 * as OID. If some backend tries to write a dirty buffer with
-	 * mdblindwrt after we've renamed the physical file, we'll be in big
-	 * trouble.
-	 *
-	 * Since we hold the exclusive lock on the relation, we don't have to
-	 * worry about more blocks being read in while we finish the rename.
-	 */
-	if (FlushRelationBuffers(targetrelation, (BlockNumber) 0) < 0)
-		elog(ERROR, "renamerel: unable to flush relation from buffer pool");
-
-	/*
-	 * Make sure smgr and lower levels close the relation's files. (Next
-	 * access to rel will reopen them.)
-	 *
-	 * Note: we rely on shared cache invalidation message to make other
-	 * backends close and re-open the files.
-	 */
-	smgrclose(DEFAULT_SMGR, targetrelation);
-
-	/*
 	 * Close rel, but keep exclusive lock!
 	 */
 	heap_close(targetrelation, NoLock);
@@ -271,8 +227,9 @@ renamerel(const char *oldrelname, const char *newrelname)
 	 * 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?
 	 */
-	targetrelation = NULL;		/* make sure I don't touch it again */
 	RelationIdInvalidateRelationCacheByRelationId(reloid);
 
 	/*
@@ -291,7 +248,8 @@ renamerel(const char *oldrelname, const char *newrelname)
 		elog(ERROR, "renamerel: relation \"%s\" exists", newrelname);
 
 	/*
-	 * Update pg_class tuple with new relname.
+	 * Update pg_class tuple with new relname.  (Scribbling on oldreltup
+	 * is OK because it's a copy...)
 	 */
 	StrNCpy(NameStr(((Form_pg_class) GETSTRUCT(oldreltup))->relname),
 			newrelname, NAMEDATALEN);
@@ -310,36 +268,4 @@ renamerel(const char *oldrelname, const char *newrelname)
 	 */
 	if (relkind != RELKIND_INDEX)
 		TypeRename(oldrelname, newrelname);
-
-#ifdef OLD_FILE_NAMING
-	/*
-	 * Perform physical rename of files.  If this fails, we haven't yet
-	 * done anything irreversible.  NOTE that this MUST be the last step;
-	 * an error occurring afterwards would leave the relation hosed!
-	 *
-	 * XXX smgr.c ought to provide an interface for this; doing it directly
-	 * is bletcherous.
-	 */
-	strcpy(oldpath, relpath(oldrelname));
-	strcpy(newpath, relpath(newrelname));
-	if (rename(oldpath, newpath) < 0)
-		elog(ERROR, "renamerel: unable to rename %s to %s: %m",
-			 oldpath, newpath);
-
-	/* rename additional segments of relation, too */
-	for (i = 1;; i++)
-	{
-		sprintf(toldpath, "%s.%d", oldpath, i);
-		sprintf(tnewpath, "%s.%d", newpath, i);
-		if (rename(toldpath, tnewpath) < 0)
-		{
-			/* expected case is that there's not another segment file */
-			if (errno == ENOENT)
-				break;
-			/* otherwise we're up the creek... */
-			elog(ERROR, "renamerel: unable to rename %s to %s: %m",
-				 toldpath, tnewpath);
-		}
-	}
-#endif
 }
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index 059bc42987f..33340291e1c 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/commands/trigger.c,v 1.78 2000/10/16 17:08:05 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/commands/trigger.c,v 1.79 2000/11/08 22:09:57 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -388,6 +388,7 @@ RelationRemoveTriggers(Relation rel)
 	HeapScanDesc tgscan;
 	ScanKeyData key;
 	HeapTuple	tup;
+	bool		found = false;
 
 	tgrel = heap_openr(TriggerRelationName, RowExclusiveLock);
 	ScanKeyEntryInitialize(&key, 0, Anum_pg_trigger_tgrelid,
@@ -403,17 +404,44 @@ RelationRemoveTriggers(Relation rel)
 		DeleteComments(tup->t_data->t_oid);
 
 		heap_delete(tgrel, &tup->t_self, NULL);
+
+		found = true;
 	}
 
 	heap_endscan(tgscan);
 
 	/* ----------
-	 * Need to bump it here so the following doesn't see
-	 * the already deleted triggers again for a self-referencing
-	 * table.
+	 * If we deleted any triggers, must update pg_class entry and
+	 * advance command counter to make the updated entry visible.
+	 * This is fairly annoying, since we'e just going to drop the
+	 * durn thing later, but it's necessary to have a consistent
+	 * state in case we do CommandCounterIncrement() below ---
+	 * if RelationBuildTriggers() runs, it will complain otherwise.
+	 * Perhaps RelationBuildTriggers() shouldn't be so picky...
 	 * ----------
 	 */
-	CommandCounterIncrement();
+	if (found)
+	{
+		Relation	pgrel;
+		Relation	ridescs[Num_pg_class_indices];
+
+		pgrel = heap_openr(RelationRelationName, RowExclusiveLock);
+		tup = SearchSysCacheTupleCopy(RELOID,
+									  RelationGetRelid(rel),
+									  0, 0, 0);
+		if (!HeapTupleIsValid(tup))
+			elog(ERROR, "RelationRemoveTriggers: relation %u not found in pg_class",
+				 RelationGetRelid(rel));
+
+		((Form_pg_class) GETSTRUCT(tup))->reltriggers = 0;
+		heap_update(pgrel, &tup->t_self, tup, NULL);
+		CatalogOpenIndices(Num_pg_class_indices, Name_pg_class_indices, ridescs);
+		CatalogIndexInsert(ridescs, Num_pg_class_indices, pgrel, tup);
+		CatalogCloseIndices(Num_pg_class_indices, ridescs);
+		heap_freetuple(tup);
+		heap_close(pgrel, RowExclusiveLock);
+		CommandCounterIncrement();
+	}
 
 	/* ----------
 	 * Also drop all constraint triggers referencing this relation
@@ -431,22 +459,21 @@ RelationRemoveTriggers(Relation rel)
 
 		pg_trigger = (Form_pg_trigger) GETSTRUCT(tup);
 
-		refrel = heap_open(pg_trigger->tgrelid, NoLock);
+		stmt.trigname = pstrdup(NameStr(pg_trigger->tgname));
+
+		/* May as well grab AccessExclusiveLock, since DropTrigger will. */
+		refrel = heap_open(pg_trigger->tgrelid, AccessExclusiveLock);
 		stmt.relname = pstrdup(RelationGetRelationName(refrel));
 		heap_close(refrel, NoLock);
 
-		stmt.trigname = DatumGetCString(DirectFunctionCall1(nameout,
-						NameGetDatum(&pg_trigger->tgname)));
-
-
 		elog(NOTICE, "DROP TABLE implicitly drops referential integrity trigger from table \"%s\"", stmt.relname);
 
 		DropTrigger(&stmt);
 
 		/* ----------
 		 * Need to do a command counter increment here to show up
-		 * new pg_class.reltriggers in the next loop invocation already
-		 * (there are multiple referential integrity action
+		 * new pg_class.reltriggers in the next loop iteration
+		 * (in case there are multiple referential integrity action
 		 * triggers for the same FK table defined on the PK table).
 		 * ----------
 		 */
@@ -747,9 +774,6 @@ equalTriggerDescs(TriggerDesc *trigdesc1, TriggerDesc *trigdesc2)
 	 * We need not examine the "index" data, just the trigger array
 	 * itself; if we have the same triggers with the same types, the
 	 * derived index data should match.
-	 *
-	 * XXX It seems possible that the same triggers could appear in different
-	 * orders in the two trigger arrays; do we need to handle that?
 	 */
 	if (trigdesc1 != NULL)
 	{