Clean up some minor problems exposed by further thought about Panon's bug

report on old-style functions invoked by RI triggers.  We had a number of
other places that were being sloppy about which memory context FmgrInfo
subsidiary data will be allocated in.  Turns out none of them actually
cause a problem in 7.1, but this is for arcane reasons such as the fact
that old-style triggers aren't supported anyway.  To avoid getting burnt
later, I've restructured the trigger support so that we don't keep trigger
FmgrInfo structs in relcache memory.  Some other related cleanups too:
it's not really necessary to call fmgr_info at all while setting up
the index support info in relcache entries, because those ScanKeyEntry
structs are never used to invoke the functions.  This should speed up
relcache initialization a tiny bit.

Author: tglsfdc

doc/src/sgml/trigger.sgml

@@ -366,7 +366,6 @@ typedef struct Trigger
     Oid         tgoid;
     char       *tgname;
     Oid         tgfoid;
-    FmgrInfo    tgfunc;
     int16       tgtype;
     bool        tgenabled;
     bool        tgisconstraint;

src/backend/access/common/scankey.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/access/common/scankey.c,v 1.18 2001/01/24 19:42:47 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/access/common/scankey.c,v 1.19 2001/06/01 02:41:35 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -40,11 +40,13 @@ ScanKeyEntrySetIllegal(ScanKey entry)
 	entry->sk_flags = 0;		/* just in case... */
 	entry->sk_attno = InvalidAttrNumber;
 	entry->sk_procedure = 0;	/* should be InvalidRegProcedure */
+	entry->sk_func.fn_oid = InvalidOid;
+	entry->sk_argument = (Datum) 0;
 }
 
 /*
  * ScanKeyEntryInitialize
- *		Initializes an scan key entry.
+ *		Initializes a scan key entry.
  *
  * Note:
  *		Assumes the scan key entry is valid.
@@ -64,7 +66,6 @@ ScanKeyEntryInitialize(ScanKey entry,
 	entry->sk_procedure = procedure;
 	entry->sk_argument = argument;
 	fmgr_info(procedure, &entry->sk_func);
-	entry->sk_nargs = entry->sk_func.fn_nargs;
 
 	Assert(ScanKeyEntryIsLegal(entry));
 }

src/backend/access/index/indexam.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/access/index/indexam.c,v 1.49 2001/05/31 18:16:54 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/access/index/indexam.c,v 1.50 2001/06/01 02:41:35 tgl Exp $
  *
  * INTERFACE ROUTINES
  *		index_open		- open an index relation by relationId
@@ -232,7 +232,7 @@ index_beginscan(Relation relation,
 				uint16 numberOfKeys,
 				ScanKey key)
 {
-	IndexScanDesc scandesc;
+	IndexScanDesc scan;
 	RegProcedure procedure;
 
 	RELATION_CHECKS;
@@ -249,14 +249,22 @@ index_beginscan(Relation relation,
 	 */
 	LockRelation(relation, AccessShareLock);
 
-	scandesc = (IndexScanDesc)
+	scan = (IndexScanDesc)
 		DatumGetPointer(OidFunctionCall4(procedure,
 										 PointerGetDatum(relation),
 										 BoolGetDatum(scanFromEnd),
 										 UInt16GetDatum(numberOfKeys),
 										 PointerGetDatum(key)));
 
-	return scandesc;
+	/*
+	 * We want to look up the amgettuple procedure just once per scan,
+	 * not once per index_getnext call.  So do it here and save
+	 * the fmgr info result in the scan descriptor.
+	 */
+	GET_SCAN_PROCEDURE(beginscan, amgettuple);
+	fmgr_info(procedure, &scan->fn_getnext);
+
+	return scan;
 }
 
 /* ----------------
@@ -345,19 +353,9 @@ index_getnext(IndexScanDesc scan,
 
 	SCAN_CHECKS;
 
-	/*
-	 * Look up the access procedure only once per scan.
-	 */
-	if (scan->fn_getnext.fn_oid == InvalidOid)
-	{
-		RegProcedure procedure;
-
-		GET_SCAN_PROCEDURE(getnext, amgettuple);
-		fmgr_info(procedure, &scan->fn_getnext);
-	}
-
 	/*
 	 * have the am's gettuple proc do all the work.
+	 * index_beginscan already set up fn_getnext.
 	 */
 	result = (RetrieveIndexResult)
 		DatumGetPointer(FunctionCall2(&scan->fn_getnext,

src/backend/access/index/istrat.c

@@ -9,7 +9,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/access/index/Attic/istrat.c,v 1.50 2001/05/30 19:53:40 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/access/index/Attic/istrat.c,v 1.51 2001/06/01 02:41:35 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -458,6 +458,8 @@ RelationInvokeStrategy(Relation relation,
 
 /* ----------------
  *		OperatorRelationFillScanKeyEntry
+ *
+ * Initialize a ScanKey entry given already-opened pg_operator relation.
  * ----------------
  */
 static void
@@ -498,6 +500,7 @@ OperatorRelationFillScanKeyEntry(Relation operatorRelation,
 			 operatorObjectId);
 	}
 
+	MemSet(entry, 0, sizeof(*entry));
 	entry->sk_flags = 0;
 	entry->sk_procedure = ((Form_pg_operator) GETSTRUCT(tuple))->oprcode;
 
@@ -511,14 +514,29 @@ OperatorRelationFillScanKeyEntry(Relation operatorRelation,
 		"OperatorRelationFillScanKeyEntry: no procedure for operator %u",
 			 operatorObjectId);
 
-	fmgr_info(entry->sk_procedure, &entry->sk_func);
-	entry->sk_nargs = entry->sk_func.fn_nargs;
+	/*
+	 * Formerly we initialized entry->sk_func here, but that's a waste of
+	 * time because ScanKey entries in strategy maps are never actually
+	 * used to invoke the operator.  Furthermore, to do that we'd have to
+	 * worry about setting the proper memory context (the map is probably
+	 * not allocated in the current memory context!)
+	 */
 }
 
 
 /*
  * IndexSupportInitialize
  *		Initializes an index strategy and associated support procedures.
+ *
+ * Data is returned into *indexStrategy, *indexSupport, and *isUnique,
+ * all of which are objects allocated by the caller.
+ *
+ * The primary input keys are indexObjectId and accessMethodObjectId.
+ * The caller also passes maxStrategyNumber, maxSupportNumber, and
+ * maxAttributeNumber, since these indicate the size of the indexStrategy
+ * and indexSupport arrays it has allocated --- but in practice these
+ * numbers must always match those obtainable from the system catalog
+ * entries for the index and access method.
  */
 void
 IndexSupportInitialize(IndexStrategy indexStrategy,
@@ -578,7 +596,7 @@ IndexSupportInitialize(IndexStrategy indexStrategy,
 		if (!OidIsValid(iform->indkey[attIndex]))
 		{
 			if (attIndex == InvalidAttrNumber)
-				elog(ERROR, "IndexSupportInitialize: no pg_index tuple");
+				elog(ERROR, "IndexSupportInitialize: bogus pg_index tuple");
 			break;
 		}
 
@@ -637,14 +655,16 @@ IndexSupportInitialize(IndexStrategy indexStrategy,
 		heap_close(relation, AccessShareLock);
 	}
 
+	/* Now load the strategy information for the index operators */
 	ScanKeyEntryInitialize(&entry[0], 0,
 						   Anum_pg_amop_amopid,
 						   F_OIDEQ,
 						   ObjectIdGetDatum(accessMethodObjectId));
 
 	ScanKeyEntryInitialize(&entry[1], 0,
 						   Anum_pg_amop_amopclaid,
-						   F_OIDEQ, 0);
+						   F_OIDEQ,
+						   0);	/* will fill below */
 
 	relation = heap_openr(AccessMethodOperatorRelationName, AccessShareLock);
 	operatorRelation = heap_openr(OperatorRelationName, AccessShareLock);
@@ -670,9 +690,11 @@ IndexSupportInitialize(IndexStrategy indexStrategy,
 			Form_pg_amop aform;
 
 			aform = (Form_pg_amop) GETSTRUCT(tuple);
+			strategy = aform->amopstrategy;
+			Assert(strategy > 0 && strategy <= maxStrategyNumber);
 			OperatorRelationFillScanKeyEntry(operatorRelation,
 											 aform->amopopr,
-				   StrategyMapGetScanKeyEntry(map, aform->amopstrategy));
+				   StrategyMapGetScanKeyEntry(map, strategy));
 		}
 
 		heap_endscan(scan);

src/backend/catalog/index.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/catalog/index.c,v 1.152 2001/05/30 20:52:32 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/catalog/index.c,v 1.153 2001/06/01 02:41:35 tgl Exp $
  *
  *
  * INTERFACE ROUTINES
@@ -722,6 +722,9 @@ UpdateIndexPredicate(Oid indexoid, Node *oldPred, Node *predicate)
 
 /* ----------------------------------------------------------------
  *		InitIndexStrategy
+ *
+ * XXX this is essentially the same as relcache.c's
+ * IndexedAccessMethodInitialize(), and probably ought to be merged with it.
  * ----------------------------------------------------------------
  */
 void
@@ -733,18 +736,16 @@ InitIndexStrategy(int numatts,
 	RegProcedure *support;
 	uint16		amstrategies;
 	uint16		amsupport;
-	Oid			attrelid;
 	Size		strsize;
 
 	/*
 	 * get information from the index relation descriptor
 	 */
-	attrelid = indexRelation->rd_att->attrs[0]->attrelid;
 	amstrategies = indexRelation->rd_am->amstrategies;
 	amsupport = indexRelation->rd_am->amsupport;
 
 	/*
-	 * get the size of the strategy
+	 * compute the size of the strategy array
 	 */
 	strsize = AttributeNumberGetIndexStrategySize(numatts, amstrategies);
 
@@ -779,7 +780,8 @@ InitIndexStrategy(int numatts,
 
 	IndexSupportInitialize(strategy, support,
 						   &indexRelation->rd_uniqueindex,
-						   attrelid, accessMethodObjectId,
+						   RelationGetRelid(indexRelation),
+						   accessMethodObjectId,
 						   amstrategies, amsupport, numatts);
 
 	/*