Code review for ON COMMIT patch. Make the actual on-commit action happen

before commit, not after :-( --- the original coding is not only unsafe
if an error occurs while it's processing, but it generates an invalid
sequence of WAL entries.  Resurrect 7.2 logic for deleting items when
no longer needed.  Use an enum instead of random macros.  Editorialize
on names used for routines and constants.  Teach backend/nodes routines
about new field in CreateTable struct.  Add a regression test.

20 files changed, 330 insertions, 300 deletions

diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 164897e534e..7586ee8a46a 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/access/transam/xact.c,v 1.136 2002/11/09 23:56:38 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/access/transam/xact.c,v 1.137 2002/11/11 22:19:20 tgl Exp $
  *
  * NOTES
  *		Transaction aborts can now occur two ways:
@@ -951,6 +951,12 @@ CommitTransaction(void)
 	 */
 	DeferredTriggerEndXact();
 
+	/*
+	 * Similarly, let ON COMMIT management do its thing before we start
+	 * to commit.
+	 */
+	PreCommit_on_commit_actions();
+
 	/* Prevent cancel/die interrupt while cleaning up */
 	HOLD_INTERRUPTS();
 
@@ -1027,7 +1033,7 @@ CommitTransaction(void)
 	AtEOXact_hash();
 	AtEOXact_nbtree();
 	AtEOXact_rtree();
-	AtEOXact_temp_relations(true,s->blockState);
+	AtEOXact_on_commit_actions(true);
 	AtEOXact_Namespace(true);
 	AtEOXact_CatCache(true);
 	AtEOXact_Files();
@@ -1138,7 +1144,7 @@ AbortTransaction(void)
 	AtEOXact_hash();
 	AtEOXact_nbtree();
 	AtEOXact_rtree();
-	AtEOXact_temp_relations(false,s->blockState);
+	AtEOXact_on_commit_actions(false);
 	AtEOXact_Namespace(false);
 	AtEOXact_CatCache(false);
 	AtEOXact_Files();
diff --git a/src/backend/bootstrap/bootparse.y b/src/backend/bootstrap/bootparse.y
index ee4a5ea9b31..1d64ca285cf 100644
--- a/src/backend/bootstrap/bootparse.y
+++ b/src/backend/bootstrap/bootparse.y
@@ -9,7 +9,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/bootstrap/bootparse.y,v 1.54 2002/11/09 23:56:38 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/bootstrap/bootparse.y,v 1.55 2002/11/11 22:19:21 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -34,7 +34,6 @@
 #include "catalog/pg_class.h"
 #include "catalog/pg_namespace.h"
 #include "commands/defrem.h"
-#include "commands/tablecmds.h"
 #include "miscadmin.h"
 #include "nodes/makefuncs.h"
 #include "nodes/nodes.h"
@@ -198,7 +197,7 @@ Boot_CreateStmt:
 													  tupdesc,
 													  RELKIND_RELATION,
 													  $3,
-													  ATEOXACTNOOP,
+													  ONCOMMIT_NOOP,
 													  true);
 						elog(DEBUG3, "relation created with oid %u", id);
 					}
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index 8a1e48a757a..9956672c12a 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/catalog/heap.c,v 1.233 2002/11/09 23:56:39 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/catalog/heap.c,v 1.234 2002/11/11 22:19:21 tgl Exp $
  *
  *
  * INTERFACE ROUTINES
@@ -31,14 +31,12 @@
 
 #include "access/heapam.h"
 #include "access/genam.h"
-#include "access/xact.h"
 #include "catalog/catalog.h"
 #include "catalog/catname.h"
 #include "catalog/dependency.h"
 #include "catalog/heap.h"
 #include "catalog/index.h"
 #include "catalog/indexing.h"
-#include "catalog/namespace.h"
 #include "catalog/pg_attrdef.h"
 #include "catalog/pg_constraint.h"
 #include "catalog/pg_inherits.h"
@@ -673,14 +671,13 @@ AddNewRelationType(const char *typeName,
  *		creates a new cataloged relation.  see comments above.
  * --------------------------------
  */
-
 Oid
 heap_create_with_catalog(const char *relname,
 						 Oid relnamespace,
 						 TupleDesc tupdesc,
 						 char relkind,
 						 bool shared_relation,
-						 char ateoxact,  /* Only used for temp relations */
+						 OnCommitAction oncommit,
 						 bool allow_system_table_mods)
 {
 	Relation	pg_class_desc;
@@ -722,25 +719,6 @@ heap_create_with_catalog(const char *relname,
 	/* Assign an OID for the relation's tuple type */
 	new_type_oid = newoid();
 
-
-	/*
-	 *  Add to temprels if we are a temp relation now that we have oid
-	 */
-
-	if(isTempNamespace(relnamespace)) {
-		TempTable	*t;
-		MemoryContext oldcxt;
-
-		oldcxt = MemoryContextSwitchTo(CacheMemoryContext);
-		t = (TempTable *) palloc(sizeof(TempTable));
-		t->relid = new_rel_oid;
-		t->ateoxact = ateoxact;
-		t->tid = GetCurrentTransactionId();
-		t->dead = false;
-		reg_temp_rel(t);
-		MemoryContextSwitchTo(oldcxt);
-	}
-
 	/*
 	 * now create an entry in pg_class for the relation.
 	 *
@@ -805,6 +783,12 @@ heap_create_with_catalog(const char *relname,
 	StoreConstraints(new_rel_desc, tupdesc);
 
 	/*
+	 * If there's a special on-commit action, remember it
+	 */
+	if (oncommit != ONCOMMIT_NOOP)
+		register_on_commit_action(new_rel_oid, oncommit);
+
+	/*
 	 * ok, the relation has been cataloged, so close our relations and
 	 * return the oid of the newly created relation.
 	 */
@@ -1165,6 +1149,11 @@ heap_drop_with_catalog(Oid rid)
 	DeleteRelationTuple(RelationGetRelid(rel));
 
 	/*
+	 * forget any ON COMMIT action for the rel
+	 */
+	remove_on_commit_action(rid);
+
+	/*
 	 * unlink the relation's physical file and finish up.
 	 */
 	if (rel->rd_rel->relkind != RELKIND_VIEW &&
@@ -1172,14 +1161,6 @@ heap_drop_with_catalog(Oid rid)
 		smgrunlink(DEFAULT_SMGR, rel);
 
 	/*
-	 * Keep temprels up to date so that we don't have ON COMMIT execution
-	 * problems at the end of the next transaction block
-	 */
-
-	if(isTempNamespace(RelationGetNamespace(rel)))
-		rm_temp_rel(rid);
-
-	/*
 	 * Close relcache entry, but *keep* AccessExclusiveLock on the
 	 * relation until transaction commit.  This ensures no one else will
 	 * try to do something with the doomed relation.
@@ -1941,12 +1922,13 @@ RelationTruncateIndexes(Oid heapId)
  *
  *	 This routine is used to truncate the data from the
  *	 storage manager of any data within the relation handed
- *	 to this routine.
+ *	 to this routine.  This is not transaction-safe!
  */
 void
 heap_truncate(Oid rid)
 {
 	Relation	rel;
+	Oid			toastrelid;
 
 	/* Open relation for processing, and grab exclusive access on it. */
 	rel = heap_open(rid, AccessExclusiveLock);
@@ -1965,6 +1947,11 @@ heap_truncate(Oid rid)
 	/* If this relation has indexes, truncate the indexes too */
 	RelationTruncateIndexes(rid);
 
+	/* If it has a toast table, recursively truncate that too */
+	toastrelid = rel->rd_rel->reltoastrelid;
+	if (OidIsValid(toastrelid))
+		heap_truncate(toastrelid);
+
 	/*
 	 * Close the relation, but keep exclusive lock on it until commit.
 	 */
diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c
index f44718f65b6..977ee85bb9d 100644
--- a/src/backend/catalog/namespace.c
+++ b/src/backend/catalog/namespace.c
@@ -13,7 +13,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/catalog/namespace.c,v 1.39 2002/11/09 23:56:39 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/catalog/namespace.c,v 1.40 2002/11/11 22:19:21 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -34,7 +34,6 @@
 #include "catalog/pg_proc.h"
 #include "catalog/pg_shadow.h"
 #include "catalog/pg_type.h"
-#include "commands/tablecmds.h"
 #include "lib/stringinfo.h"
 #include "miscadmin.h"
 #include "nodes/makefuncs.h"
@@ -1671,7 +1670,6 @@ RemoveTempRelationsCallback(void)
 
 		CommitTransactionCommand(true);
 	}
-	free_temp_rels();
 }
 
 
diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index ed0b306c447..48cc81ea841 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -11,7 +11,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/commands/cluster.c,v 1.92 2002/11/09 23:56:39 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/commands/cluster.c,v 1.93 2002/11/11 22:19:21 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -25,7 +25,6 @@
 #include "catalog/index.h"
 #include "catalog/indexing.h"
 #include "catalog/catname.h"
-#include "catalog/namespace.h"
 #include "commands/cluster.h"
 #include "commands/tablecmds.h"
 #include "miscadmin.h"
@@ -204,7 +203,7 @@ make_new_heap(Oid OIDOldHeap, const char *NewName)
 										  tupdesc,
 										  OldHeap->rd_rel->relkind,
 										  OldHeap->rd_rel->relisshared,
-										  ATEOXACTNOOP,
+										  ONCOMMIT_NOOP,
 										  allowSystemTableMods);
 
 	/*
diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c
index 31fb270c63e..f5d41b3df2c 100644
--- a/src/backend/commands/sequence.c
+++ b/src/backend/commands/sequence.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/commands/sequence.c,v 1.89 2002/11/10 00:10:20 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/commands/sequence.c,v 1.90 2002/11/11 22:19:21 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -193,6 +193,7 @@ DefineSequence(CreateSeqStmt *seq)
 	stmt->inhRelations = NIL;
 	stmt->constraints = NIL;
 	stmt->hasoids = false;
+	stmt->oncommit = ONCOMMIT_NOOP;
 
 	seqoid = DefineRelation(stmt, RELKIND_SEQUENCE);
 
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 40a790ca17e..8023ba83420 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -8,13 +8,12 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/commands/tablecmds.c,v 1.52 2002/11/09 23:56:39 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/commands/tablecmds.c,v 1.53 2002/11/11 22:19:21 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
 #include "postgres.h"
 
-#include "access/xact.h"
 #include "access/genam.h"
 #include "access/tuptoaster.h"
 #include "catalog/catalog.h"
@@ -52,7 +51,27 @@
 #include "utils/relcache.h"
 #include "utils/syscache.h"
 
-static List *temprels = NIL;
+
+/*
+ * ON COMMIT action list
+ */
+typedef struct OnCommitItem
+{
+	Oid				relid;			/* relid of relation */
+	OnCommitAction	oncommit;		/* what to do at end of xact */
+
+	/*
+	 * If this entry was created during this xact, it should be deleted at
+	 * xact abort.	Conversely, if this entry was deleted during this
+	 * xact, it should be removed at xact commit.  We leave deleted
+	 * entries in the list until commit so that we can roll back if needed.
+	 */
+	bool		created_in_cur_xact;
+	bool		deleted_in_cur_xact;
+} OnCommitItem;
+
+static List *on_commits = NIL;
+
 
 static List *MergeAttributes(List *schema, List *supers, bool istemp,
 				List **supOids, List **supconstr, bool *supHasOids);
@@ -118,7 +137,6 @@ DefineRelation(CreateStmt *stmt, char relkind)
 	int			i;
 	AttrNumber	attnum;
 
-
 	/*
 	 * Truncate relname to appropriate length (probably a waste of time,
 	 * as parser should have done this already).
@@ -126,6 +144,12 @@ DefineRelation(CreateStmt *stmt, char relkind)
 	StrNCpy(relname, stmt->relation->relname, NAMEDATALEN);
 
 	/*
+	 * Check consistency of arguments
+	 */
+	if (stmt->oncommit != ONCOMMIT_NOOP && !stmt->relation->istemp)
+		elog(ERROR, "ON COMMIT can only be used on TEMP tables");
+
+	/*
 	 * Look up the namespace in which we are supposed to create the
 	 * relation.  Check we have permission to create there. Skip check if
 	 * bootstrapping, since permissions machinery may not be working yet.
@@ -225,7 +249,7 @@ DefineRelation(CreateStmt *stmt, char relkind)
 										  descriptor,
 										  relkind,
 										  false,
-										  stmt->ateoxact,
+										  stmt->oncommit,
 										  allowSystemTableMods);
 
 	StoreCatalogInheritance(relationId, inheritOids);
@@ -333,20 +357,16 @@ RemoveRelation(const RangeVar *relation, DropBehavior behavior)
 
 /*
  * TruncateRelation
- *				  Removes all the rows from a relation
- *
- * Exceptions:
- *				  BadArg if name is invalid
+ *		Removes all the rows from a relation.
  *
- * Note:
- *				  Rows are removed, indexes are truncated and reconstructed.
+ * Note: This routine only does safety and permissions checks;
+ * heap_truncate does the actual work.
  */
 void
 TruncateRelation(const RangeVar *relation)
 {
 	Relation	rel;
 	Oid			relid;
-	Oid			toastrelid;
 	ScanKeyData key;
 	Relation	fkeyRel;
 	SysScanDesc fkeyScan;
@@ -426,17 +446,11 @@ TruncateRelation(const RangeVar *relation)
 	systable_endscan(fkeyScan);
 	heap_close(fkeyRel, AccessShareLock);
 
-	toastrelid = rel->rd_rel->reltoastrelid;
-
 	/* Keep the lock until transaction commit */
 	heap_close(rel, NoLock);
 
-	/* Truncate the table proper */
+	/* Do the real work */
 	heap_truncate(relid);
-
-	/* If it has a toast table, truncate that too */
-	if (OidIsValid(toastrelid))
-		heap_truncate(toastrelid);
 }
 
 /*----------
@@ -3787,18 +3801,12 @@ AlterTableCreateToastTable(Oid relOid, bool silent)
 	 * when its master is, so there's no need to handle the toast rel as
 	 * temp.
 	 */
-
-	/*
-	 * Pass ATEOXACTNOOP for ateoxact since we want heap_drop_with_catalog()
-	 * to remove TOAST tables for temp tables, not AtEOXact_temp_relations()
-	 */
-
 	toast_relid = heap_create_with_catalog(toast_relname,
 										   PG_TOAST_NAMESPACE,
 										   tupdesc,
 										   RELKIND_TOASTVALUE,
 										   shared_relation,
-										   ATEOXACTNOOP,
+										   ONCOMMIT_NOOP,
 										   true);
 
 	/* make the toast relation visible, else index creation will fail */
@@ -3934,205 +3942,159 @@ needs_toast_table(Relation rel)
 	return (tuple_length > TOAST_TUPLE_THRESHOLD);
 }
 
+
 /*
- * To handle ON COMMIT { DROP | PRESERVE ROWS | DELETE ROWS }
+ * This code supports
+ *	CREATE TEMP TABLE ... ON COMMIT { DROP | PRESERVE ROWS | DELETE ROWS }
+ *
+ * Because we only support this for TEMP tables, it's sufficient to remember
+ * the state in a backend-local data structure.
+ */
+
+/*
+ * Register a newly-created relation's ON COMMIT action.
  */
 void
-AtEOXact_temp_relations(bool iscommit, int bstate)
+register_on_commit_action(Oid relid, OnCommitAction action)
 {
-	List	   *l,
-			   *prev;
-	MemoryContext oldctx;
-
-	if (temprels == NIL)
-		return;
+	OnCommitItem	*oc;
+	MemoryContext oldcxt;
 
 	/*
-	 *	These loops are tricky because we are removing items from the List
-	 *	while we are traversing it.
+	 * We needn't bother registering the relation unless there is an ON COMMIT
+	 * action we need to take.
 	 */
-
-
-	/* Remove 'dead' entries on commit and clear 'dead' status on abort */
-	l = temprels;
-	prev = NIL;
-	while (l != NIL)
-	{
-		TempTable  *t = lfirst(l);
-
-		if (t->dead)
-		{
-			if (iscommit)
-			{
-				/* Remove from temprels, since the user has DROP'd */
-				oldctx = MemoryContextSwitchTo(CacheMemoryContext);
-				if (prev == NIL)
-				{
-					pfree(t);
-					temprels = lnext(l);
-					pfree(l);
-					l = temprels;
-				}
-				else
-				{
-					pfree(t);
-					lnext(prev) = lnext(l);
-					pfree(l);
-					l = lnext(prev);
-				}
-				MemoryContextSwitchTo(oldctx);
-				continue;
-			}
-			else
-				/* user dropped but now we're aborted */
-				t->dead = false;
-		}
-		prev = l;
-		l = lnext(l);
-	}
-
-	if ((iscommit && bstate != TBLOCK_END) ||
-		(!iscommit && bstate != TBLOCK_ABORT))
+	if (action == ONCOMMIT_NOOP || action == ONCOMMIT_PRESERVE_ROWS)
 		return;
 
-	/* Perform per-xact actions */
-	l = temprels;
-	prev = NIL;
-
-	if (iscommit)
-	{
-		while (l != NIL)
-		{
-			TempTable  *t = lfirst(l);
+	oldcxt = MemoryContextSwitchTo(CacheMemoryContext);
 
-			if (t->ateoxact == ATEOXACTDROP)
-			{
-				ObjectAddress object;
+	oc = (OnCommitItem *) palloc(sizeof(OnCommitItem));
+	oc->relid = relid;
+	oc->oncommit = action;
+	oc->created_in_cur_xact = true;
+	oc->deleted_in_cur_xact = false;
 
-				object.classId = RelOid_pg_class;
-				object.objectId = t->relid;
-				object.objectSubId = 0;
+	on_commits = lcons(oc, on_commits);
 
-				performDeletion(&object, DROP_CASCADE);
-				oldctx = MemoryContextSwitchTo(CacheMemoryContext);
+	MemoryContextSwitchTo(oldcxt);
+}
 
-				if (prev == NIL)
-				{
-					pfree(t);
-					temprels = lnext(l);
-					pfree(l);
-					l = temprels;
-				}
-				else
-				{
-					pfree(t);
-					lnext(prev) = lnext(l);
-					pfree(l);
-					l = lnext(prev);
-				}
+/*
+ * Unregister any ON COMMIT action when a relation is deleted.
+ *
+ * Actually, we only mark the OnCommitItem entry as to be deleted after commit.
+ */
+void
+remove_on_commit_action(Oid relid)
+{
+	List	   *l;
 
-				MemoryContextSwitchTo(oldctx);
-				CommandCounterIncrement();
-				continue;
-			}
-			else if (t->ateoxact == ATEOXACTDELETE)
-			{
-				heap_truncate(t->relid);
-				CommandCounterIncrement();
-			}
-			prev = l;
-			l = lnext(l);
-		}
-	}
-	else
+	foreach(l, on_commits)
 	{
-		/* Abort --- remove entries added by this xact */
-		TransactionId curtid = GetCurrentTransactionId();
-
-		oldctx = MemoryContextSwitchTo(CacheMemoryContext);
+		OnCommitItem  *oc = (OnCommitItem *) lfirst(l);
 
-		while (l != NIL)
+		if (oc->relid == relid)
 		{
-			TempTable  *t = lfirst(l);
-
-			if (t->tid == curtid)
-			{
-				if (prev == NIL)
-				{
-					pfree(t);
-					temprels = lnext(l);
-					pfree(l);
-					l = temprels;
-				}
-				else
-				{
-					pfree(t);
-					lnext(prev) = lnext(l);
-					pfree(l);
-					l = lnext(prev);
-				}
-				continue;
-			}
-			prev = l;
-			l = lnext(l);
+			oc->deleted_in_cur_xact = true;
+			break;
 		}
-		MemoryContextSwitchTo(oldctx);
 	}
 }
 
 /*
- * Register a temp rel in temprels
+ * Perform ON COMMIT actions.
+ *
+ * This is invoked just before actually committing, since it's possible
+ * to encounter errors.
  */
-
 void
-reg_temp_rel(TempTable * t)
+PreCommit_on_commit_actions(void)
 {
-	temprels = lcons(t, temprels);
-}
+	List	   *l;
 
-/*
- * return the ON COMMIT/ateoxact value for a given temp rel
- */
+	foreach(l, on_commits)
+	{
+		OnCommitItem  *oc = (OnCommitItem *) lfirst(l);
 
-void
-free_temp_rels(void)
-{
-	MemoryContext oldctx;
+		/* Ignore entry if already dropped in this xact */
+		if (oc->deleted_in_cur_xact)
+			continue;
 
-	oldctx = MemoryContextSwitchTo(CacheMemoryContext);
-	while (temprels != NIL)
-	{
-		List	   *l = temprels;
+		switch (oc->oncommit)
+		{
+			case ONCOMMIT_NOOP:
+			case ONCOMMIT_PRESERVE_ROWS:
+				/* Do nothing (there shouldn't be such entries, actually) */
+				break;
+			case ONCOMMIT_DELETE_ROWS:
+				heap_truncate(oc->relid);
+				CommandCounterIncrement(); /* XXX needed? */
+				break;
+			case ONCOMMIT_DROP:
+			{
+				ObjectAddress object;
 
-		temprels = lnext(temprels);
-		pfree(lfirst(l));
-		pfree(l);
+				object.classId = RelOid_pg_class;
+				object.objectId = oc->relid;
+				object.objectSubId = 0;
+				performDeletion(&object, DROP_CASCADE);
+				/*
+				 * Note that table deletion will call remove_on_commit_action,
+				 * so the entry should get marked as deleted.
+				 */
+				Assert(oc->deleted_in_cur_xact);
+				break;
+			}
+		}
 	}
-	MemoryContextSwitchTo(oldctx);
 }
 
 /*
- * Remove (actually just mark for deletion, in case we abort)
- * Relid from the temprels list
+ * Post-commit or post-abort cleanup for ON COMMIT management.
+ *
+ * All we do here is remove no-longer-needed OnCommitItem entries.
+ *
+ * During commit, remove entries that were deleted during this transaction;
+ * during abort, remove those created during this transaction.
  */
-
 void
-rm_temp_rel(Oid relid)
+AtEOXact_on_commit_actions(bool isCommit)
 {
-	List	   *l;
+	List	   *l,
+			   *prev;
 
-	foreach(l, temprels)
+	prev = NIL;
+	l = on_commits;
+	while (l != NIL)
 	{
-		TempTable  *t = lfirst(l);
+		OnCommitItem  *oc = (OnCommitItem *) lfirst(l);
 
-		if (t->relid == relid)
+		if (isCommit ? oc->deleted_in_cur_xact :
+			oc->created_in_cur_xact)
 		{
-			t->dead = true;
-			return;
+			/* This entry must be removed */
+			if (prev != NIL)
+			{
+				lnext(prev) = lnext(l);
+				pfree(l);
+				l = lnext(prev);
+			}
+			else
+			{
+				on_commits = lnext(l);
+				pfree(l);
+				l = on_commits;
+			}
+			pfree(oc);
+		}
+		else
+		{
+			/* This entry must be preserved */
+			oc->created_in_cur_xact = false;
+			oc->deleted_in_cur_xact = false;
+			prev = l;
+			l = lnext(l);
 		}
 	}
-
-	/* If we get here, we're in trouble */
-	Assert(1==1);
 }
-
diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c
index 3dea8bf6330..a573cac27de 100644
--- a/src/backend/commands/typecmds.c
+++ b/src/backend/commands/typecmds.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/commands/typecmds.c,v 1.15 2002/09/21 18:39:25 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/commands/typecmds.c,v 1.16 2002/11/11 22:19:22 tgl Exp $
  *
  * DESCRIPTION
  *	  The "DefineFoo" routines take the parse tree and pick out the
@@ -846,6 +846,7 @@ DefineCompositeType(const RangeVar *typevar, List *coldeflist)
 	createStmt->inhRelations = NIL;
 	createStmt->constraints = NIL;
 	createStmt->hasoids = false;
+	createStmt->oncommit = ONCOMMIT_NOOP;
 
 	/*
 	 * finally create the relation...
diff --git a/src/backend/commands/view.c b/src/backend/commands/view.c
index 17a5cdaa0d4..329fd3146bb 100644
--- a/src/backend/commands/view.c
+++ b/src/backend/commands/view.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/commands/view.c,v 1.72 2002/09/22 19:42:50 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/commands/view.c,v 1.73 2002/11/11 22:19:22 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -137,6 +137,7 @@ DefineVirtualRelation(const RangeVar *relation, List *tlist, bool replace)
 		createStmt->inhRelations = NIL;
 		createStmt->constraints = NIL;
 		createStmt->hasoids = false;
+		createStmt->oncommit = ONCOMMIT_NOOP;
 
 		/*
 		 * finally create the relation (this will error out if there's an
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index d85e59fb009..efad76fcc1c 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -27,7 +27,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/executor/execMain.c,v 1.183 2002/11/11 03:02:18 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/executor/execMain.c,v 1.184 2002/11/11 22:19:22 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -732,7 +732,7 @@ InitPlan(CmdType operation, Query *parseTree, Plan *plan, EState *estate)
 											 tupdesc,
 											 RELKIND_RELATION,
 											 false,
-											 ATEOXACTNOOP,
+											 ONCOMMIT_NOOP,
 											 allowSystemTableMods);
 
 				FreeTupleDesc(tupdesc);
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 447d5600643..c354abf5dad 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -15,7 +15,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/nodes/copyfuncs.c,v 1.216 2002/11/06 22:31:23 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/nodes/copyfuncs.c,v 1.217 2002/11/11 22:19:22 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -2037,6 +2037,7 @@ _copyCreateStmt(CreateStmt *from)
 	Node_Copy(from, newnode, inhRelations);
 	Node_Copy(from, newnode, constraints);
 	newnode->hasoids = from->hasoids;
+	newnode->oncommit = from->oncommit;
 
 	return newnode;
 }
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 122de38fed9..ab84f5d3d20 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -20,7 +20,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/nodes/equalfuncs.c,v 1.162 2002/11/06 00:00:43 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/nodes/equalfuncs.c,v 1.163 2002/11/11 22:19:22 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -840,6 +840,8 @@ _equalCreateStmt(CreateStmt *a, CreateStmt *b)
 		return false;
 	if (a->hasoids != b->hasoids)
 		return false;
+	if (a->oncommit != b->oncommit)
+		return false;
 
 	return true;
 }
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index b35763f23da..c5b5a493583 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -5,7 +5,7 @@
  * Portions Copyright (c) 1996-2002, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- *	$Header: /cvsroot/pgsql/src/backend/nodes/outfuncs.c,v 1.178 2002/11/06 22:31:24 tgl Exp $
+ *	$Header: /cvsroot/pgsql/src/backend/nodes/outfuncs.c,v 1.179 2002/11/11 22:19:22 tgl Exp $
  *
  * NOTES
  *	  Every (plan) node in POSTGRES has an associated "out" routine which
@@ -117,8 +117,9 @@ _outCreateStmt(StringInfo str, CreateStmt *node)
 	appendStringInfo(str, " :constraints ");
 	_outNode(str, node->constraints);
 
-	appendStringInfo(str, " :hasoids %s ",
-					 booltostr(node->hasoids));
+	appendStringInfo(str, " :hasoids %s :oncommit %d ",
+					 booltostr(node->hasoids),
+					 (int) node->oncommit);
 }
 
 static void
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 1ce4cc1bfde..96d12f1b560 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -11,7 +11,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/parser/gram.y,v 2.375 2002/11/10 00:10:20 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/parser/gram.y,v 2.376 2002/11/11 22:19:23 tgl Exp $
  *
  * HISTORY
  *	  AUTHOR			DATE			MAJOR EVENT
@@ -54,7 +54,6 @@
 #include "catalog/index.h"
 #include "catalog/namespace.h"
 #include "catalog/pg_type.h"
-#include "commands/tablecmds.h"
 #include "nodes/makefuncs.h"
 #include "nodes/params.h"
 #include "nodes/parsenodes.h"
@@ -106,6 +105,7 @@ static void doNegateFloat(Value *v);
 	bool				boolean;
 	JoinType			jtype;
 	DropBehavior		dbehavior;
+	OnCommitAction		oncommit;
 	List				*list;
 	Node				*node;
 	Value				*value;
@@ -225,7 +225,7 @@ static void doNegateFloat(Value *v);
 %type <typnam>	func_arg func_return func_type aggr_argtype
 
 %type <boolean> opt_arg TriggerForType OptTemp OptWithOids
-%type <chr>		OptEOXact
+%type <oncommit>	OnCommitOption
 
 %type <list>	for_update_clause opt_for_update_clause update_list
 %type <boolean>	opt_all
@@ -1375,24 +1375,20 @@ opt_using:
  *****************************************************************************/
 
 CreateStmt:	CREATE OptTemp TABLE qualified_name '(' OptTableElementList ')'
-			OptInherit OptWithOids OptEOXact
+			OptInherit OptWithOids OnCommitOption
 				{
 					CreateStmt *n = makeNode(CreateStmt);
-
-					if($2 == FALSE && $10 != ATEOXACTNOOP)
-						elog(ERROR,"ON COMMIT can only be used on TEMP tables"); 
-
 					$4->istemp = $2;
 					n->relation = $4;
 					n->tableElts = $6;
 					n->inhRelations = $8;
 					n->constraints = NIL;
 					n->hasoids = $9;
-					n->ateoxact = $10;
+					n->oncommit = $10;
 					$$ = (Node *)n;
 				}
 		| CREATE OptTemp TABLE qualified_name OF qualified_name
-			'(' OptTableElementList ')' OptWithOids
+			'(' OptTableElementList ')' OptWithOids OnCommitOption
 				{
 					/* SQL99 CREATE TABLE OF <UDT> (cols) seems to be satisfied
 					 * by our inheritance capabilities. Let's try it...
@@ -1404,6 +1400,7 @@ CreateStmt:	CREATE OptTemp TABLE qualified_name '(' OptTableElementList ')'
 					n->inhRelations = makeList1($6);
 					n->constraints = NIL;
 					n->hasoids = $10;
+					n->oncommit = $11;
 					$$ = (Node *)n;
 				}
 		;
@@ -1807,11 +1804,13 @@ OptWithOids:
 			| /*EMPTY*/								{ $$ = TRUE; }
 		;
 
-OptEOXact:    ON COMMIT DROP       			{ $$ = ATEOXACTDROP; }
-			| ON COMMIT DELETE_P ROWS 		{ $$ = ATEOXACTDELETE; }
-			| ON COMMIT PRESERVE ROWS		{ $$ = ATEOXACTPRESERVE; }
-			| /*EMPTY*/         { $$ = ATEOXACTNOOP; }
-       ;
+OnCommitOption:  ON COMMIT DROP				{ $$ = ONCOMMIT_DROP; }
+			| ON COMMIT DELETE_P ROWS		{ $$ = ONCOMMIT_DELETE_ROWS; }
+			| ON COMMIT PRESERVE ROWS		{ $$ = ONCOMMIT_PRESERVE_ROWS; }
+			| /*EMPTY*/						{ $$ = ONCOMMIT_NOOP; }
+		;
+
+
 /*
  * Note: CREATE TABLE ... AS SELECT ... is just another spelling for
  * SELECT ... INTO.
diff --git a/src/backend/parser/keywords.c b/src/backend/parser/keywords.c
index 6c15cc50801..1c9c064e474 100644
--- a/src/backend/parser/keywords.c
+++ b/src/backend/parser/keywords.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/parser/keywords.c,v 1.128 2002/11/09 23:56:39 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/parser/keywords.c,v 1.129 2002/11/11 22:19:23 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -253,7 +253,7 @@ static const ScanKeyword ScanKeywords[] = {
 	{"right", RIGHT},
 	{"rollback", ROLLBACK},
 	{"row", ROW},
-	{"rows",ROWS},
+	{"rows", ROWS},
 	{"rule", RULE},
 	{"schema", SCHEMA},
 	{"scroll", SCROLL},
diff --git a/src/include/catalog/heap.h b/src/include/catalog/heap.h
index 74abf47bc77..677347c24f5 100644
--- a/src/include/catalog/heap.h
+++ b/src/include/catalog/heap.h
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2002, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: heap.h,v 1.58 2002/11/09 23:56:39 momjian Exp $
+ * $Id: heap.h,v 1.59 2002/11/11 22:19:23 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -41,7 +41,7 @@ extern Oid heap_create_with_catalog(const char *relname,
 						 TupleDesc tupdesc,
 						 char relkind,
 						 bool shared_relation,
-						 char ateoxact, 
+						 OnCommitAction oncommit,
 						 bool allow_system_table_mods);
 
 extern void heap_drop_with_catalog(Oid rid);
diff --git a/src/include/commands/tablecmds.h b/src/include/commands/tablecmds.h
index 329f0989dd9..13873dad1b5 100644
--- a/src/include/commands/tablecmds.h
+++ b/src/include/commands/tablecmds.h
@@ -7,14 +7,13 @@
  * Portions Copyright (c) 1996-2002, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: tablecmds.h,v 1.9 2002/11/09 23:56:39 momjian Exp $
+ * $Id: tablecmds.h,v 1.10 2002/11/11 22:19:24 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
 #ifndef TABLECMDS_H
 #define TABLECMDS_H
 
-#include "access/htup.h"
 #include "nodes/parsenodes.h"
 
 extern void AlterTableAddColumn(Oid myrelid, bool recurse, ColumnDef *colDef);
@@ -63,29 +62,10 @@ extern void renameatt(Oid myrelid,
 extern void renamerel(Oid myrelid,
 		  const char *newrelname);
 
-/*
- *  Temp rel stuff
- */
-typedef struct TempTable
-{
-	Oid				relid;			/* relid of temp relation */
-	char 			ateoxact;		/* what to do at end of xact */
-	TransactionId	tid;			/* trans id where in rel was created */
-	bool			dead;			/* table was dropped in the current xact */
-} TempTable;
-
-extern void AtEOXact_temp_relations(bool iscommit, int bstate);
-extern void reg_temp_rel(TempTable *t);
-extern void free_temp_rels(void);
-extern void rm_temp_rel(Oid relid);
-
-/*
- *  What to do at commit time for temporary relations
- */
+extern void register_on_commit_action(Oid relid, OnCommitAction action);
+extern void remove_on_commit_action(Oid relid);
 
-#define ATEOXACTNOOP		0 		/* no operation at commit */
-#define ATEOXACTPRESERVE	1		/* preserve rows */
-#define ATEOXACTDELETE		2		/* delete rows */
-#define ATEOXACTDROP		3		/* drop temp table */
+extern void PreCommit_on_commit_actions(void);
+extern void AtEOXact_on_commit_actions(bool isCommit);
 
 #endif   /* TABLECMDS_H */
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 210acd86edd..7554f00361d 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2002, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: parsenodes.h,v 1.211 2002/11/09 23:56:39 momjian Exp $
+ * $Id: parsenodes.h,v 1.212 2002/11/11 22:19:24 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -911,6 +911,16 @@ typedef struct CopyStmt
  * implementation).
  * ----------------------
  */
+
+/* What to do at commit time for temporary relations */
+typedef enum OnCommitAction
+{
+	ONCOMMIT_NOOP,				/* No ON COMMIT clause (do nothing) */
+	ONCOMMIT_PRESERVE_ROWS,		/* ON COMMIT PRESERVE ROWS (do nothing) */
+	ONCOMMIT_DELETE_ROWS,		/* ON COMMIT DELETE ROWS */
+	ONCOMMIT_DROP				/* ON COMMIT DROP */
+} OnCommitAction;
+
 typedef struct CreateStmt
 {
 	NodeTag		type;
@@ -919,7 +929,7 @@ typedef struct CreateStmt
 	List	   *inhRelations;	/* relations to inherit from */
 	List	   *constraints;	/* constraints (list of Constraint nodes) */
 	bool		hasoids;		/* should it have OIDs? */
-	char		ateoxact;		/* what do we do at COMMIT for TEMP ? */
+	OnCommitAction oncommit;	/* what do we do at COMMIT? */
 } CreateStmt;
 
 /* ----------
diff --git a/src/test/regress/expected/temp.out b/src/test/regress/expected/temp.out
index 1800b9ebe56..75d758d36e3 100644
--- a/src/test/regress/expected/temp.out
+++ b/src/test/regress/expected/temp.out
@@ -5,21 +5,31 @@
 -- test temp table/index masking
 CREATE TABLE temptest(col int);
 CREATE INDEX i_temptest ON temptest(col);
-CREATE TEMP TABLE temptest(col int);
-CREATE INDEX i_temptest ON temptest(col);
+CREATE TEMP TABLE temptest(tcol int);
+CREATE INDEX i_temptest ON temptest(tcol);
+SELECT * FROM temptest;
+ tcol 
+------
+(0 rows)
+
 DROP INDEX i_temptest;
 DROP TABLE temptest;
+SELECT * FROM temptest;
+ col 
+-----
+(0 rows)
+
 DROP INDEX i_temptest;
 DROP TABLE temptest;
 -- test temp table selects
 CREATE TABLE temptest(col int);
 INSERT INTO temptest VALUES (1);
-CREATE TEMP TABLE temptest(col int);
-INSERT INTO temptest VALUES (2);
+CREATE TEMP TABLE temptest(tcol float);
+INSERT INTO temptest VALUES (2.1);
 SELECT * FROM temptest;
- col 
------
-   2
+ tcol 
+------
+  2.1
 (1 row)
 
 DROP TABLE temptest;
@@ -30,9 +40,46 @@ SELECT * FROM temptest;
 (1 row)
 
 DROP TABLE temptest;
-CREATE TEMP TABLE temptest(col int);
 -- test temp table deletion
+CREATE TEMP TABLE temptest(col int);
 \c regression
 SET autocommit TO 'on';
 SELECT * FROM temptest;
 ERROR:  Relation "temptest" does not exist
+-- Test ON COMMIT DELETE ROWS
+CREATE TEMP TABLE temptest(col int) ON COMMIT DELETE ROWS;
+BEGIN;
+INSERT INTO temptest VALUES (1);
+INSERT INTO temptest VALUES (2);
+SELECT * FROM temptest;
+ col 
+-----
+   1
+   2
+(2 rows)
+
+COMMIT;
+SELECT * FROM temptest;
+ col 
+-----
+(0 rows)
+
+DROP TABLE temptest;
+-- Test ON COMMIT DROP
+BEGIN;
+CREATE TEMP TABLE temptest(col int) ON COMMIT DROP;
+INSERT INTO temptest VALUES (1);
+INSERT INTO temptest VALUES (2);
+SELECT * FROM temptest;
+ col 
+-----
+   1
+   2
+(2 rows)
+
+COMMIT;
+SELECT * FROM temptest;
+ERROR:  Relation "temptest" does not exist
+-- ON COMMIT is only allowed for TEMP
+CREATE TABLE temptest(col int) ON COMMIT DELETE ROWS;
+ERROR:  ON COMMIT can only be used on TEMP tables
diff --git a/src/test/regress/sql/temp.sql b/src/test/regress/sql/temp.sql
index 6f45b2eaac7..5ebd13c83cc 100644
--- a/src/test/regress/sql/temp.sql
+++ b/src/test/regress/sql/temp.sql
@@ -9,14 +9,18 @@ CREATE TABLE temptest(col int);
 
 CREATE INDEX i_temptest ON temptest(col);
 
-CREATE TEMP TABLE temptest(col int);
+CREATE TEMP TABLE temptest(tcol int);
 
-CREATE INDEX i_temptest ON temptest(col);
+CREATE INDEX i_temptest ON temptest(tcol);
+
+SELECT * FROM temptest;
 
 DROP INDEX i_temptest;
 
 DROP TABLE temptest;
 
+SELECT * FROM temptest;
+
 DROP INDEX i_temptest;
 
 DROP TABLE temptest;
@@ -27,9 +31,9 @@ CREATE TABLE temptest(col int);
 
 INSERT INTO temptest VALUES (1);
 
-CREATE TEMP TABLE temptest(col int);
+CREATE TEMP TABLE temptest(tcol float);
 
-INSERT INTO temptest VALUES (2);
+INSERT INTO temptest VALUES (2.1);
 
 SELECT * FROM temptest;
 
@@ -39,12 +43,44 @@ SELECT * FROM temptest;
 
 DROP TABLE temptest;
 
-CREATE TEMP TABLE temptest(col int);
-
 -- test temp table deletion
 
+CREATE TEMP TABLE temptest(col int);
+
 \c regression
 SET autocommit TO 'on';
 
 SELECT * FROM temptest;
 
+-- Test ON COMMIT DELETE ROWS
+
+CREATE TEMP TABLE temptest(col int) ON COMMIT DELETE ROWS;
+
+BEGIN;
+INSERT INTO temptest VALUES (1);
+INSERT INTO temptest VALUES (2);
+
+SELECT * FROM temptest;
+COMMIT;
+
+SELECT * FROM temptest;
+
+DROP TABLE temptest;
+
+-- Test ON COMMIT DROP
+
+BEGIN;
+
+CREATE TEMP TABLE temptest(col int) ON COMMIT DROP;
+
+INSERT INTO temptest VALUES (1);
+INSERT INTO temptest VALUES (2);
+
+SELECT * FROM temptest;
+COMMIT;
+
+SELECT * FROM temptest;
+
+-- ON COMMIT is only allowed for TEMP
+
+CREATE TABLE temptest(col int) ON COMMIT DELETE ROWS;