Mega-commit to make heap_open/heap_openr/heap_close take an

additional argument specifying the kind of lock to acquire/release (or 'NoLock' to do no lock processing). Ensure that all relations are locked with some appropriate lock level before being examined --- this ensures that relevant shared-inval messages have been processed and should prevent problems caused by concurrent VACUUM. Fix several bugs having to do with mismatched increment/decrement of relation ref count and mismatched heap_open/close (which amounts to the same thing). A bogus ref count on a relation doesn't matter much *unless* a SI Inval message happens to arrive at the wrong time, which is probably why we got away with this sloppiness for so long. Repair missing grab of AccessExclusiveLock in DROP TABLE, ALTER/RENAME TABLE, etc, as noted by Hiroshi. Recommend 'make clean all' after pulling this update; I modified the Relation struct layout slightly. Will post further discussion to pghackers list shortly.
2025-11-12 05:01:15 +03:00 · 1999-09-18 19:08:25 +00:00
parent 6c86fd5ba4
commit bd272cace6
70 changed files with 1094 additions and 1215 deletions
--- a/src/backend/utils/adt/not_in.c
+++ b/src/backend/utils/adt/not_in.c
@@ -7,7 +7,7 @@
 *
 *
 * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/utils/adt/Attic/not_in.c,v 1.20 1999/07/17 20:17:58 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/utils/adt/Attic/not_in.c,v 1.21 1999/09/18 19:07:49 tgl Exp $
 *
 *-------------------------------------------------------------------------
 */
@@ -58,12 +58,7 @@ int4notin(int32 not_in_arg, char *relation_and_attr)

 	/* Open the relation and get a relation descriptor */

-	relation_to_scan = heap_openr(relation);
-	if (!RelationIsValid(relation_to_scan))
-	{
-		elog(ERROR, "int4notin: unknown relation %s",
-			 relation);
-	}
+	relation_to_scan = heap_openr(relation, AccessShareLock);

 	/* Find the column to search */

@@ -95,7 +90,9 @@ int4notin(int32 not_in_arg, char *relation_and_attr)
 	}

 	/* close the relation */
-	heap_close(relation_to_scan);
+	heap_endscan(scan_descriptor);
+	heap_close(relation_to_scan, AccessShareLock);
+
 	return retval;
 }

--- a/src/backend/utils/adt/regproc.c
+++ b/src/backend/utils/adt/regproc.c
@@ -7,7 +7,7 @@
 *
 *
 * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/utils/adt/regproc.c,v 1.42 1999/07/17 20:17:59 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/utils/adt/regproc.c,v 1.43 1999/09/18 19:07:49 tgl Exp $
 *
 *-------------------------------------------------------------------------
 */
@@ -78,7 +78,7 @@ regprocin(char *pro_name_or_oid)
 								   (RegProcedure) F_NAMEEQ,
 								   PointerGetDatum(pro_name_or_oid));

-			hdesc = heap_openr(ProcedureRelationName);
+			hdesc = heap_openr(ProcedureRelationName, AccessShareLock);
 			idesc = index_openr(ProcedureNameIndex);

 			sd = index_beginscan(idesc, false, 1, skey);
@@ -102,6 +102,7 @@ regprocin(char *pro_name_or_oid)
 			index_endscan(sd);
 			pfree(sd);
 			index_close(idesc);
+			heap_close(hdesc, AccessShareLock);

 			if (matches > 1)
 				elog(ERROR, "There is more than one procedure named %s.\n\tSupply the pg_proc oid inside single quotes.", pro_name_or_oid);
@@ -116,13 +117,7 @@ regprocin(char *pro_name_or_oid)
 		ScanKeyData key;
 		bool		isnull;

-		proc = heap_openr(ProcedureRelationName);
-		if (!RelationIsValid(proc))
-		{
-			elog(ERROR, "regprocin: could not open %s",
-				 ProcedureRelationName);
-			return 0;
-		}
+		proc = heap_openr(ProcedureRelationName, AccessShareLock);
 		ScanKeyEntryInitialize(&key,
 							   (bits16) 0,
 							   (AttrNumber) 1,
@@ -132,8 +127,8 @@ regprocin(char *pro_name_or_oid)
 		procscan = heap_beginscan(proc, 0, SnapshotNow, 1, &key);
 		if (!HeapScanIsValid(procscan))
 		{
-			heap_close(proc);
-			elog(ERROR, "regprocin: could not being scan of %s",
+			heap_close(proc, AccessShareLock);
+			elog(ERROR, "regprocin: could not begin scan of %s",
 				 ProcedureRelationName);
 			return 0;
 		}
@@ -151,7 +146,7 @@ regprocin(char *pro_name_or_oid)
 			result = (RegProcedure) 0;

 		heap_endscan(procscan);
-		heap_close(proc);
+		heap_close(proc, AccessShareLock);
 	}

 	return (int32) result;
@@ -193,12 +188,7 @@ regprocout(RegProcedure proid)
 		HeapScanDesc procscan;
 		ScanKeyData key;

-		proc = heap_openr(ProcedureRelationName);
-		if (!RelationIsValid(proc))
-		{
-			elog(ERROR, "regprocout: could not open %s", ProcedureRelationName);
-			return 0;
-		}
+		proc = heap_openr(ProcedureRelationName, AccessShareLock);
 		ScanKeyEntryInitialize(&key,
 							   (bits16) 0,
 							   (AttrNumber) ObjectIdAttributeNumber,
@@ -208,8 +198,8 @@ regprocout(RegProcedure proid)
 		procscan = heap_beginscan(proc, 0, SnapshotNow, 1, &key);
 		if (!HeapScanIsValid(procscan))
 		{
-			heap_close(proc);
-			elog(ERROR, "regprocout: could not being scan of %s",
+			heap_close(proc, AccessShareLock);
+			elog(ERROR, "regprocout: could not begin scan of %s",
 				 ProcedureRelationName);
 			return 0;
 		}
@@ -232,8 +222,7 @@ regprocout(RegProcedure proid)
 			result[1] = '\0';
 		}
 		heap_endscan(procscan);
-		heap_close(proc);
-		return result;
+		heap_close(proc, AccessShareLock);
 	}

 	return result;
--- a/src/backend/utils/adt/selfuncs.c
+++ b/src/backend/utils/adt/selfuncs.c
@@ -10,7 +10,7 @@
 *
 *
 * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/utils/adt/selfuncs.c,v 1.40 1999/09/09 02:35:58 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/utils/adt/selfuncs.c,v 1.41 1999/09/18 19:07:49 tgl Exp $
 *
 *-------------------------------------------------------------------------
 */
@@ -598,7 +598,7 @@ getattstatistics(Oid relid, AttrNumber attnum, Oid typid, int32 typmod,
 	HeapTuple	typeTuple;
 	FmgrInfo	inputproc;

-	rel = heap_openr(StatisticRelationName);
+	rel = heap_openr(StatisticRelationName, AccessShareLock);

 	key[0].sk_argument = ObjectIdGetDatum(relid);
 	key[1].sk_argument = Int16GetDatum((int16) attnum);
@@ -609,7 +609,7 @@ getattstatistics(Oid relid, AttrNumber attnum, Oid typid, int32 typmod,
 	{
 		/* no such stats entry */
 		heap_endscan(scan);
-		heap_close(rel);
+		heap_close(rel, AccessShareLock);
 		return false;
 	}

@@ -694,7 +694,7 @@ getattstatistics(Oid relid, AttrNumber attnum, Oid typid, int32 typmod,
 	}

 	heap_endscan(scan);
-	heap_close(rel);
+	heap_close(rel, AccessShareLock);
 	return true;
 }

--- a/src/backend/utils/adt/sets.c
+++ b/src/backend/utils/adt/sets.c
@@ -9,7 +9,7 @@
 *
 *
 * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/utils/adt/Attic/sets.c,v 1.25 1999/07/17 20:18:00 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/utils/adt/Attic/sets.c,v 1.26 1999/09/18 19:07:49 tgl Exp $
 *
 *-------------------------------------------------------------------------
 */
@@ -100,8 +100,7 @@ SetDefine(char *querystr, char *typename)
 			replNull[i] = ' ';

 		/* change the pg_proc tuple */
-		procrel = heap_openr(ProcedureRelationName);
-		LockRelation(procrel, AccessExclusiveLock);
+		procrel = heap_openr(ProcedureRelationName, RowExclusiveLock);

 		tup = SearchSysCacheTuple(PROOID,
 								  ObjectIdGetDatum(setoid),
@@ -131,8 +130,7 @@ SetDefine(char *querystr, char *typename)
 			CatalogIndexInsert(idescs, Num_pg_proc_indices, procrel, newtup);
 			CatalogCloseIndices(Num_pg_proc_indices, idescs);
 		}
-		UnlockRelation(procrel, AccessExclusiveLock);
-		heap_close(procrel);
+		heap_close(procrel, RowExclusiveLock);
 	}
 	return setoid;
 }
--- a/src/backend/utils/cache/catcache.c
+++ b/src/backend/utils/cache/catcache.c
@@ -7,7 +7,7 @@
 *
 *
 * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/utils/cache/catcache.c,v 1.48 1999/07/17 20:18:01 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/utils/cache/catcache.c,v 1.49 1999/09/18 19:07:55 tgl Exp $
 *
 *-------------------------------------------------------------------------
 */
@@ -130,7 +130,8 @@ CatalogCacheInitializeCache(struct catcache * cache,
 	/* ----------------
 	 *	If no relation was passed we must open it to get access to
 	 *	its fields.  If one of the other caches has already opened
-	 *	it we use heap_open() instead of heap_openr()
+	 *	it we use heap_open() instead of heap_openr().
+	 *	XXX is that really worth the trouble of checking?
 	 * ----------------
 	 */
 	if (!RelationIsValid(relation))
@@ -155,9 +156,9 @@ CatalogCacheInitializeCache(struct catcache * cache,
 		 * ----------------
 		 */
 		if (cp)
-			relation = heap_open(cp->relationId);
+			relation = heap_open(cp->relationId, NoLock);
 		else
-			relation = heap_openr(cache->cc_relname);
+			relation = heap_openr(cache->cc_relname, NoLock);

 		didopen = 1;
 	}
@@ -217,7 +218,7 @@ CatalogCacheInitializeCache(struct catcache * cache,
 	 * ----------------
 	 */
 	if (didopen)
-		heap_close(relation);
+		heap_close(relation, NoLock);

 	/* ----------------
 	 *	initialize index information for the cache.  this
@@ -891,10 +892,10 @@ SearchSysCache(struct catcache * cache,
 		DLMoveToFront(elt);

 #ifdef CACHEDEBUG
-		relation = heap_open(cache->relationId);
+		relation = heap_open(cache->relationId, NoLock);
 		CACHE3_elog(DEBUG, "SearchSysCache(%s): found in bucket %d",
 					RelationGetRelationName(relation), hash);
-		heap_close(relation);
+		heap_close(relation, NoLock);
 #endif	 /* CACHEDEBUG */

 		return ct->ct_tup;
@@ -925,7 +926,7 @@ SearchSysCache(struct catcache * cache,
 	 *	open the relation associated with the cache
 	 * ----------------
 	 */
-	relation = heap_open(cache->relationId);
+	relation = heap_open(cache->relationId, AccessShareLock);
 	CACHE2_elog(DEBUG, "SearchSysCache(%s)",
 				RelationGetRelationName(relation));

@@ -1082,7 +1083,7 @@ SearchSysCache(struct catcache * cache,
 	 *	and return the tuple we found (or NULL)
 	 * ----------------
 	 */
-	heap_close(relation);
+	heap_close(relation, AccessShareLock);

 	MemoryContextSwitchTo(oldcxt);
 	return ntp;
@@ -1146,8 +1147,6 @@ RelationInvalidateCatalogCacheTuple(Relation relation,
 		(*function) (ccp->id,
 				 CatalogCacheComputeTupleHashIndex(ccp, relation, tuple),
 					 &tuple->t_self);
-
-		heap_close(relation);
 	}

 	/* ----------------
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -7,7 +7,7 @@
 *
 *
 * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/utils/cache/relcache.c,v 1.72 1999/09/06 19:33:16 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/utils/cache/relcache.c,v 1.73 1999/09/18 19:07:55 tgl Exp $
 *
 *-------------------------------------------------------------------------
 */
@@ -337,7 +337,7 @@ scan_pg_rel_seq(RelationBuildDescInfo buildinfo)
 	 *	open pg_class and fetch a tuple
 	 * ----------------
 	 */
-	pg_class_desc = heap_openr(RelationRelationName);
+	pg_class_desc = heap_openr(RelationRelationName, AccessShareLock);
 	pg_class_scan = heap_beginscan(pg_class_desc, 0, SnapshotNow, 1, &key);
 	pg_class_tuple = heap_getnext(pg_class_scan, 0);

@@ -361,7 +361,7 @@ scan_pg_rel_seq(RelationBuildDescInfo buildinfo)

 	/* all done */
 	heap_endscan(pg_class_scan);
-	heap_close(pg_class_desc);
+	heap_close(pg_class_desc, AccessShareLock);

 	return return_tuple;
 }
@@ -372,9 +372,7 @@ scan_pg_rel_ind(RelationBuildDescInfo buildinfo)
 	Relation	pg_class_desc;
 	HeapTuple	return_tuple;

-	pg_class_desc = heap_openr(RelationRelationName);
-	if (!IsInitProcessingMode())
-		LockRelation(pg_class_desc, AccessShareLock);
+	pg_class_desc = heap_openr(RelationRelationName, AccessShareLock);

 	switch (buildinfo.infotype)
 	{
@@ -389,18 +387,10 @@ scan_pg_rel_ind(RelationBuildDescInfo buildinfo)

 		default:
 			elog(ERROR, "ScanPgRelation: bad buildinfo");
-
-			/*
-			 * XXX I hope this is right.  It seems better than returning
-			 * an uninitialized value
-			 */
-			return_tuple = NULL;
+			return_tuple = NULL; /* keep compiler quiet */
 	}

-	/* all done */
-	if (!IsInitProcessingMode())
-		UnlockRelation(pg_class_desc, AccessShareLock);
-	heap_close(pg_class_desc);
+	heap_close(pg_class_desc, AccessShareLock);

 	return return_tuple;
 }
@@ -508,7 +498,7 @@ build_tupdesc_seq(RelationBuildDescInfo buildinfo,
 	 *	open pg_attribute and begin a scan
 	 * ----------------
 	 */
-	pg_attribute_desc = heap_openr(AttributeRelationName);
+	pg_attribute_desc = heap_openr(AttributeRelationName, AccessShareLock);
 	pg_attribute_scan = heap_beginscan(pg_attribute_desc, 0, SnapshotNow, 1, &key);

 	/* ----------------
@@ -544,7 +534,7 @@ build_tupdesc_seq(RelationBuildDescInfo buildinfo,
 	 * ----------------
 	 */
 	heap_endscan(pg_attribute_scan);
-	heap_close(pg_attribute_desc);
+	heap_close(pg_attribute_desc, AccessShareLock);
 }

 static void
@@ -562,7 +552,7 @@ build_tupdesc_ind(RelationBuildDescInfo buildinfo,

 	constr->has_not_null = false;

-	attrel = heap_openr(AttributeRelationName);
+	attrel = heap_openr(AttributeRelationName, AccessShareLock);

 	for (i = 1; i <= relation->rd_rel->relnatts; i++)
 	{
@@ -597,7 +587,7 @@ build_tupdesc_ind(RelationBuildDescInfo buildinfo,
 		}
 	}

-	heap_close(attrel);
+	heap_close(attrel, AccessShareLock);

 	if (constr->has_not_null || ndef > 0 || relation->rd_rel->relchecks)
 	{
@@ -677,7 +667,7 @@ RelationBuildRuleLock(Relation relation)
 	 *	open pg_attribute and begin a scan
 	 * ----------------
 	 */
-	pg_rewrite_desc = heap_openr(RewriteRelationName);
+	pg_rewrite_desc = heap_openr(RewriteRelationName, AccessShareLock);
 	pg_rewrite_scan = heap_beginscan(pg_rewrite_desc, 0, SnapshotNow, 1, &key);
 	pg_rewrite_tupdesc = RelationGetDescr(pg_rewrite_desc);

@@ -732,7 +722,7 @@ RelationBuildRuleLock(Relation relation)
 	 * ----------------
 	 */
 	heap_endscan(pg_rewrite_scan);
-	heap_close(pg_rewrite_desc);
+	heap_close(pg_rewrite_desc, AccessShareLock);

 	/* ----------------
 	 *	form a RuleLock and insert into relation
@@ -765,9 +755,9 @@ RelationBuildRuleLock(Relation relation)
 *	uint16				   rd_refcnt;	 reference count
 *	Form_pg_am			   rd_am;		 AM tuple
 *	Form_pg_class		   rd_rel;		 RELATION tuple
- *	Oid					   rd_id;		 relations's object id
- *	Pointer				   lockInfo;	 ptr. to misc. info.
- *	TupleDesc			   rd_att;		 tuple desciptor
+ *	Oid					   rd_id;		 relation's object id
+ *	LockInfoData		   rd_lockInfo;	 lock manager's info
+ *	TupleDesc			   rd_att;		 tuple descriptor
 *
 *		Note: rd_ismem (rel is in-memory only) is currently unused
 *		by any part of the system.	someday this will indicate that
@@ -1048,14 +1038,18 @@ formrdesc(char *relationName,
 	 */
 	RelationGetRelid(relation) = relation->rd_att->attrs[0]->attrelid;

+	/* ----------------
+	 *	initialize the relation lock manager information
+	 * ----------------
+	 */
+	RelationInitLockInfo(relation);		/* see lmgr.c */
+
 	/* ----------------
 	 *	add new reldesc to relcache
 	 * ----------------
 	 */
 	RelationCacheInsert(relation);

-	RelationInitLockInfo(relation);
-
 	/*
 	 * Determining this requires a scan on pg_class, but to do the scan
 	 * the rdesc for pg_class must already exist.  Therefore we must do
@@ -1074,9 +1068,13 @@ formrdesc(char *relationName,
 /* --------------------------------
 *		RelationIdCacheGetRelation
 *
- *		only try to get the reldesc by looking up the cache
- *		do not go to the disk.	this is used by BlockPrepareFile()
- *		and RelationIdGetRelation below.
+ *		Lookup a reldesc by OID.
+ *		Only try to get the reldesc by looking up the cache
+ *		do not go to the disk.
+ *
+ *		NB: relation ref count is incremented if successful.
+ *		Caller should eventually decrement count.  (Usually,
+ *		that happens by calling RelationClose().)
 * --------------------------------
 */
 Relation
@@ -1103,6 +1101,8 @@ RelationIdCacheGetRelation(Oid relationId)

 /* --------------------------------
 *		RelationNameCacheGetRelation
+ *
+ *		As above, but lookup by name.
 * --------------------------------
 */
 static Relation
@@ -1136,8 +1136,11 @@ RelationNameCacheGetRelation(char *relationName)
 /* --------------------------------
 *		RelationIdGetRelation
 *
- *		return a relation descriptor based on its id.
- *		return a cached value if possible
+ *		Lookup a reldesc by OID; make one if not already in cache.
+ *
+ *		NB: relation ref count is incremented, or set to 1 if new entry.
+ *		Caller should eventually decrement count.  (Usually,
+ *		that happens by calling RelationClose().)
 * --------------------------------
 */
 Relation
@@ -1176,8 +1179,7 @@ RelationIdGetRelation(Oid relationId)
 /* --------------------------------
 *		RelationNameGetRelation
 *
- *		return a relation descriptor based on its name.
- *		return a cached value if possible
+ *		As above, but lookup by name.
 * --------------------------------
 */
 Relation
@@ -1289,7 +1291,6 @@ RelationFlushRelation(Relation *relationPtr,
 	/* Free all the subsidiary data structures of the relcache entry */
 	FreeTupleDesc(relation->rd_att);
 	FreeTriggerDesc(relation);
-	pfree(RelationGetLockInfo(relation));
 	pfree(RelationGetForm(relation));

 	/* If we're really done with the relcache entry, blow it away.
@@ -1530,10 +1531,10 @@ RelationRegisterRelation(Relation relation)
 	if (oldcxt != (MemoryContext) CacheCxt)
 		elog(NOIND, "RelationRegisterRelation: WARNING: Context != CacheCxt");

-	RelationCacheInsert(relation);
-
 	RelationInitLockInfo(relation);

+	RelationCacheInsert(relation);
+
 	/*
 	 * we've just created the relation. It is invisible to anyone else
 	 * before the transaction is committed. Setting rd_myxactonly allows
@@ -1692,7 +1693,7 @@ AttrDefaultFetch(Relation relation)
 						   (RegProcedure) F_OIDEQ,
 						   ObjectIdGetDatum(RelationGetRelid(relation)));

-	adrel = heap_openr(AttrDefaultRelationName);
+	adrel = heap_openr(AttrDefaultRelationName, AccessShareLock);
 	irel = index_openr(AttrDefaultIndex);
 	sd = index_beginscan(irel, false, 1, &skey);
 	tuple.t_data = NULL;
@@ -1754,7 +1755,7 @@ AttrDefaultFetch(Relation relation)
 	index_endscan(sd);
 	pfree(sd);
 	index_close(irel);
-	heap_close(adrel);
+	heap_close(adrel, AccessShareLock);
 }

 static void
@@ -1779,7 +1780,7 @@ RelCheckFetch(Relation relation)
 						   (RegProcedure) F_OIDEQ,
 						   ObjectIdGetDatum(RelationGetRelid(relation)));

-	rcrel = heap_openr(RelCheckRelationName);
+	rcrel = heap_openr(RelCheckRelationName, AccessShareLock);
 	irel = index_openr(RelCheckIndex);
 	sd = index_beginscan(irel, false, 1, &skey);
 	tuple.t_data = NULL;
@@ -1834,8 +1835,7 @@ RelCheckFetch(Relation relation)
 	index_endscan(sd);
 	pfree(sd);
 	index_close(irel);
-	heap_close(rcrel);
-
+	heap_close(rcrel, AccessShareLock);
 }

 /*
@@ -1929,9 +1929,6 @@ init_irels(void)
 		/* the file descriptor is not yet opened */
 		ird->rd_fd = -1;

-		/* lock info is not initialized */
-		ird->lockInfo = (char *) NULL;
-
 		/* next, read the access method tuple form */
 		if ((nread = FileRead(fd, (char *) &len, sizeof(len))) != sizeof(len))
 		{
@@ -2038,8 +2035,9 @@ init_irels(void)

 		ird->rd_support = support;

-		RelationCacheInsert(ird);
 		RelationInitLockInfo(ird);
+
+		RelationCacheInsert(ird);
 	}
 }

--- a/src/backend/utils/cache/syscache.c
+++ b/src/backend/utils/cache/syscache.c
@@ -7,7 +7,7 @@
 *
 *
 * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/utils/cache/syscache.c,v 1.35 1999/09/04 22:00:30 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/utils/cache/syscache.c,v 1.36 1999/09/18 19:07:55 tgl Exp $
 *
 * NOTES
 *	  These routines allow the parser/planner/executor to perform
@@ -575,11 +575,19 @@ SearchSysCacheGetAttribute(int cacheId,
 	Datum		attributeValue;
 	void	   *returnValue;

-	tp = SearchSysCacheTuple(cacheId, key1, key2, key3, key4);
+	/*
+	 * Open the relation first, to ensure we are in sync with SI inval
+	 * events --- we don't want the tuple found in the cache to be
+	 * invalidated out from under us.
+	 */
 	cacheName = cacheinfo[cacheId].name;
+	relation = heap_openr(cacheName, AccessShareLock);
+
+	tp = SearchSysCacheTuple(cacheId, key1, key2, key3, key4);

 	if (!HeapTupleIsValid(tp))
 	{
+		heap_close(relation, AccessShareLock);
 #ifdef	CACHEDEBUG
 		elog(DEBUG,
 			 "SearchSysCacheGetAttribute: Lookup in %s(%d) failed",
@@ -588,8 +596,6 @@ SearchSysCacheGetAttribute(int cacheId,
 		return NULL;
 	}

-	relation = heap_openr(cacheName);
-
 	if (attributeNumber < 0 &&
 		attributeNumber > FirstLowInvalidHeapAttributeNumber)
 	{
@@ -604,6 +610,7 @@ SearchSysCacheGetAttribute(int cacheId,
 	}
 	else
 	{
+		heap_close(relation, AccessShareLock);
 		elog(ERROR,
 			 "SearchSysCacheGetAttribute: Bad attr # %d in %s(%d)",
 			 attributeNumber, cacheName, cacheId);
@@ -617,11 +624,11 @@ SearchSysCacheGetAttribute(int cacheId,

 	if (isNull)
 	{
-
 		/*
 		 * Used to be an elog(DEBUG, ...) here and a claim that it should
 		 * be a FATAL error, I don't think either is warranted -mer 6/9/92
 		 */
+		heap_close(relation, AccessShareLock);
 		return NULL;
 	}

@@ -639,6 +646,6 @@ SearchSysCacheGetAttribute(int cacheId,
 		returnValue = (void *) tmp;
 	}

-	heap_close(relation);
+	heap_close(relation, AccessShareLock);
 	return returnValue;
 }
--- a/src/backend/utils/fmgr/dfmgr.c
+++ b/src/backend/utils/fmgr/dfmgr.c
@@ -7,7 +7,7 @@
 *
 *
 * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/utils/fmgr/dfmgr.c,v 1.31 1999/07/17 20:18:04 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/utils/fmgr/dfmgr.c,v 1.32 1999/09/18 19:08:01 tgl Exp $
 *
 *-------------------------------------------------------------------------
 */
@@ -56,8 +56,13 @@ fmgr_dynamic(Oid procedureId, int *pronargs)

 	/*
 	 * The procedure isn't a builtin, so we'll have to do a catalog lookup
-	 * to find its pg_proc entry.
+	 * to find its pg_proc entry.  Moreover, since probin is varlena, we're
+	 * going to have to use heap_getattr, which means we need the reldesc,
+	 * which means we need to open the relation.  So we might as well do that
+	 * first and get the benefit of SI inval if needed.
 	 */
+	rel = heap_openr(ProcedureRelationName, AccessShareLock);
+
 	procedureTuple = SearchSysCacheTuple(PROOID,
 										 ObjectIdGetDatum(procedureId),
 										 0, 0, 0);
@@ -71,36 +76,24 @@ fmgr_dynamic(Oid procedureId, int *pronargs)
 	procedureStruct = (Form_pg_proc) GETSTRUCT(procedureTuple);
 	proname = procedureStruct->proname.data;
 	pronargs_save = *pronargs = procedureStruct->pronargs;
-
-	/*
-	 * Extract the procedure info from the pg_proc tuple. Since probin is
-	 * varlena, do a amgetattr() on the procedure tuple.  To do that, we
-	 * need the rel for the procedure relation, so...
-	 */
-
-	/* open pg_procedure */
-
-	rel = heap_openr(ProcedureRelationName);
-	if (!RelationIsValid(rel))
-	{
-		elog(ERROR, "fmgr: Could not open relation %s",
-			 ProcedureRelationName);
-		return (func_ptr) NULL;
-	}
 	probinattr = heap_getattr(procedureTuple,
 							  Anum_pg_proc_probin,
 							  RelationGetDescr(rel), &isnull);
 	if (!PointerIsValid(probinattr) /* || isnull */ )
 	{
-		heap_close(rel);
+		heap_close(rel, AccessShareLock);
 		elog(ERROR, "fmgr: Could not extract probin for %u from %s",
 			 procedureId, ProcedureRelationName);
 		return (func_ptr) NULL;
 	}
 	probinstring = textout((struct varlena *) probinattr);

+	heap_close(rel, AccessShareLock);
+
 	user_fn = handle_load(probinstring, proname);

+	pfree(probinstring);
+
 	procedureId_save = procedureId;
 	user_fn_save = user_fn;

--- a/src/backend/utils/misc/database.c
+++ b/src/backend/utils/misc/database.c
@@ -7,7 +7,7 @@
 *
 *
 * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/utils/misc/Attic/database.c,v 1.28 1999/07/17 20:18:11 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/utils/misc/Attic/database.c,v 1.29 1999/09/18 19:08:07 tgl Exp $
 *
 *-------------------------------------------------------------------------
 */
@@ -43,10 +43,7 @@ GetDatabaseInfo(char *name, int4 *owner, char *path)
 	HeapScanDesc scan;
 	ScanKeyData scanKey;

-	dbrel = heap_openr(DatabaseRelationName);
-	if (!RelationIsValid(dbrel))
-		elog(FATAL, "GetDatabaseInfo: cannot open relation \"%-.*s\"",
-			 DatabaseRelationName);
+	dbrel = heap_openr(DatabaseRelationName, AccessShareLock);

 	ScanKeyEntryInitialize(&scanKey, 0, Anum_pg_database_datname,
 						   F_NAMEEQ, NameGetDatum(name));
@@ -71,6 +68,7 @@ GetDatabaseInfo(char *name, int4 *owner, char *path)
 	{
 		elog(NOTICE, "GetDatabaseInfo: %s entry not found %s",
 			 DatabaseRelationName, name);
+		heap_close(dbrel, AccessShareLock);
 		return TRUE;
 	}

@@ -88,7 +86,7 @@ GetDatabaseInfo(char *name, int4 *owner, char *path)
 	memcpy(dbpath, VARDATA(dbtext), (VARSIZE(dbtext) - VARHDRSZ));
 	*(dbpath + (VARSIZE(dbtext) - VARHDRSZ)) = '\0';

-	heap_close(dbrel);
+	heap_close(dbrel, AccessShareLock);

 	owner = palloc(sizeof(Oid));
 	*owner = dbowner;