Tweak indexscan machinery to avoid taking an AccessShareLock on an index

if we already have a stronger lock due to the index's table being the
update target table of the query.  Same optimization I applied earlier
at the table level.  There doesn't seem to be much interest in the more
radical idea of not locking indexes at all, so do what we can ...

Author: tglsfdc

src/backend/access/heap/tuptoaster.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/access/heap/tuptoaster.c,v 1.56 2005/11/22 18:17:06 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/heap/tuptoaster.c,v 1.57 2005/12/03 05:50:59 tgl Exp $
  *
  *
  * INTERFACE ROUTINES
@@ -1142,8 +1142,8 @@ toast_delete_datum(Relation rel, Datum value)
 	/*
 	 * Find the chunks by index
 	 */
-	toastscan = index_beginscan(toastrel, toastidx, SnapshotToast,
-								1, &toastkey);
+	toastscan = index_beginscan(toastrel, toastidx, true,
+								SnapshotToast, 1, &toastkey);
 	while ((toasttup = index_getnext(toastscan, ForwardScanDirection)) != NULL)
 	{
 		/*
@@ -1219,8 +1219,8 @@ toast_fetch_datum(varattrib *attr)
 	 */
 	nextidx = 0;
 
-	toastscan = index_beginscan(toastrel, toastidx, SnapshotToast,
-								1, &toastkey);
+	toastscan = index_beginscan(toastrel, toastidx, true,
+								SnapshotToast, 1, &toastkey);
 	while ((ttup = index_getnext(toastscan, ForwardScanDirection)) != NULL)
 	{
 		/*
@@ -1394,8 +1394,8 @@ toast_fetch_datum_slice(varattrib *attr, int32 sliceoffset, int32 length)
 	 * The index is on (valueid, chunkidx) so they will come in order
 	 */
 	nextidx = startchunk;
-	toastscan = index_beginscan(toastrel, toastidx, SnapshotToast,
-								nscankeys, toastkey);
+	toastscan = index_beginscan(toastrel, toastidx, true,
+								SnapshotToast, nscankeys, toastkey);
 	while ((ttup = index_getnext(toastscan, ForwardScanDirection)) != NULL)
 	{
 		/*

src/backend/access/index/genam.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/access/index/genam.c,v 1.51 2005/11/22 18:17:06 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/index/genam.c,v 1.52 2005/12/03 05:51:00 tgl Exp $
  *
  * NOTES
  *	  many of the old access method routines have been turned into
@@ -86,7 +86,8 @@ RelationGetIndexScan(Relation indexRelation,
 	else
 		scan->keyData = NULL;
 
-	scan->is_multiscan = false; /* caller may change this */
+	scan->is_multiscan = false;			/* caller may change this */
+	scan->have_lock = false;			/* ditto */
 	scan->kill_prior_tuple = false;
 	scan->ignore_killed_tuples = true;	/* default setting */
 	scan->keys_are_unique = false;		/* may be set by index AM */
@@ -211,8 +212,8 @@ systable_beginscan(Relation heapRelation,
 			key[i].sk_attno = i + 1;
 		}
 
-		sysscan->iscan = index_beginscan(heapRelation, irel, snapshot,
-										 nkeys, key);
+		sysscan->iscan = index_beginscan(heapRelation, irel, true,
+										 snapshot, nkeys, key);
 		sysscan->scan = NULL;
 	}
 	else

src/backend/access/index/indexam.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/access/index/indexam.c,v 1.86 2005/10/15 02:49:09 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/index/indexam.c,v 1.87 2005/12/03 05:51:00 tgl Exp $
  *
  * INTERFACE ROUTINES
  *		index_open		- open an index relation by relation OID
@@ -111,6 +111,7 @@ do { \
 } while(0)
 
 static IndexScanDesc index_beginscan_internal(Relation indexRelation,
+						 bool need_index_lock,
 						 int nkeys, ScanKey key);
 
 
@@ -229,16 +230,23 @@ index_insert(Relation indexRelation,
  * heapRelation link (nor the snapshot).  However, the caller had better
  * be holding some kind of lock on the heap relation in any case, to ensure
  * no one deletes it (or the index) out from under us.
+ *
+ * Most callers should pass need_index_lock = true to cause the index code
+ * to take AccessShareLock on the index for the duration of the scan.  But
+ * if it is known that a lock is already held on the index, pass false to
+ * skip taking an unnecessary lock.
  */
 IndexScanDesc
 index_beginscan(Relation heapRelation,
 				Relation indexRelation,
+				bool need_index_lock,
 				Snapshot snapshot,
 				int nkeys, ScanKey key)
 {
 	IndexScanDesc scan;
 
-	scan = index_beginscan_internal(indexRelation, nkeys, key);
+	scan = index_beginscan_internal(indexRelation, need_index_lock,
+									nkeys, key);
 
 	/*
 	 * Save additional parameters into the scandesc.  Everything else was set
@@ -259,12 +267,14 @@ index_beginscan(Relation heapRelation,
  */
 IndexScanDesc
 index_beginscan_multi(Relation indexRelation,
+					  bool need_index_lock,
 					  Snapshot snapshot,
 					  int nkeys, ScanKey key)
 {
 	IndexScanDesc scan;
 
-	scan = index_beginscan_internal(indexRelation, nkeys, key);
+	scan = index_beginscan_internal(indexRelation, need_index_lock,
+									nkeys, key);
 
 	/*
 	 * Save additional parameters into the scandesc.  Everything else was set
@@ -281,6 +291,7 @@ index_beginscan_multi(Relation indexRelation,
  */
 static IndexScanDesc
 index_beginscan_internal(Relation indexRelation,
+						 bool need_index_lock,
 						 int nkeys, ScanKey key)
 {
 	IndexScanDesc scan;
@@ -291,13 +302,15 @@ index_beginscan_internal(Relation indexRelation,
 	RelationIncrementReferenceCount(indexRelation);
 
 	/*
-	 * Acquire AccessShareLock for the duration of the scan
+	 * Acquire AccessShareLock for the duration of the scan, unless caller
+	 * says it already has lock on the index.
 	 *
 	 * Note: we could get an SI inval message here and consequently have to
 	 * rebuild the relcache entry.	The refcount increment above ensures that
 	 * we will rebuild it and not just flush it...
 	 */
-	LockRelation(indexRelation, AccessShareLock);
+	if (need_index_lock)
+		LockRelation(indexRelation, AccessShareLock);
 
 	/*
 	 * LockRelation can clean rd_aminfo structure, so fill procedure after
@@ -315,6 +328,9 @@ index_beginscan_internal(Relation indexRelation,
 									  Int32GetDatum(nkeys),
 									  PointerGetDatum(key)));
 
+	/* Save flag to tell index_endscan whether to release lock */
+	scan->have_lock = need_index_lock;
+
 	return scan;
 }
 
@@ -380,7 +396,8 @@ index_endscan(IndexScanDesc scan)
 
 	/* Release index lock and refcount acquired by index_beginscan */
 
-	UnlockRelation(scan->indexRelation, AccessShareLock);
+	if (scan->have_lock)
+		UnlockRelation(scan->indexRelation, AccessShareLock);
 
 	RelationDecrementReferenceCount(scan->indexRelation);
 

src/backend/catalog/catalog.c

@@ -9,7 +9,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/catalog/catalog.c,v 1.64 2005/10/15 02:49:12 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/catalog/catalog.c,v 1.65 2005/12/03 05:51:00 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -309,7 +309,8 @@ GetNewOidWithIndex(Relation relation, Relation indexrel)
 					ObjectIdGetDatum(newOid));
 
 		/* see notes above about using SnapshotDirty */
-		scan = index_beginscan(relation, indexrel, SnapshotDirty, 1, &key);
+		scan = index_beginscan(relation, indexrel, true,
+							   SnapshotDirty, 1, &key);
 
 		collides = HeapTupleIsValid(index_getnext(scan, ForwardScanDirection));
 

src/backend/commands/cluster.c

@@ -11,7 +11,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/commands/cluster.c,v 1.142 2005/11/22 18:17:08 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/commands/cluster.c,v 1.143 2005/12/03 05:51:01 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -643,7 +643,8 @@ copy_heap_data(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex)
 	 * Scan through the OldHeap on the OldIndex and copy each tuple into the
 	 * NewHeap.
 	 */
-	scan = index_beginscan(OldHeap, OldIndex, SnapshotNow, 0, (ScanKey) NULL);
+	scan = index_beginscan(OldHeap, OldIndex, true,
+						   SnapshotNow, 0, (ScanKey) NULL);
 
 	while ((tuple = index_getnext(scan, ForwardScanDirection)) != NULL)
 	{

src/backend/executor/execUtils.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/executor/execUtils.c,v 1.130 2005/12/02 20:03:40 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/executor/execUtils.c,v 1.131 2005/12/03 05:51:01 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -693,6 +693,28 @@ ExecAssignScanTypeFromOuterPlan(ScanState *scanstate)
  * ----------------------------------------------------------------
  */
 
+/* ----------------------------------------------------------------
+ *		ExecRelationIsTargetRelation
+ *
+ *		Detect whether a relation (identified by rangetable index)
+ *		is one of the target relations of the query.
+ * ----------------------------------------------------------------
+ */
+bool
+ExecRelationIsTargetRelation(EState *estate, Index scanrelid)
+{
+	ResultRelInfo *resultRelInfos;
+	int			i;
+
+	resultRelInfos = estate->es_result_relations;
+	for (i = 0; i < estate->es_num_result_relations; i++)
+	{
+		if (resultRelInfos[i].ri_RangeTableIndex == scanrelid)
+			return true;
+	}
+	return false;
+}
+
 /* ----------------------------------------------------------------
  *		ExecOpenScanRelation
  *

src/backend/executor/nodeBitmapIndexscan.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/executor/nodeBitmapIndexscan.c,v 1.13 2005/12/02 20:03:40 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/executor/nodeBitmapIndexscan.c,v 1.14 2005/12/03 05:51:01 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -214,6 +214,7 @@ BitmapIndexScanState *
 ExecInitBitmapIndexScan(BitmapIndexScan *node, EState *estate)
 {
 	BitmapIndexScanState *indexstate;
+	bool		relistarget;
 
 	/*
 	 * create state structure
@@ -294,13 +295,19 @@ ExecInitBitmapIndexScan(BitmapIndexScan *node, EState *estate)
 	indexstate->ss.ss_currentScanDesc = NULL;
 
 	/*
-	 * open the index relation and initialize relation and scan descriptors.
+	 * 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.
+	 * and unlocks.  (We rely on having a lock on the parent table to
+	 * ensure the index won't go away!)  Furthermore, if the parent table
+	 * is one of the target relations of the query, then InitPlan already
+	 * opened and write-locked the index, so we can tell the index machinery
+	 * not to bother getting an extra lock.
 	 */
 	indexstate->biss_RelationDesc = index_open(node->indexid);
+	relistarget = ExecRelationIsTargetRelation(estate, node->scan.scanrelid);
 	indexstate->biss_ScanDesc =
 		index_beginscan_multi(indexstate->biss_RelationDesc,
+							  !relistarget,
 							  estate->es_snapshot,
 							  indexstate->biss_NumScanKeys,
 							  indexstate->biss_ScanKeys);

src/backend/executor/nodeIndexscan.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/executor/nodeIndexscan.c,v 1.108 2005/12/02 20:03:40 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/executor/nodeIndexscan.c,v 1.109 2005/12/03 05:51:02 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -461,6 +461,7 @@ ExecInitIndexScan(IndexScan *node, EState *estate)
 {
 	IndexScanState *indexstate;
 	Relation	currentRelation;
+	bool		relistarget;
 
 	/*
 	 * create state structure
@@ -557,14 +558,19 @@ ExecInitIndexScan(IndexScan *node, EState *estate)
 	ExecAssignScanType(&indexstate->ss, RelationGetDescr(currentRelation), false);
 
 	/*
-	 * open the index relation and initialize relation and scan descriptors.
+	 * 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 a lock on the parent table to
-	 * ensure the index won't go away!)
+	 * ensure the index won't go away!)  Furthermore, if the parent table
+	 * is one of the target relations of the query, then InitPlan already
+	 * opened and write-locked the index, so we can tell the index machinery
+	 * not to bother getting an extra lock.
 	 */
 	indexstate->iss_RelationDesc = index_open(node->indexid);
+	relistarget = ExecRelationIsTargetRelation(estate, node->scan.scanrelid);
 	indexstate->iss_ScanDesc = index_beginscan(currentRelation,
 											   indexstate->iss_RelationDesc,
+											   !relistarget,
 											   estate->es_snapshot,
 											   indexstate->iss_NumScanKeys,
 											   indexstate->iss_ScanKeys);

src/backend/storage/large_object/inv_api.c

@@ -9,7 +9,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/storage/large_object/inv_api.c,v 1.113 2005/10/15 02:49:26 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/storage/large_object/inv_api.c,v 1.114 2005/12/03 05:51:02 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -299,7 +299,7 @@ inv_getsize(LargeObjectDesc *obj_desc)
 				BTEqualStrategyNumber, F_OIDEQ,
 				ObjectIdGetDatum(obj_desc->id));
 
-	sd = index_beginscan(lo_heap_r, lo_index_r,
+	sd = index_beginscan(lo_heap_r, lo_index_r, true,
 						 obj_desc->snapshot, 1, skey);
 
 	/*
@@ -410,7 +410,7 @@ inv_read(LargeObjectDesc *obj_desc, char *buf, int nbytes)
 				BTGreaterEqualStrategyNumber, F_INT4GE,
 				Int32GetDatum(pageno));
 
-	sd = index_beginscan(lo_heap_r, lo_index_r,
+	sd = index_beginscan(lo_heap_r, lo_index_r, true,
 						 obj_desc->snapshot, 2, skey);
 
 	while ((tuple = index_getnext(sd, ForwardScanDirection)) != NULL)
@@ -526,7 +526,7 @@ inv_write(LargeObjectDesc *obj_desc, char *buf, int nbytes)
 				BTGreaterEqualStrategyNumber, F_INT4GE,
 				Int32GetDatum(pageno));
 
-	sd = index_beginscan(lo_heap_r, lo_index_r,
+	sd = index_beginscan(lo_heap_r, lo_index_r, false /* got lock */,
 						 obj_desc->snapshot, 2, skey);
 
 	oldtuple = NULL;

src/include/access/genam.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/access/genam.h,v 1.53 2005/10/15 02:49:42 momjian Exp $
+ * $PostgreSQL: pgsql/src/include/access/genam.h,v 1.54 2005/12/03 05:51:03 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -79,9 +79,11 @@ extern bool index_insert(Relation indexRelation,
 
 extern IndexScanDesc index_beginscan(Relation heapRelation,
 				Relation indexRelation,
+				bool need_index_lock,
 				Snapshot snapshot,
 				int nkeys, ScanKey key);
 extern IndexScanDesc index_beginscan_multi(Relation indexRelation,
+					  bool need_index_lock,
 					  Snapshot snapshot,
 					  int nkeys, ScanKey key);
 extern void index_rescan(IndexScanDesc scan, ScanKey key);