Use a private memory context to store rule information in each relcache

entry that has rules.  This allows us to release the rule parsetrees
on relcache flush without needing a working freeObject() routine.
Formerly, the rule trees were leaked permanently at relcache flush.
Also, clean up handling of rule creation and deletion --- there was
not sufficient locking of the relation being modified, and there was
no reliable notification of other backends that a relcache reload
was needed.  Also, clean up relcache.c code so that scans of system
tables needed to load a relcache entry are done in the caller's
memory context, not in CacheMemoryContext.  This prevents any
un-pfreed memory from those scans from becoming a permanent memory
leak.

src/backend/catalog/index.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/catalog/index.c,v 1.120 2000/06/28 03:31:23 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/catalog/index.c,v 1.121 2000/06/30 07:04:17 tgl Exp $
  *
  *
  * INTERFACE ROUTINES
@@ -369,16 +369,21 @@ ConstructTupleDescriptor(Oid heapoid,
 
 /* ----------------------------------------------------------------
  * AccessMethodObjectIdGetForm
- *		Returns the formated access method tuple given its object identifier.
+ *		Returns an access method tuple given its object identifier,
+ *		or NULL if no such AM tuple can be found.
  *
- * XXX ADD INDEXING
+ * Scanning is done using CurrentMemoryContext as working storage,
+ * but the returned tuple will be allocated in resultCxt (which is
+ * typically CacheMemoryContext).
  *
- * Note:
- *		Assumes object identifier is valid.
+ * There was a note here about adding indexing, but I don't see a need
+ * for it.  There are so few tuples in pg_am that an indexscan would
+ * surely be slower.
  * ----------------------------------------------------------------
  */
 Form_pg_am
-AccessMethodObjectIdGetForm(Oid accessMethodObjectId)
+AccessMethodObjectIdGetForm(Oid accessMethodObjectId,
+							MemoryContext resultCxt)
 {
 	Relation	pg_am_desc;
 	HeapScanDesc pg_am_scan;
@@ -415,10 +420,10 @@ AccessMethodObjectIdGetForm(Oid accessMethodObjectId)
 	}
 
 	/* ----------------
-	 *	if found am tuple, then copy the form and return the copy
+	 *	if found AM tuple, then copy it into resultCxt and return the copy
 	 * ----------------
 	 */
-	aform = (Form_pg_am) palloc(sizeof *aform);
+	aform = (Form_pg_am) MemoryContextAlloc(resultCxt, sizeof *aform);
 	memcpy(aform, GETSTRUCT(pg_am_tuple), sizeof *aform);
 
 	heap_endscan(pg_am_scan);
@@ -434,22 +439,8 @@ AccessMethodObjectIdGetForm(Oid accessMethodObjectId)
 static void
 ConstructIndexReldesc(Relation indexRelation, Oid amoid)
 {
-	MemoryContext oldcxt;
-
-	/* ----------------
-	 *	  here we make certain to allocate the access method
-	 *	  tuple within the cache context lest it vanish when the
-	 *	  context changes
-	 * ----------------
-	 */
-	if (!CacheMemoryContext)
-		CreateCacheMemoryContext();
-
-	oldcxt = MemoryContextSwitchTo(CacheMemoryContext);
-
-	indexRelation->rd_am = AccessMethodObjectIdGetForm(amoid);
-
-	MemoryContextSwitchTo(oldcxt);
+	indexRelation->rd_am = AccessMethodObjectIdGetForm(amoid,
+													   CacheMemoryContext);
 
 	/* ----------------
 	 *	 XXX missing the initialization of some other fields

src/backend/commands/trigger.c

@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/commands/trigger.c,v 1.70 2000/06/28 03:31:28 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/commands/trigger.c,v 1.71 2000/06/30 07:04:17 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -431,10 +431,17 @@ RelationRemoveTriggers(Relation rel)
 	heap_close(tgrel, RowExclusiveLock);
 }
 
+/*
+ * Build trigger data to attach to the given relcache entry.
+ *
+ * Note that trigger data must be allocated in CacheMemoryContext
+ * to ensure it survives as long as the relcache entry.  But we
+ * are probably running in a less long-lived working context.
+ */
 void
 RelationBuildTriggers(Relation relation)
 {
-	TriggerDesc *trigdesc = (TriggerDesc *) palloc(sizeof(TriggerDesc));
+	TriggerDesc *trigdesc;
 	int			ntrigs = relation->rd_rel->reltriggers;
 	Trigger    *triggers = NULL;
 	Trigger    *build;
@@ -453,6 +460,8 @@ RelationBuildTriggers(Relation relation)
 	int			found;
 	bool		hasindex;
 
+	trigdesc = (TriggerDesc *) MemoryContextAlloc(CacheMemoryContext,
+												  sizeof(TriggerDesc));
 	MemSet(trigdesc, 0, sizeof(TriggerDesc));
 
 	ScanKeyEntryInitialize(&skey,
@@ -499,13 +508,16 @@ RelationBuildTriggers(Relation relation)
 		pg_trigger = (Form_pg_trigger) GETSTRUCT(htup);
 
 		if (triggers == NULL)
-			triggers = (Trigger *) palloc(sizeof(Trigger));
+			triggers = (Trigger *) MemoryContextAlloc(CacheMemoryContext,
+													  sizeof(Trigger));
 		else
-			triggers = (Trigger *) repalloc(triggers, (found + 1) * sizeof(Trigger));
+			triggers = (Trigger *) repalloc(triggers,
+											(found + 1) * sizeof(Trigger));
 		build = &(triggers[found]);
 
 		build->tgoid = htup->t_data->t_oid;
-		build->tgname = nameout(&pg_trigger->tgname);
+		build->tgname = MemoryContextStrdup(CacheMemoryContext,
+											nameout(&pg_trigger->tgname));
 		build->tgfoid = pg_trigger->tgfoid;
 		build->tgfunc.fn_oid = InvalidOid; /* mark FmgrInfo as uninitialized */
 		build->tgtype = pg_trigger->tgtype;
@@ -514,7 +526,8 @@ RelationBuildTriggers(Relation relation)
 		build->tgdeferrable = pg_trigger->tgdeferrable;
 		build->tginitdeferred = pg_trigger->tginitdeferred;
 		build->tgnargs = pg_trigger->tgnargs;
-		memcpy(build->tgattr, &(pg_trigger->tgattr), FUNC_MAX_ARGS * sizeof(int16));
+		memcpy(build->tgattr, &(pg_trigger->tgattr),
+			   FUNC_MAX_ARGS * sizeof(int16));
 		val = (struct varlena *) fastgetattr(htup,
 											 Anum_pg_trigger_tgargs,
 											 tgrel->rd_att, &isnull);
@@ -533,10 +546,13 @@ RelationBuildTriggers(Relation relation)
 				elog(ERROR, "RelationBuildTriggers: tgargs IS NULL for rel %s",
 					 RelationGetRelationName(relation));
 			p = (char *) VARDATA(val);
-			build->tgargs = (char **) palloc(build->tgnargs * sizeof(char *));
+			build->tgargs = (char **)
+				MemoryContextAlloc(CacheMemoryContext,
+								   build->tgnargs * sizeof(char *));
 			for (i = 0; i < build->tgnargs; i++)
 			{
-				build->tgargs[i] = pstrdup(p);
+				build->tgargs[i] = MemoryContextStrdup(CacheMemoryContext,
+													   p);
 				p += strlen(p) + 1;
 			}
 		}
@@ -611,7 +627,8 @@ DescribeTrigger(TriggerDesc *trigdesc, Trigger *trigger)
 	{
 		tp = &(t[TRIGGER_EVENT_INSERT]);
 		if (*tp == NULL)
-			*tp = (Trigger **) palloc(sizeof(Trigger *));
+			*tp = (Trigger **) MemoryContextAlloc(CacheMemoryContext,
+												  sizeof(Trigger *));
 		else
 			*tp = (Trigger **) repalloc(*tp, (n[TRIGGER_EVENT_INSERT] + 1) *
 										sizeof(Trigger *));
@@ -623,7 +640,8 @@ DescribeTrigger(TriggerDesc *trigdesc, Trigger *trigger)
 	{
 		tp = &(t[TRIGGER_EVENT_DELETE]);
 		if (*tp == NULL)
-			*tp = (Trigger **) palloc(sizeof(Trigger *));
+			*tp = (Trigger **) MemoryContextAlloc(CacheMemoryContext,
+												  sizeof(Trigger *));
 		else
 			*tp = (Trigger **) repalloc(*tp, (n[TRIGGER_EVENT_DELETE] + 1) *
 										sizeof(Trigger *));
@@ -635,7 +653,8 @@ DescribeTrigger(TriggerDesc *trigdesc, Trigger *trigger)
 	{
 		tp = &(t[TRIGGER_EVENT_UPDATE]);
 		if (*tp == NULL)
-			*tp = (Trigger **) palloc(sizeof(Trigger *));
+			*tp = (Trigger **) MemoryContextAlloc(CacheMemoryContext,
+												  sizeof(Trigger *));
 		else
 			*tp = (Trigger **) repalloc(*tp, (n[TRIGGER_EVENT_UPDATE] + 1) *
 										sizeof(Trigger *));
@@ -1023,10 +1042,9 @@ ltrmark:;
 
 /* ----------
  * Internal data to the deferred trigger mechanism is held
- * during entire session in a global memor created at startup and
- * over statements/commands in a separate global memory which
- * is created at transaction start and destroyed at transaction
- * end.
+ * during entire session in a global context created at startup and
+ * over statements/commands in a separate context which
+ * is created at transaction start and destroyed at transaction end.
  * ----------
  */
 static MemoryContext deftrig_gcxt = NULL;

src/backend/rewrite/rewriteDefine.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/rewrite/rewriteDefine.c,v 1.47 2000/06/28 03:31:56 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/rewrite/rewriteDefine.c,v 1.48 2000/06/30 07:04:22 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -29,27 +29,16 @@
  * InsertRule -
  *	  takes the arguments and inserts them as attributes into the system
  *	  relation "pg_rewrite"
- *
- *		ARGS :	rulname			-		name of the rule
- *				evtype			-		one of RETRIEVE,REPLACE,DELETE,APPEND
- *				evobj			-		name of relation
- *				evslot			-		comma delimited list of slots
- *										if null => multi-attr rule
- *				evinstead		-		is an instead rule
- *				actiontree		-		parsetree(s) of rule action
  */
 static Oid
 InsertRule(char *rulname,
 		   int evtype,
-		   char *evobj,
-		   char *evslot,
-		   char *evqual,
+		   Oid eventrel_oid,
+		   AttrNumber evslot_index,
 		   bool evinstead,
+		   char *evqual,
 		   char *actiontree)
 {
-	Relation	eventrel;
-	Oid			eventrel_oid;
-	AttrNumber	evslot_index;
 	int			i;
 	Datum		values[Natts_pg_rewrite];
 	char		nulls[Natts_pg_rewrite];
@@ -59,21 +48,6 @@ InsertRule(char *rulname,
 	HeapTuple	tup;
 	Oid			rewriteObjectId;
 
-	eventrel = heap_openr(evobj, AccessShareLock);
-	eventrel_oid = RelationGetRelid(eventrel);
-
-	/*
-	 * if the slotname is null, we know that this is a multi-attr rule
-	 */
-	if (evslot == NULL)
-		evslot_index = -1;
-	else
-		evslot_index = attnameAttNum(eventrel, evslot);
-	heap_close(eventrel, AccessShareLock);
-
-	if (evqual == NULL)
-		evqual = "<>";
-
 	if (IsDefinedRewriteRule(rulname))
 		elog(ERROR, "Attempt to insert rule '%s' failed: already exists",
 			 rulname);
@@ -177,17 +151,27 @@ DefineQueryRewrite(RuleStmt *stmt)
 	Node	   *event_qual = stmt->whereClause;
 	bool		is_instead = stmt->instead;
 	List	   *action = stmt->actions;
-	Relation	event_relation = NULL;
+	Relation	event_relation;
+	Oid			ev_relid;
 	Oid			ruleId;
-	Oid			ev_relid = 0;
 	char	   *eslot_string = NULL;
-	int			event_attno = 0;
-	Oid			event_attype = 0;
+	int			event_attno;
+	Oid			event_attype;
 	char	   *actionP,
 			   *event_qualP;
 	List	   *l;
 	Query	   *query;
 
+	/*
+	 * If we are installing an ON SELECT rule, we had better grab
+	 * AccessExclusiveLock to ensure no SELECTs are currently running on
+	 * the event relation.  For other types of rules, it might be sufficient
+	 * to grab ShareLock to lock out insert/update/delete actions.  But
+	 * for now, let's just grab AccessExclusiveLock all the time.
+	 */
+	event_relation = heap_openr(event_obj->relname, AccessExclusiveLock);
+	ev_relid = RelationGetRelid(event_relation);
+
 	/* ----------
 	 * The current rewrite handler is known to work on relation level
 	 * rules only. And for SELECT events, it expects one non-nothing
@@ -209,19 +193,18 @@ DefineQueryRewrite(RuleStmt *stmt)
 	/*
 	 * No rule actions that modify OLD or NEW
 	 */
-	if (action != NIL)
-		foreach(l, action)
+	foreach(l, action)
 	{
 		query = (Query *) lfirst(l);
 		if (query->resultRelation == 1)
 		{
-			elog(NOTICE, "rule actions on OLD currently not supported");
-			elog(ERROR, " use views or triggers instead");
+			elog(ERROR, "rule actions on OLD currently not supported"
+				 "\n\tuse views or triggers instead");
 		}
 		if (query->resultRelation == 2)
 		{
-			elog(NOTICE, "rule actions on NEW currently not supported");
-			elog(ERROR, " use triggers instead");
+			elog(ERROR, "rule actions on NEW currently not supported"
+				 "\n\tuse triggers instead");
 		}
 	}
 
@@ -242,8 +225,8 @@ DefineQueryRewrite(RuleStmt *stmt)
 		 */
 		if (length(action) == 0)
 		{
-			elog(NOTICE, "instead nothing rules on select currently not supported");
-			elog(ERROR, " use views instead");
+			elog(ERROR, "instead nothing rules on select currently not supported"
+				 "\n\tuse views instead");
 		}
 
 		/*
@@ -265,8 +248,6 @@ DefineQueryRewrite(RuleStmt *stmt)
 		 * ... the targetlist of the SELECT action must exactly match the
 		 * event relation, ...
 		 */
-		event_relation = heap_openr(event_obj->relname, AccessShareLock);
-
 		if (event_relation->rd_att->natts != length(query->targetList))
 			elog(ERROR, "select rules target list must match event relations structure");
 
@@ -275,7 +256,7 @@ DefineQueryRewrite(RuleStmt *stmt)
 			tle = (TargetEntry *) nth(i - 1, query->targetList);
 			resdom = tle->resdom;
 			attr = event_relation->rd_att->attrs[i - 1];
-			attname = pstrdup(NameStr(attr->attname));
+			attname = NameStr(attr->attname);
 
 			if (strcmp(resdom->resname, attname) != 0)
 				elog(ERROR, "select rules target entry %d has different column name from %s", i, attname);
@@ -303,8 +284,6 @@ DefineQueryRewrite(RuleStmt *stmt)
 			}
 		}
 
-		heap_close(event_relation, AccessShareLock);
-
 		/*
 		 * LIMIT in view is not supported
 		 */
@@ -337,62 +316,46 @@ DefineQueryRewrite(RuleStmt *stmt)
 	/*
 	 * This rule is allowed - install it.
 	 */
-
-	event_relation = heap_openr(event_obj->relname, AccessShareLock);
-	ev_relid = RelationGetRelid(event_relation);
-
 	if (eslot_string == NULL)
 	{
 		event_attno = -1;
-		event_attype = -1;		/* XXX - don't care */
+		event_attype = InvalidOid;
 	}
 	else
 	{
 		event_attno = attnameAttNum(event_relation, eslot_string);
 		event_attype = attnumTypeId(event_relation, event_attno);
 	}
-	heap_close(event_relation, AccessShareLock);
 
 	/* fix bug about instead nothing */
 	ValidateRule(event_type, event_obj->relname,
 				 eslot_string, event_qual, &action,
 				 is_instead, event_attype);
 
-	if (action == NULL)
-	{
-		if (!is_instead)
-			return;				/* doesn't do anything */
-
-		event_qualP = nodeToString(event_qual);
-
-		ruleId = InsertRule(stmt->rulename,
-							event_type,
-							event_obj->relname,
-							eslot_string,
-							event_qualP,
-							true,
-							"<>");
-		prs2_addToRelation(ev_relid, ruleId, event_type, event_attno, TRUE,
-						   event_qual, NIL);
-
-	}
-	else
+	/* discard rule if it's null action and not INSTEAD; it's a no-op */
+	if (action != NULL || is_instead)
 	{
 		event_qualP = nodeToString(event_qual);
 		actionP = nodeToString(action);
 
 		ruleId = InsertRule(stmt->rulename,
 							event_type,
-							event_obj->relname,
-							eslot_string,
-							event_qualP,
+							ev_relid,
+							event_attno,
 							is_instead,
+							event_qualP,
 							actionP);
 
-		/* what is the max size of type text? XXX -- glass */
-		if (length(action) > 15)
-			elog(ERROR, "max # of actions exceeded");
-		prs2_addToRelation(ev_relid, ruleId, event_type, event_attno,
-						   is_instead, event_qual, action);
+		/*
+		 * Set pg_class 'relhasrules' field TRUE for event relation.
+		 *
+		 * Important side effect: an SI notice is broadcast to force all
+		 * backends (including me!) to update relcache entries with the new
+		 * rule.
+		 */
+		setRelhasrulesInRelation(ev_relid, true);
 	}
+
+	/* Close rel, but keep lock till commit... */
+	heap_close(event_relation, NoLock);
 }

src/backend/rewrite/rewriteHandler.c

@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/rewrite/rewriteHandler.c,v 1.76 2000/06/15 03:32:22 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/rewrite/rewriteHandler.c,v 1.77 2000/06/30 07:04:22 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1332,11 +1332,11 @@ RewriteQuery(Query *parsetree, bool *instead_flag, List **qual_products)
 	rt_entry = rt_fetch(result_relation, parsetree->rtable);
 	rt_entry_relation = heap_openr(rt_entry->relname, AccessShareLock);
 	rt_entry_locks = rt_entry_relation->rd_rules;
-	heap_close(rt_entry_relation, AccessShareLock);
 
 	if (rt_entry_locks != NULL)
 	{
-		List	   *locks = matchLocks(event, rt_entry_locks, result_relation, parsetree);
+		List	   *locks = matchLocks(event, rt_entry_locks,
+									   result_relation, parsetree);
 
 		product_queries = fireRules(parsetree,
 									result_relation,
@@ -1346,13 +1346,15 @@ RewriteQuery(Query *parsetree, bool *instead_flag, List **qual_products)
 									qual_products);
 	}
 
+	heap_close(rt_entry_relation, AccessShareLock);
+
 	return product_queries;
 }
 
 
 /*
  * to avoid infinite recursion, we restrict the number of times a query
- * can be rewritten. Detecting cycles is left for the reader as an excercise.
+ * can be rewritten. Detecting cycles is left for the reader as an exercise.
  */
 #ifndef REWRITE_INVOKE_MAX
 #define REWRITE_INVOKE_MAX		10
@@ -1373,8 +1375,6 @@ deepRewriteQuery(Query *parsetree)
 	bool		instead;
 	List	   *qual_products = NIL;
 
-
-
 	if (++numQueryRewriteInvoked > REWRITE_INVOKE_MAX)
 	{
 		elog(ERROR, "query rewritten %d times, may contain cycles",

src/backend/rewrite/rewriteRemove.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/rewrite/rewriteRemove.c,v 1.37 2000/05/28 17:56:02 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/rewrite/rewriteRemove.c,v 1.38 2000/06/30 07:04:23 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -52,30 +52,20 @@ RewriteGetRuleEventRel(char *rulename)
 	return NameStr(((Form_pg_class) GETSTRUCT(htup))->relname);
 }
 
-/* ----------------------------------------------------------------
- *
+/*
  * RemoveRewriteRule
  *
  * Delete a rule given its rulename.
- *
- * There are three steps.
- *	 1) Find the corresponding tuple in 'pg_rewrite' relation.
- *		Find the rule Id (i.e. the Oid of the tuple) and finally delete
- *		the tuple.
- *	 3) Delete the locks from the 'pg_class' relation.
- *
- *
- * ----------------------------------------------------------------
  */
 void
 RemoveRewriteRule(char *ruleName)
 {
-	Relation	RewriteRelation = NULL;
-	HeapTuple	tuple = NULL;
-	Oid			ruleId = (Oid) 0;
-	Oid			eventRelationOid = (Oid) NULL;
-	Datum		eventRelationOidDatum = (Datum) NULL;
-	bool		isNull = false;
+	Relation	RewriteRelation;
+	Relation	event_relation;
+	HeapTuple	tuple;
+	Oid			ruleId;
+	Oid			eventRelationOid;
+	bool		hasMoreRules;
 
 	/*
 	 * Open the pg_rewrite relation.
@@ -83,7 +73,7 @@ RemoveRewriteRule(char *ruleName)
 	RewriteRelation = heap_openr(RewriteRelationName, RowExclusiveLock);
 
 	/*
-	 * Scan the RuleRelation ('pg_rewrite') until we find a tuple
+	 * Find the tuple for the target rule.
 	 */
 	tuple = SearchSysCacheTupleCopy(RULENAME,
 									PointerGetDatum(ruleName),
@@ -99,44 +89,49 @@ RemoveRewriteRule(char *ruleName)
 	}
 
 	/*
-	 * Store the OID of the rule (i.e. the tuple's OID) and the event
+	 * Save the OID of the rule (i.e. the tuple's OID) and the event
 	 * relation's OID
 	 */
 	ruleId = tuple->t_data->t_oid;
-	eventRelationOidDatum = heap_getattr(tuple,
-										 Anum_pg_rewrite_ev_class,
-									   RelationGetDescr(RewriteRelation),
-										 &isNull);
-	if (isNull)
-	{
-		/* XXX strange!!! */
-		heap_freetuple(tuple);
-		elog(ERROR, "RemoveRewriteRule: internal error; null event target relation!");
-	}
-	eventRelationOid = DatumGetObjectId(eventRelationOidDatum);
+	eventRelationOid = ((Form_pg_rewrite) GETSTRUCT(tuple))->ev_class;
 
 	/*
-	 * Now delete the relation level locks from the updated relation.
-	 * (Make sure we do this before we remove the rule from pg_rewrite.
-	 * Otherwise, heap_openr on eventRelationOid which reads pg_rwrite for
-	 * the rules will fail.)
+	 * We had better grab AccessExclusiveLock so that we know no other
+	 * rule additions/deletions are going on for this relation.  Else
+	 * we cannot set relhasrules correctly.  Besides, we don't want to
+	 * be changing the ruleset while queries are executing on the rel.
 	 */
-	prs2_deleteFromRelation(eventRelationOid, ruleId);
+	event_relation = heap_open(eventRelationOid, AccessExclusiveLock);
+
+	hasMoreRules = event_relation->rd_rules != NULL &&
+		event_relation->rd_rules->numLocks > 1;
 
 	/*
 	 * Delete any comments associated with this rule
-	 *
 	 */
-
 	DeleteComments(ruleId);
 
 	/*
-	 * Now delete the tuple...
+	 * Now delete the pg_rewrite tuple for the rule
 	 */
 	heap_delete(RewriteRelation, &tuple->t_self, NULL);
 
 	heap_freetuple(tuple);
+
 	heap_close(RewriteRelation, RowExclusiveLock);
+
+	/*
+	 * Set pg_class 'relhasrules' field correctly for event relation.
+	 *
+	 * Important side effect: an SI notice is broadcast to force all
+	 * backends (including me!) to update relcache entries with the
+	 * new rule set.  Therefore, must do this even if relhasrules is
+	 * still true!
+	 */
+	setRelhasrulesInRelation(eventRelationOid, hasMoreRules);
+
+	/* Close rel, but keep lock till commit... */
+	heap_close(event_relation, NoLock);
 }
 
 /*

src/backend/rewrite/rewriteSupport.c

@@ -8,13 +8,12 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/rewrite/rewriteSupport.c,v 1.42 2000/06/28 03:31:56 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/rewrite/rewriteSupport.c,v 1.43 2000/06/30 07:04:23 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
 #include "postgres.h"
 
-
 #include "access/heapam.h"
 #include "catalog/catname.h"
 #include "catalog/indexing.h"
@@ -22,60 +21,6 @@
 #include "utils/catcache.h"
 #include "utils/syscache.h"
 
-/*
- * RuleIdGetActionInfo -
- *	   given a rule oid, look it up and return the rule-event-qual and
- *	   list of parsetrees for the rule (in parseTrees)
- */
-#ifdef NOT_USED
-static Node *
-RuleIdGetActionInfo(Oid ruleoid, bool *instead_flag, Query **parseTrees)
-{
-	HeapTuple	ruletuple;
-	char	   *ruleaction = NULL;
-	bool		action_is_null = false;
-	bool		instead_is_null = false;
-	Relation	ruleRelation = NULL;
-	TupleDesc	ruleTupdesc = NULL;
-	Query	   *ruleparse = NULL;
-	char	   *rule_evqual_string = NULL;
-	Node	   *rule_evqual = NULL;
-
-	ruleRelation = heap_openr(RewriteRelationName, AccessShareLock);
-	ruleTupdesc = RelationGetDescr(ruleRelation);
-	ruletuple = SearchSysCacheTuple(RULEOID,
-									ObjectIdGetDatum(ruleoid),
-									0, 0, 0);
-	if (ruletuple == NULL)
-		elog(ERROR, "rule %u isn't in rewrite system relation", ruleoid);
-
-	ruleaction = (char *) heap_getattr(ruletuple,
-									   Anum_pg_rewrite_ev_action,
-									   ruleTupdesc,
-									   &action_is_null);
-	rule_evqual_string = (char *) heap_getattr(ruletuple,
-											   Anum_pg_rewrite_ev_qual,
-										   ruleTupdesc, &action_is_null);
-	*instead_flag = !!heap_getattr(ruletuple,
-								   Anum_pg_rewrite_is_instead,
-								   ruleTupdesc, &instead_is_null);
-
-	if (action_is_null || instead_is_null)
-		elog(ERROR, "internal error: rewrite rule not properly set up");
-
-	ruleaction = textout((struct varlena *) ruleaction);
-	rule_evqual_string = textout((struct varlena *) rule_evqual_string);
-
-	ruleparse = (Query *) stringToNode(ruleaction);
-	rule_evqual = (Node *) stringToNode(rule_evqual_string);
-
-	heap_close(ruleRelation, AccessShareLock);
-
-	*parseTrees = ruleparse;
-	return rule_evqual;
-}
-
-#endif
 
 int
 IsDefinedRewriteRule(char *ruleName)
@@ -88,7 +33,20 @@ IsDefinedRewriteRule(char *ruleName)
 	return HeapTupleIsValid(tuple);
 }
 
-static void
+/*
+ * setRelhasrulesInRelation
+ *		Set the value of the relation's relhasrules field in pg_class.
+ *
+ * NOTE: caller should be holding an appropriate lock on the relation.
+ *
+ * NOTE: an important side-effect of this operation is that an SI invalidation
+ * message is sent out to all backends --- including me --- causing relcache
+ * entries to be flushed or updated with the new set of rules for the table.
+ * Therefore, we execute the update even if relhasrules has the right value
+ * already.  Possible future improvement: skip the disk update and just send
+ * an SI message in that case.
+ */
+void
 setRelhasrulesInRelation(Oid relationId, bool relhasrules)
 {
 	Relation	relationRelation;
@@ -96,9 +54,7 @@ setRelhasrulesInRelation(Oid relationId, bool relhasrules)
 	Relation	idescs[Num_pg_class_indices];
 
 	/*
-	 * Lock a relation given its Oid. Go to the RelationRelation (i.e.
-	 * pg_relation), find the appropriate tuple, and add the specified
-	 * lock to it.
+	 * Find the tuple to update in pg_class, using syscache for the lookup.
 	 */
 	relationRelation = heap_openr(RelationRelationName, RowExclusiveLock);
 	tuple = SearchSysCacheTupleCopy(RELOID,
@@ -106,10 +62,11 @@ setRelhasrulesInRelation(Oid relationId, bool relhasrules)
 									0, 0, 0);
 	Assert(HeapTupleIsValid(tuple));
 
+	/* Do the update */
 	((Form_pg_class) GETSTRUCT(tuple))->relhasrules = relhasrules;
 	heap_update(relationRelation, &tuple->t_self, tuple, NULL);
 
-	/* keep the catalog indices up to date */
+	/* Keep the catalog indices up to date */
 	CatalogOpenIndices(Num_pg_class_indices, Name_pg_class_indices, idescs);
 	CatalogIndexInsert(idescs, Num_pg_class_indices, relationRelation, tuple);
 	CatalogCloseIndices(Num_pg_class_indices, idescs);
@@ -117,120 +74,3 @@ setRelhasrulesInRelation(Oid relationId, bool relhasrules)
 	heap_freetuple(tuple);
 	heap_close(relationRelation, RowExclusiveLock);
 }
-
-void
-prs2_addToRelation(Oid relid,
-				   Oid ruleId,
-				   CmdType event_type,
-				   AttrNumber attno,
-				   bool isInstead,
-				   Node *qual,
-				   List *actions)
-{
-	Relation	relation;
-	RewriteRule *thisRule;
-	RuleLock   *rulelock;
-	MemoryContext oldcxt;
-
-	/*
-	 * create an in memory RewriteRule data structure which is cached by
-	 * every Relation descriptor. (see utils/cache/relcache.c)
-	 */
-	oldcxt = MemoryContextSwitchTo(CacheMemoryContext);
-	thisRule = (RewriteRule *) palloc(sizeof(RewriteRule));
-	if (qual != NULL)
-		qual = copyObject(qual);
-	if (actions != NIL)
-		actions = copyObject(actions);
-	MemoryContextSwitchTo(oldcxt);
-
-	thisRule->ruleId = ruleId;
-	thisRule->event = event_type;
-	thisRule->attrno = attno;
-	thisRule->qual = qual;
-	thisRule->actions = actions;
-	thisRule->isInstead = isInstead;
-
-	relation = heap_open(relid, AccessShareLock);
-
-	/*
-	 * modify or create a RuleLock cached by Relation
-	 */
-	if (relation->rd_rules == NULL)
-	{
-
-		oldcxt = MemoryContextSwitchTo(CacheMemoryContext);
-		rulelock = (RuleLock *) palloc(sizeof(RuleLock));
-		rulelock->numLocks = 1;
-		rulelock->rules = (RewriteRule **) palloc(sizeof(RewriteRule *));
-		rulelock->rules[0] = thisRule;
-		relation->rd_rules = rulelock;
-		MemoryContextSwitchTo(oldcxt);
-
-		/*
-		 * the fact that relation->rd_rules is NULL means the relhasrules
-		 * attribute of the tuple of this relation in pg_class is false.
-		 * We need to set it to true.
-		 */
-		setRelhasrulesInRelation(relid, TRUE);
-	}
-	else
-	{
-		int			numlock;
-
-		rulelock = relation->rd_rules;
-		numlock = rulelock->numLocks;
-		/* expand, for safety reasons */
-		oldcxt = MemoryContextSwitchTo(CacheMemoryContext);
-		rulelock->rules = (RewriteRule **) repalloc(rulelock->rules,
-								  sizeof(RewriteRule *) * (numlock + 1));
-		MemoryContextSwitchTo(oldcxt);
-		rulelock->rules[numlock] = thisRule;
-		rulelock->numLocks++;
-	}
-
-	heap_close(relation, AccessShareLock);
-}
-
-void
-prs2_deleteFromRelation(Oid relid, Oid ruleId)
-{
-	RuleLock   *rulelock;
-	Relation	relation;
-	int			numlock;
-	int			i;
-	MemoryContext oldcxt;
-
-	relation = heap_open(relid, AccessShareLock);
-	rulelock = relation->rd_rules;
-	Assert(rulelock != NULL);
-
-	numlock = rulelock->numLocks;
-	for (i = 0; i < numlock; i++)
-	{
-		if (rulelock->rules[i]->ruleId == ruleId)
-			break;
-	}
-	Assert(i < numlock);
-	oldcxt = MemoryContextSwitchTo(CacheMemoryContext);
-	pfree(rulelock->rules[i]);
-	MemoryContextSwitchTo(oldcxt);
-	if (numlock == 1)
-	{
-		relation->rd_rules = NULL;
-
-		/*
-		 * we don't have rules any more, flag the relhasrules attribute of
-		 * the tuple of this relation in pg_class false.
-		 */
-		setRelhasrulesInRelation(relid, FALSE);
-	}
-	else
-	{
-		rulelock->rules[i] = rulelock->rules[numlock - 1];
-		rulelock->rules[numlock - 1] = NULL;
-		rulelock->numLocks--;
-	}
-
-	heap_close(relation, AccessShareLock);
-}

src/include/catalog/index.h

@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2000, PostgreSQL, Inc
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: index.h,v 1.25 2000/06/17 23:41:51 tgl Exp $
+ * $Id: index.h,v 1.26 2000/06/30 07:04:06 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -17,7 +17,8 @@
 #include "access/itup.h"
 #include "nodes/execnodes.h"
 
-extern Form_pg_am AccessMethodObjectIdGetForm(Oid accessMethodObjectId);
+extern Form_pg_am AccessMethodObjectIdGetForm(Oid accessMethodObjectId,
+											  MemoryContext resultCxt);
 
 extern void UpdateIndexPredicate(Oid indexoid, Node *oldPred, Node *predicate);
 

src/include/rewrite/rewriteSupport.h

@@ -7,22 +7,15 @@
  * Portions Copyright (c) 1996-2000, PostgreSQL, Inc
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: rewriteSupport.h,v 1.11 2000/01/26 05:58:30 momjian Exp $
+ * $Id: rewriteSupport.h,v 1.12 2000/06/30 07:04:04 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
 #ifndef REWRITESUPPORT_H
 #define REWRITESUPPORT_H
 
-#include "access/attnum.h"
-#include "nodes/pg_list.h"
-
 extern int	IsDefinedRewriteRule(char *ruleName);
 
-extern void prs2_addToRelation(Oid relid, Oid ruleId, CmdType event_type,
-				   AttrNumber attno, bool isInstead, Node *qual,
-				   List *actions);
-extern void prs2_deleteFromRelation(Oid relid, Oid ruleId);
-
+extern void setRelhasrulesInRelation(Oid relationId, bool relhasrules);
 
 #endif	 /* REWRITESUPPORT_H */

src/include/utils/rel.h

@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2000, PostgreSQL, Inc
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: rel.h,v 1.38 2000/06/18 22:44:34 tgl Exp $
+ * $Id: rel.h,v 1.39 2000/06/30 07:04:03 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -85,7 +85,7 @@ typedef struct TriggerDesc
 
 typedef struct RelationData
 {
-	File		rd_fd;			/* open file descriptor */
+	File		rd_fd;			/* open file descriptor, or -1 if none */
 	int			rd_nblocks;		/* number of blocks in rel */
 	uint16		rd_refcnt;		/* reference count */
 	bool		rd_myxactonly;	/* rel uses the local buffer mgr */
@@ -99,6 +99,7 @@ typedef struct RelationData
 	LockInfoData rd_lockInfo;	/* lock mgr's info for locking relation */
 	TupleDesc	rd_att;			/* tuple descriptor */
 	RuleLock   *rd_rules;		/* rewrite rules */
+	MemoryContext rd_rulescxt;	/* private memory cxt for rd_rules, if any */
 	IndexStrategy rd_istrat;	/* info needed if rel is an index */
 	RegProcedure *rd_support;
 	TriggerDesc *trigdesc;		/* Trigger info, or NULL if rel has none */