Fix relcache to account properly for subtransaction status of 'new'

relcache entries.  Also, change TransactionIdIsCurrentTransactionId()
so that if consulted during transaction abort, it will not say that
the aborted xact is still current.  (It would be better to ensure that
it's never called at all during abort, but I'm not sure we can easily
guarantee that.)  In combination, these fix a crash we have seen
occasionally during parallel regression tests of 8.0.

src/backend/access/hash/hashpage.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/access/hash/hashpage.c,v 1.43 2003/11/29 19:51:40 pgsql Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/hash/hashpage.c,v 1.44 2004/08/28 20:31:43 tgl Exp $
  *
  * NOTES
  *	  Postgres hash pages look like ordinary relation pages.  The opaque
@@ -44,14 +44,12 @@ static void _hash_splitbucket(Relation rel, Buffer metabuf,
 
 /*
  * We use high-concurrency locking on hash indexes (see README for an overview
- * of the locking rules).  There are two cases in which we don't do locking.
- * One is when the index is newly created in the current transaction.  Since
- * the creating transaction has not committed, no one else can see the index,
- * and there's no reason to take locks.  The second case is for temp
- * relations, which no one else can see either.  (We still take buffer-level
- * locks, but not lmgr locks.)
- */
-#define USELOCKING(rel)		(!((rel)->rd_isnew || (rel)->rd_istemp))
+ * of the locking rules).  However, we can skip taking lmgr locks when the
+ * index is local to the current backend (ie, either temp or new in the
+ * current transaction).  No one else can see it, so there's no reason to
+ * take locks.  We still take buffer-level locks, but not lmgr locks.
+ */
+#define USELOCKING(rel)		(!RELATION_IS_LOCAL(rel))
 
 
 /*

src/backend/access/heap/hio.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/access/heap/hio.c,v 1.51 2003/11/29 22:39:39 pgsql Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/heap/hio.c,v 1.52 2004/08/28 20:31:43 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -232,7 +232,7 @@ RelationGetBufferForTuple(Relation relation, Size len,
 	 * page.  We can skip locking for new or temp relations, however,
 	 * since no one else could be accessing them.
 	 */
-	needLock = !(relation->rd_isnew || relation->rd_istemp);
+	needLock = !RELATION_IS_LOCAL(relation);
 
 	if (needLock)
 		LockPage(relation, 0, ExclusiveLock);

src/backend/access/nbtree/nbtpage.c

@@ -9,7 +9,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/access/nbtree/nbtpage.c,v 1.77 2004/07/21 22:31:20 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/nbtree/nbtpage.c,v 1.78 2004/08/28 20:31:43 tgl Exp $
  *
  *	NOTES
  *	   Postgres btree pages look like ordinary relation pages.	The opaque
@@ -483,7 +483,7 @@ _bt_getbuf(Relation rel, BlockNumber blkno, int access)
 		 * new page.  We can skip locking for new or temp relations,
 		 * however, since no one else could be accessing them.
 		 */
-		needLock = !(rel->rd_isnew || rel->rd_istemp);
+		needLock = !RELATION_IS_LOCAL(rel);
 
 		if (needLock)
 			LockPage(rel, 0, ExclusiveLock);

src/backend/access/transam/xact.c

@@ -10,7 +10,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.179 2004/08/25 18:43:42 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.180 2004/08/28 20:31:43 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -40,6 +40,7 @@
 #include "utils/inval.h"
 #include "utils/memutils.h"
 #include "utils/portal.h"
+#include "utils/relcache.h"
 #include "utils/resowner.h"
 #include "pgstat.h"
 
@@ -352,7 +353,7 @@ GetCurrentTransactionNestLevel(void)
 bool
 TransactionIdIsCurrentTransactionId(TransactionId xid)
 {
-	TransactionState s = CurrentTransactionState;
+	TransactionState s;
 
 	if (AMI_OVERRIDE)
 	{
@@ -363,12 +364,16 @@ TransactionIdIsCurrentTransactionId(TransactionId xid)
 	/*
 	 * We will return true for the Xid of the current subtransaction,
 	 * any of its subcommitted children, any of its parents, or any of
-	 * their previously subcommitted children.
+	 * their previously subcommitted children.  However, a transaction
+	 * being aborted is no longer "current", even though it may still
+	 * have an entry on the state stack.
 	 */
-	while (s != NULL)
+	for (s = CurrentTransactionState; s != NULL; s = s->parent)
 	{
 		ListCell *cell;
 
+		if (s->state == TRANS_ABORT)
+			continue;
 		if (TransactionIdEquals(xid, s->transactionIdData))
 			return true;
 		foreach(cell, s->childXids)
@@ -376,8 +381,6 @@ TransactionIdIsCurrentTransactionId(TransactionId xid)
 			if (TransactionIdEquals(xid, lfirst_xid(cell)))
 				return true;
 		}
-
-		s = s->parent;
 	}
 
 	return false;
@@ -2997,6 +3000,8 @@ CommitSubTransaction(void)
 	ResourceOwnerRelease(s->curTransactionOwner,
 						 RESOURCE_RELEASE_BEFORE_LOCKS,
 						 true, false);
+	AtEOSubXact_RelationCache(true, s->transactionIdData,
+							  s->parent->transactionIdData);
 	AtEOSubXact_Inval(true);
 	ResourceOwnerRelease(s->curTransactionOwner,
 						 RESOURCE_RELEASE_LOCKS,
@@ -3090,6 +3095,8 @@ AbortSubTransaction(void)
 	ResourceOwnerRelease(s->curTransactionOwner,
 						 RESOURCE_RELEASE_BEFORE_LOCKS,
 						 false, false);
+	AtEOSubXact_RelationCache(false, s->transactionIdData,
+							  s->parent->transactionIdData);
 	AtEOSubXact_Inval(false);
 	ResourceOwnerRelease(s->curTransactionOwner,
 						 RESOURCE_RELEASE_LOCKS,

src/backend/utils/cache/relcache.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/cache/relcache.c,v 1.207 2004/07/17 03:29:25 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/cache/relcache.c,v 1.208 2004/08/28 20:31:44 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -72,7 +72,7 @@
  */
 #define RELCACHE_INIT_FILENAME	"pg_internal.init"
 
-#define RELCACHE_INIT_FILEMAGIC		0x573261 /* version ID value */
+#define RELCACHE_INIT_FILEMAGIC		0x573262 /* version ID value */
 
 /*
  *		hardcoded tuple descriptors.  see include/catalog/pg_attribute.h
@@ -835,8 +835,8 @@ RelationBuildDesc(RelationBuildDescInfo buildinfo,
 	 * flush the entry.)
 	 */
 	relation->rd_refcnt = 0;
-	relation->rd_isnailed = 0;
-	relation->rd_isnew = false;
+	relation->rd_isnailed = false;
+	relation->rd_createxact = InvalidTransactionId;
 	relation->rd_istemp = isTempNamespace(relation->rd_rel->relnamespace);
 
 	/*
@@ -886,6 +886,9 @@ RelationBuildDesc(RelationBuildDescInfo buildinfo,
 	RelationCacheInsert(relation);
 	MemoryContextSwitchTo(oldcxt);
 
+	/* It's fully valid */
+	relation->rd_isvalid = true;
+
 	return relation;
 }
 
@@ -1283,8 +1286,8 @@ formrdesc(const char *relationName,
 	 * all entries built with this routine are nailed-in-cache; none are
 	 * for new or temp relations.
 	 */
-	relation->rd_isnailed = 1;
-	relation->rd_isnew = false;
+	relation->rd_isnailed = true;
+	relation->rd_createxact = InvalidTransactionId;
 	relation->rd_istemp = false;
 
 	/*
@@ -1385,6 +1388,9 @@ formrdesc(const char *relationName,
 	 * add new reldesc to relcache
 	 */
 	RelationCacheInsert(relation);
+
+	/* It's fully valid */
+	relation->rd_isvalid = true;
 }
 
 
@@ -1416,7 +1422,7 @@ RelationIdCacheGetRelation(Oid relationId)
 	{
 		RelationIncrementReferenceCount(rd);
 		/* revalidate nailed index if necessary */
-		if (rd->rd_isnailed == 2)
+		if (!rd->rd_isvalid)
 			RelationReloadClassinfo(rd);
 	}
 
@@ -1445,7 +1451,7 @@ RelationSysNameCacheGetRelation(const char *relationName)
 	{
 		RelationIncrementReferenceCount(rd);
 		/* revalidate nailed index if necessary */
-		if (rd->rd_isnailed == 2)
+		if (!rd->rd_isvalid)
 			RelationReloadClassinfo(rd);
 	}
 
@@ -1572,7 +1578,7 @@ RelationClose(Relation relation)
 
 #ifdef RELCACHE_FORCE_RELEASE
 	if (RelationHasReferenceCountZero(relation) &&
-		!relation->rd_isnew)
+		!TransactionIdIsValid(relation->rd_createxact))
 		RelationClearRelation(relation, false);
 #endif
 }
@@ -1589,7 +1595,7 @@ RelationClose(Relation relation)
  *	We can't necessarily reread the pg_class row right away; we might be
  *	in a failed transaction when we receive the SI notification.  If so,
  *	RelationClearRelation just marks the entry as invalid by setting
- *	rd_isnailed to 2.  This routine is called to fix the entry when it
+ *	rd_isvalid to false.  This routine is called to fix the entry when it
  *	is next needed.
  */
 static void
@@ -1601,7 +1607,7 @@ RelationReloadClassinfo(Relation relation)
 	Form_pg_class relp;
 
 	/* Should be called only for invalidated nailed indexes */
-	Assert(relation->rd_isnailed == 2 &&
+	Assert(relation->rd_isnailed && !relation->rd_isvalid &&
 		   relation->rd_rel->relkind == RELKIND_INDEX);
 	/* Read the pg_class row */
 	buildinfo.infotype = INFO_RELID;
@@ -1622,7 +1628,7 @@ RelationReloadClassinfo(Relation relation)
 	heap_freetuple(pg_class_tuple);
 	relation->rd_targblock = InvalidBlockNumber;
 	/* Okay, now it's valid again */
-	relation->rd_isnailed = 1;
+	relation->rd_isvalid = true;
 }
 
 /*
@@ -1671,7 +1677,7 @@ RelationClearRelation(Relation relation, bool rebuild)
 		relation->rd_targblock = InvalidBlockNumber;
 		if (relation->rd_rel->relkind == RELKIND_INDEX)
 		{
-			relation->rd_isnailed = 2;	/* needs to be revalidated */
+			relation->rd_isvalid = false;	/* needs to be revalidated */
 			if (relation->rd_refcnt > 1)
 				RelationReloadClassinfo(relation);
 		}
@@ -1729,15 +1735,15 @@ RelationClearRelation(Relation relation, bool rebuild)
 	{
 		/*
 		 * When rebuilding an open relcache entry, must preserve ref count
-		 * and rd_isnew flag.  Also attempt to preserve the tupledesc and
-		 * rewrite-rule substructures in place.
+		 * and rd_createxact state.  Also attempt to preserve the tupledesc
+		 * and rewrite-rule substructures in place.
 		 *
 		 * Note that this process does not touch CurrentResourceOwner;
 		 * which is good because whatever ref counts the entry may have
 		 * do not necessarily belong to that resource owner.
 		 */
 		int			old_refcnt = relation->rd_refcnt;
-		bool		old_isnew = relation->rd_isnew;
+		TransactionId old_createxact = relation->rd_createxact;
 		TupleDesc	old_att = relation->rd_att;
 		RuleLock   *old_rules = relation->rd_rules;
 		MemoryContext old_rulescxt = relation->rd_rulescxt;
@@ -1758,7 +1764,7 @@ RelationClearRelation(Relation relation, bool rebuild)
 				 buildinfo.i.info_id);
 		}
 		relation->rd_refcnt = old_refcnt;
-		relation->rd_isnew = old_isnew;
+		relation->rd_createxact = old_createxact;
 		if (equalTupleDescs(old_att, relation->rd_att))
 		{
 			/* needn't flush typcache here */
@@ -1795,7 +1801,7 @@ RelationFlushRelation(Relation relation)
 {
 	bool		rebuild;
 
-	if (relation->rd_isnew)
+	if (TransactionIdIsValid(relation->rd_createxact))
 	{
 		/*
 		 * New relcache entries are always rebuilt, not flushed; else we'd
@@ -1941,7 +1947,7 @@ RelationCacheInvalidate(void)
 		}
 
 		/* Ignore new relations, since they are never SI targets */
-		if (relation->rd_isnew)
+		if (TransactionIdIsValid(relation->rd_createxact))
 			continue;
 
 		relcacheInvalsReceived++;
@@ -2018,18 +2024,18 @@ AtEOXact_RelationCache(bool isCommit)
 		/*
 		 * Is it a relation created in the current transaction?
 		 *
-		 * During commit, reset the flag to false, since we are now out of
+		 * During commit, reset the flag to zero, since we are now out of
 		 * the creating transaction.  During abort, simply delete the
 		 * relcache entry --- it isn't interesting any longer.  (NOTE: if
-		 * we have forgotten the isnew state of a new relation due to a
+		 * we have forgotten the new-ness of a new relation due to a
 		 * forced cache flush, the entry will get deleted anyway by
 		 * shared-cache-inval processing of the aborted pg_class
 		 * insertion.)
 		 */
-		if (relation->rd_isnew)
+		if (TransactionIdIsValid(relation->rd_createxact))
 		{
 			if (isCommit)
-				relation->rd_isnew = false;
+				relation->rd_createxact = InvalidTransactionId;
 			else
 			{
 				RelationClearRelation(relation, false);
@@ -2083,6 +2089,56 @@ AtEOXact_RelationCache(bool isCommit)
 	}
 }
 
+/*
+ * AtEOSubXact_RelationCache
+ *
+ *	Clean up the relcache at sub-transaction commit or abort.
+ *
+ * Note: this must be called *before* processing invalidation messages.
+ */
+void
+AtEOSubXact_RelationCache(bool isCommit, TransactionId myXid,
+						  TransactionId parentXid)
+{
+	HASH_SEQ_STATUS status;
+	RelIdCacheEnt *idhentry;
+
+	hash_seq_init(&status, RelationIdCache);
+
+	while ((idhentry = (RelIdCacheEnt *) hash_seq_search(&status)) != NULL)
+	{
+		Relation	relation = idhentry->reldesc;
+
+		/*
+		 * Is it a relation created in the current subtransaction?
+		 *
+		 * During subcommit, mark it as belonging to the parent, instead.
+		 * During subabort, simply delete the relcache entry.
+		 */
+		if (TransactionIdEquals(relation->rd_createxact, myXid))
+		{
+			if (isCommit)
+				relation->rd_createxact = parentXid;
+			else
+			{
+				Assert(RelationHasReferenceCountZero(relation));
+				RelationClearRelation(relation, false);
+				continue;
+			}
+		}
+
+		/*
+		 * Flush any temporary index list.
+		 */
+		if (relation->rd_indexvalid == 2)
+		{
+			list_free(relation->rd_indexlist);
+			relation->rd_indexlist = NIL;
+			relation->rd_indexvalid = 0;
+		}
+	}
+}
+
 /*
  *		RelationBuildLocalRelation
  *			Build a relcache entry for an about-to-be-created relation,
@@ -2126,7 +2182,7 @@ RelationBuildLocalRelation(const char *relname,
 	rel->rd_refcnt = nailit ? 1 : 0;
 
 	/* it's being created in this transaction */
-	rel->rd_isnew = true;
+	rel->rd_createxact = GetCurrentTransactionId();
 
 	/* is it a temporary relation? */
 	rel->rd_istemp = isTempNamespace(relnamespace);
@@ -2137,7 +2193,7 @@ RelationBuildLocalRelation(const char *relname,
 	 * want it kicked out.	e.g. pg_attribute!!!
 	 */
 	if (nailit)
-		rel->rd_isnailed = 1;
+		rel->rd_isnailed = true;
 
 	/*
 	 * create a new tuple descriptor from the one passed in.  We do this
@@ -2205,6 +2261,9 @@ RelationBuildLocalRelation(const char *relname,
 	 */
 	MemoryContextSwitchTo(oldcxt);
 
+	/* It's fully valid */
+	rel->rd_isvalid = true;
+
 	/*
 	 * Caller expects us to pin the returned entry.
 	 */
@@ -2326,7 +2385,7 @@ RelationCacheInitializePhase2(void)
 			buildinfo.infotype = INFO_RELNAME; \
 			buildinfo.i.info_name = (indname); \
 			ird = RelationBuildDesc(buildinfo, NULL); \
-			ird->rd_isnailed = 1; \
+			ird->rd_isnailed = true; \
 			ird->rd_refcnt = 1; \
 		} while (0)
 
@@ -2677,7 +2736,7 @@ RelationSetIndexList(Relation relation, List *indexIds)
 {
 	MemoryContext oldcxt;
 
-	Assert(relation->rd_isnailed == 1);
+	Assert(relation->rd_isnailed);
 	/* Copy the list into the cache context (could fail for lack of mem) */
 	oldcxt = MemoryContextSwitchTo(CacheMemoryContext);
 	indexIds = list_copy(indexIds);

src/include/utils/rel.h

@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2003, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/utils/rel.h,v 1.76 2004/07/17 03:31:47 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/utils/rel.h,v 1.77 2004/08/28 20:31:44 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -110,17 +110,18 @@ typedef struct RelationData
 	BlockNumber rd_targblock;	/* current insertion target block, or
 								 * InvalidBlockNumber */
 	int			rd_refcnt;		/* reference count */
-	bool		rd_isnew;		/* rel was created in current xact */
-
-	/*
-	 * NOTE: rd_isnew should be relied on only for optimization purposes;
-	 * it is possible for new-ness to be "forgotten" (eg, after CLUSTER).
-	 */
 	bool		rd_istemp;		/* rel uses the local buffer mgr */
-	char		rd_isnailed;	/* rel is nailed in cache: 0 = no, 1 = yes,
-								 * 2 = yes but possibly invalid */
+	bool		rd_isnailed;	/* rel is nailed in cache */
+	bool		rd_isvalid;		/* relcache entry is valid */
 	char		rd_indexvalid;	/* state of rd_indexlist: 0 = not valid,
 								 * 1 = valid, 2 = temporarily forced */
+	TransactionId rd_createxact; /* rel was created in current xact */
+	/*
+	 * rd_createxact is the XID of the highest subtransaction the rel has
+	 * survived into; or zero if the rel was not created in the current
+	 * transaction.  This should be relied on only for optimization purposes;
+	 * it is possible for new-ness to be "forgotten" (eg, after CLUSTER).
+	 */
 	Form_pg_class rd_rel;		/* RELATION tuple */
 	TupleDesc	rd_att;			/* tuple descriptor */
 	Oid			rd_id;			/* relation's object id */
@@ -230,6 +231,16 @@ typedef Relation *RelationPtr;
 #define RelationGetNamespace(relation) \
 	((relation)->rd_rel->relnamespace)
 
+/*
+ * RELATION_IS_LOCAL
+ *		If a rel is either temp or newly created in the current transaction,
+ *		it can be assumed to be visible only to the current backend.
+ *
+ * Beware of multiple eval of argument
+ */
+#define RELATION_IS_LOCAL(relation) \
+	((relation)->rd_istemp || TransactionIdIsValid((relation)->rd_createxact))
+
 /* routines in utils/cache/relcache.c */
 extern void RelationIncrementReferenceCount(Relation rel);
 extern void RelationDecrementReferenceCount(Relation rel);

src/include/utils/relcache.h

@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2003, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/utils/relcache.h,v 1.42 2004/07/17 03:31:47 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/utils/relcache.h,v 1.43 2004/08/28 20:31:44 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -66,6 +66,8 @@ extern void RelationCacheInvalidateEntry(Oid relationId, RelFileNode *rnode);
 extern void RelationCacheInvalidate(void);
 
 extern void AtEOXact_RelationCache(bool isCommit);
+extern void AtEOSubXact_RelationCache(bool isCommit, TransactionId myXid,
+									  TransactionId parentXid);
 
 /*
  * Routines to help manage rebuilding of relcache init file