Adjust scan plan nodes to avoid getting an extra AccessShareLock on a

relation if it's already been locked by execMain.c as either a result
relation or a FOR UPDATE/SHARE relation.  This avoids an extra trip to
the shared lock manager state.  Per my suggestion yesterday.

src/backend/executor/execMain.c

@@ -26,7 +26,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/executor/execMain.c,v 1.261 2005/11/22 18:17:10 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/executor/execMain.c,v 1.262 2005/12/02 20:03:40 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -52,13 +52,6 @@
 #include "utils/memutils.h"
 
 
-typedef struct execRowMark
-{
-	Relation	relation;
-	Index		rti;
-	char		resname[32];
-} execRowMark;
-
 typedef struct evalPlanQual
 {
 	Index		rti;
@@ -567,10 +560,10 @@ InitPlan(QueryDesc *queryDesc, bool explainOnly)
 			Index		rti = lfirst_int(l);
 			Oid			relid = getrelid(rti, rangeTable);
 			Relation	relation;
-			execRowMark *erm;
+			ExecRowMark *erm;
 
 			relation = heap_open(relid, RowShareLock);
-			erm = (execRowMark *) palloc(sizeof(execRowMark));
+			erm = (ExecRowMark *) palloc(sizeof(ExecRowMark));
 			erm->relation = relation;
 			erm->rti = rti;
 			snprintf(erm->resname, sizeof(erm->resname), "ctid%u", rti);
@@ -1020,7 +1013,7 @@ ExecEndPlan(PlanState *planstate, EState *estate)
 	 */
 	foreach(l, estate->es_rowMarks)
 	{
-		execRowMark *erm = lfirst(l);
+		ExecRowMark *erm = lfirst(l);
 
 		heap_close(erm->relation, NoLock);
 	}
@@ -1165,7 +1158,7 @@ lnext:	;
 		lmark:	;
 				foreach(l, estate->es_rowMarks)
 				{
-					execRowMark *erm = lfirst(l);
+					ExecRowMark *erm = lfirst(l);
 					HeapTupleData tuple;
 					Buffer		buffer;
 					ItemPointerData update_ctid;
@@ -1859,9 +1852,9 @@ EvalPlanQual(EState *estate, Index rti,
 		relation = NULL;
 		foreach(l, estate->es_rowMarks)
 		{
-			if (((execRowMark *) lfirst(l))->rti == rti)
+			if (((ExecRowMark *) lfirst(l))->rti == rti)
 			{
-				relation = ((execRowMark *) lfirst(l))->relation;
+				relation = ((ExecRowMark *) lfirst(l))->relation;
 				break;
 			}
 		}

src/backend/executor/execUtils.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/executor/execUtils.c,v 1.129 2005/11/23 20:27:57 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/executor/execUtils.c,v 1.130 2005/12/02 20:03:40 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -24,6 +24,9 @@
  *		ExecAssignResultType
  *		etc
  *
+ *		ExecOpenScanRelation	Common code for scan node init routines.
+ *		ExecCloseScanRelation
+ *
  *		ExecOpenIndices			\
  *		ExecCloseIndices		 | referenced by InitPlan, EndPlan,
  *		ExecInsertIndexTuples	/  ExecInsert, ExecUpdate
@@ -45,6 +48,7 @@
 #include "catalog/pg_index.h"
 #include "executor/execdebug.h"
 #include "miscadmin.h"
+#include "parser/parsetree.h"
 #include "utils/builtins.h"
 #include "utils/fmgroids.h"
 #include "utils/memutils.h"
@@ -684,6 +688,90 @@ ExecAssignScanTypeFromOuterPlan(ScanState *scanstate)
 }
 
 
+/* ----------------------------------------------------------------
+ *				  Scan node support
+ * ----------------------------------------------------------------
+ */
+
+/* ----------------------------------------------------------------
+ *		ExecOpenScanRelation
+ *
+ *		Open the heap relation to be scanned by a base-level scan plan node.
+ *		This should be called during the node's ExecInit routine.
+ *
+ * By default, this acquires AccessShareLock on the relation.  However,
+ * if the relation was already locked by InitPlan, we don't need to acquire
+ * any additional lock.  This saves trips to the shared lock manager.
+ * ----------------------------------------------------------------
+ */
+Relation
+ExecOpenScanRelation(EState *estate, Index scanrelid)
+{
+	RangeTblEntry *rtentry;
+	Oid			reloid;
+	LOCKMODE	lockmode;
+	ResultRelInfo *resultRelInfos;
+	int			i;
+
+	/*
+	 * First determine the lock type we need.  Scan to see if target relation
+	 * is either a result relation or a FOR UPDATE/FOR SHARE relation.
+	 */
+	lockmode = AccessShareLock;
+	resultRelInfos = estate->es_result_relations;
+	for (i = 0; i < estate->es_num_result_relations; i++)
+	{
+		if (resultRelInfos[i].ri_RangeTableIndex == scanrelid)
+		{
+			lockmode = NoLock;
+			break;
+		}
+	}
+
+	if (lockmode == AccessShareLock)
+	{
+		ListCell   *l;
+
+		foreach(l, estate->es_rowMarks)
+		{
+			ExecRowMark *erm = lfirst(l);
+
+			if (erm->rti == scanrelid)
+			{
+				lockmode = NoLock;
+				break;
+			}
+		}
+	}
+
+	/* OK, open the relation and acquire lock as needed */
+	rtentry = rt_fetch(scanrelid, estate->es_range_table);
+	reloid = rtentry->relid;
+
+	return heap_open(reloid, lockmode);
+}
+
+/* ----------------------------------------------------------------
+ *		ExecCloseScanRelation
+ *
+ *		Close the heap relation scanned by a base-level scan plan node.
+ *		This should be called during the node's ExecEnd routine.
+ *
+ * Currently, we do not release the lock acquired by ExecOpenScanRelation.
+ * This lock should be held till end of transaction.  (There is a faction
+ * that considers this too much locking, however.)
+ *
+ * If we did want to release the lock, we'd have to repeat the logic in
+ * ExecOpenScanRelation in order to figure out what to release.
+ * ----------------------------------------------------------------
+ */
+void
+ExecCloseScanRelation(Relation scanrel)
+{
+	heap_close(scanrel, NoLock);
+}
+
+
 /* ----------------------------------------------------------------
  *				  ExecInsertIndexTuples support
  * ----------------------------------------------------------------
@@ -760,7 +848,7 @@ ExecOpenIndices(ResultRelInfo *resultRelInfo)
 		 *
 		 * If the index AM is not safe for concurrent updates, obtain an
 		 * exclusive lock on the index to lock out other updaters as well as
-		 * readers (index_beginscan places AccessShareLock).
+		 * readers (index_beginscan places AccessShareLock on the index).
 		 *
 		 * If there are multiple not-concurrent-safe indexes, all backends
 		 * must lock the indexes in the same order or we will get deadlocks

src/backend/executor/nodeBitmapHeapscan.c

@@ -21,7 +21,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/executor/nodeBitmapHeapscan.c,v 1.7 2005/12/02 01:29:55 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/executor/nodeBitmapHeapscan.c,v 1.8 2005/12/02 20:03:40 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -141,9 +141,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
 
 			/*
 			 * Ignore any claimed entries past what we think is the end of the
-			 * relation.  (This is probably not necessary given that we got
+			 * relation.  (This is probably not necessary given that we got at
-			 * AccessShareLock before performing any of the indexscans, but
+			 * least AccessShareLock on the table before performing any of the
-			 * let's be safe.)
+			 * indexscans, but let's be safe.)
 			 */
 			if (tbmres->blockno >= scan->rs_nblocks)
 			{
@@ -448,13 +448,8 @@ ExecEndBitmapHeapScan(BitmapHeapScanState *node)
 
 	/*
 	 * close the heap relation.
-	 *
-	 * Currently, we do not release the AccessShareLock acquired by
-	 * ExecInitBitmapHeapScan.	This lock should be held till end of
-	 * transaction. (There is a faction that considers this too much locking,
-	 * however.)
 	 */
-	heap_close(relation, NoLock);
+	ExecCloseScanRelation(relation);
 }
 
 /* ----------------------------------------------------------------
@@ -467,9 +462,6 @@ BitmapHeapScanState *
 ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate)
 {
 	BitmapHeapScanState *scanstate;
-	RangeTblEntry *rtentry;
-	Index		relid;
-	Oid			reloid;
 	Relation	currentRelation;
 
 	/*
@@ -519,13 +511,9 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate)
 	CXT1_printf("ExecInitBitmapHeapScan: context is %d\n", CurrentMemoryContext);
 
 	/*
-	 * open the base relation and acquire AccessShareLock on it.
+	 * open the base relation and acquire appropriate lock on it.
 	 */
-	relid = node->scan.scanrelid;
+	currentRelation = ExecOpenScanRelation(estate, node->scan.scanrelid);
-	rtentry = rt_fetch(relid, estate->es_range_table);
-	reloid = rtentry->relid;
-
-	currentRelation = heap_open(reloid, AccessShareLock);
 
 	scanstate->ss.ss_currentRelation = currentRelation;
 

src/backend/executor/nodeBitmapIndexscan.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/executor/nodeBitmapIndexscan.c,v 1.12 2005/11/25 19:47:49 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/executor/nodeBitmapIndexscan.c,v 1.13 2005/12/02 20:03:40 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -286,8 +286,8 @@ ExecInitBitmapIndexScan(BitmapIndexScan *node, EState *estate)
 
 	/*
 	 * We do not open or lock the base relation here.  We assume that an
-	 * ancestor BitmapHeapScan node is holding AccessShareLock on the heap
+	 * ancestor BitmapHeapScan node is holding AccessShareLock (or better)
-	 * relation throughout the execution of the plan tree.
+	 * on the heap relation throughout the execution of the plan tree.
 	 */
 
 	indexstate->ss.ss_currentRelation = NULL;

src/backend/executor/nodeIndexscan.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/executor/nodeIndexscan.c,v 1.107 2005/11/25 19:47:49 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/executor/nodeIndexscan.c,v 1.108 2005/12/02 20:03:40 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -421,12 +421,8 @@ ExecEndIndexScan(IndexScanState *node)
 
 	/*
 	 * close the heap relation.
-	 *
-	 * Currently, we do not release the AccessShareLock acquired by
-	 * ExecInitIndexScan.  This lock should be held till end of transaction.
-	 * (There is a faction that considers this too much locking, however.)
 	 */
-	heap_close(relation, NoLock);
+	ExecCloseScanRelation(relation);
 }
 
 /* ----------------------------------------------------------------
@@ -464,9 +460,6 @@ IndexScanState *
 ExecInitIndexScan(IndexScan *node, EState *estate)
 {
 	IndexScanState *indexstate;
-	RangeTblEntry *rtentry;
-	Index		relid;
-	Oid			reloid;
 	Relation	currentRelation;
 
 	/*
@@ -551,13 +544,9 @@ ExecInitIndexScan(IndexScan *node, EState *estate)
 	}
 
 	/*
-	 * open the base relation and acquire AccessShareLock on it.
+	 * open the base relation and acquire appropriate lock on it.
 	 */
-	relid = node->scan.scanrelid;
+	currentRelation = ExecOpenScanRelation(estate, node->scan.scanrelid);
-	rtentry = rt_fetch(relid, estate->es_range_table);
-	reloid = rtentry->relid;
-
-	currentRelation = heap_open(reloid, AccessShareLock);
 
 	indexstate->ss.ss_currentRelation = currentRelation;
 	indexstate->ss.ss_currentScanDesc = NULL;	/* no heap scan here */
@@ -570,7 +559,7 @@ ExecInitIndexScan(IndexScan *node, EState *estate)
 	/*
 	 * open the index relation and initialize relation and scan descriptors.
 	 * Note we acquire no locks here; the index machinery does its own locks
-	 * and unlocks.  (We rely on having AccessShareLock on the parent table to
+	 * and unlocks.  (We rely on having a lock on the parent table to
 	 * ensure the index won't go away!)
 	 */
 	indexstate->iss_RelationDesc = index_open(node->indexid);

src/backend/executor/nodeSeqscan.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/executor/nodeSeqscan.c,v 1.55 2005/11/25 04:24:48 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/executor/nodeSeqscan.c,v 1.56 2005/12/02 20:03:40 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -141,25 +141,15 @@ ExecSeqScan(SeqScanState *node)
 static void
 InitScanRelation(SeqScanState *node, EState *estate)
 {
-	Index		relid;
-	List	   *rangeTable;
-	RangeTblEntry *rtentry;
-	Oid			reloid;
 	Relation	currentRelation;
 	HeapScanDesc currentScanDesc;
 
 	/*
 	 * get the relation object id from the relid'th entry in the range table,
-	 * open that relation and initialize the scan state.
+	 * open that relation and acquire appropriate lock on it.
-	 *
-	 * We acquire AccessShareLock for the duration of the scan.
 	 */
-	relid = ((SeqScan *) node->ps.plan)->scanrelid;
+	currentRelation = ExecOpenScanRelation(estate,
-	rangeTable = estate->es_range_table;
+										   ((SeqScan *) node->ps.plan)->scanrelid);
-	rtentry = rt_fetch(relid, rangeTable);
-	reloid = rtentry->relid;
-
-	currentRelation = heap_open(reloid, AccessShareLock);
 
 	currentScanDesc = heap_beginscan(currentRelation,
 									 estate->es_snapshot,
@@ -281,12 +271,8 @@ ExecEndSeqScan(SeqScanState *node)
 
 	/*
 	 * close the heap relation.
-	 *
-	 * Currently, we do not release the AccessShareLock acquired by
-	 * InitScanRelation.  This lock should be held till end of transaction.
-	 * (There is a faction that considers this too much locking, however.)
 	 */
-	heap_close(relation, NoLock);
+	ExecCloseScanRelation(relation);
 }
 
 /* ----------------------------------------------------------------

src/backend/executor/nodeTidscan.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/executor/nodeTidscan.c,v 1.45 2005/11/26 22:14:56 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/executor/nodeTidscan.c,v 1.46 2005/12/02 20:03:41 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -421,12 +421,8 @@ ExecEndTidScan(TidScanState *node)
 
 	/*
 	 * close the heap relation.
-	 *
-	 * Currently, we do not release the AccessShareLock acquired by
-	 * ExecInitTidScan.  This lock should be held till end of transaction.
-	 * (There is a faction that considers this too much locking, however.)
 	 */
-	heap_close(node->ss.ss_currentRelation, NoLock);
+	ExecCloseScanRelation(node->ss.ss_currentRelation);
 }
 
 /* ----------------------------------------------------------------
@@ -472,9 +468,6 @@ TidScanState *
 ExecInitTidScan(TidScan *node, EState *estate)
 {
 	TidScanState *tidstate;
-	RangeTblEntry *rtentry;
-	Oid			relid;
-	Oid			reloid;
 	Relation	currentRelation;
 
 	/*
@@ -521,15 +514,9 @@ ExecInitTidScan(TidScan *node, EState *estate)
 	tidstate->tss_TidPtr = -1;
 
 	/*
-	 * open the base relation
+	 * open the base relation and acquire appropriate lock on it.
-	 *
-	 * We acquire AccessShareLock for the duration of the scan.
 	 */
-	relid = node->scan.scanrelid;
+	currentRelation = ExecOpenScanRelation(estate, node->scan.scanrelid);
-	rtentry = rt_fetch(relid, estate->es_range_table);
-	reloid = rtentry->relid;
-
-	currentRelation = heap_open(reloid, AccessShareLock);
 
 	tidstate->ss.ss_currentRelation = currentRelation;
 	tidstate->ss.ss_currentScanDesc = NULL;		/* no heap scan here */

src/include/executor/executor.h

@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/executor/executor.h,v 1.121 2005/11/23 20:27:58 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/executor/executor.h,v 1.122 2005/12/02 20:03:42 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -230,6 +230,9 @@ extern void ExecAssignScanType(ScanState *scanstate,
 				   TupleDesc tupDesc, bool shouldFree);
 extern void ExecAssignScanTypeFromOuterPlan(ScanState *scanstate);
 
+extern Relation ExecOpenScanRelation(EState *estate, Index scanrelid);
+extern void ExecCloseScanRelation(Relation scanrel);
+
 extern void ExecOpenIndices(ResultRelInfo *resultRelInfo);
 extern void ExecCloseIndices(ResultRelInfo *resultRelInfo);
 extern void ExecInsertIndexTuples(TupleTableSlot *slot, ItemPointer tupleid,

src/include/nodes/execnodes.h

@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/nodes/execnodes.h,v 1.145 2005/11/28 23:46:03 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/nodes/execnodes.h,v 1.146 2005/12/02 20:03:42 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -346,6 +346,15 @@ typedef struct EState
 } EState;
 
 
+/* es_rowMarks is a list of these structs: */
+typedef struct ExecRowMark
+{
+	Relation	relation;		/* opened and RowShareLock'd relation */
+	Index		rti;			/* its range table index */
+	char		resname[32];	/* name for its ctid junk attribute */
+} ExecRowMark;
+
+
 /* ----------------------------------------------------------------
  *				 Tuple Hash Tables
  *