Replace pg_class.reltriggers with relhastriggers, which is just a boolean hint

("there might be triggers") rather than an exact count.  This is necessary
catalog infrastructure for the upcoming patch to reduce the strength of
locking needed for trigger addition/removal.  Split out and committed
separately for ease of reviewing/testing.

In passing, also get rid of the unused pg_class columns relukeys, relfkeys,
and relrefs, which haven't been maintained in many years and now have no
chance of ever being maintained (because of wishing to avoid locking).

Simon Riggs

5 files changed, 54 insertions, 69 deletions

diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index f7585632e05..ad0dca13e1b 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/catalog/heap.c,v 1.342 2008/11/02 01:45:27 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/catalog/heap.c,v 1.343 2008/11/09 21:24:32 tgl Exp $
  *
  *
  * INTERFACE ROUTINES
@@ -641,13 +641,10 @@ InsertPgClassTuple(Relation pg_class_desc,
 	values[Anum_pg_class_relkind - 1] = CharGetDatum(rd_rel->relkind);
 	values[Anum_pg_class_relnatts - 1] = Int16GetDatum(rd_rel->relnatts);
 	values[Anum_pg_class_relchecks - 1] = Int16GetDatum(rd_rel->relchecks);
-	values[Anum_pg_class_reltriggers - 1] = Int16GetDatum(rd_rel->reltriggers);
-	values[Anum_pg_class_relukeys - 1] = Int16GetDatum(rd_rel->relukeys);
-	values[Anum_pg_class_relfkeys - 1] = Int16GetDatum(rd_rel->relfkeys);
-	values[Anum_pg_class_relrefs - 1] = Int16GetDatum(rd_rel->relrefs);
 	values[Anum_pg_class_relhasoids - 1] = BoolGetDatum(rd_rel->relhasoids);
 	values[Anum_pg_class_relhaspkey - 1] = BoolGetDatum(rd_rel->relhaspkey);
 	values[Anum_pg_class_relhasrules - 1] = BoolGetDatum(rd_rel->relhasrules);
+	values[Anum_pg_class_relhastriggers - 1] = BoolGetDatum(rd_rel->relhastriggers);
 	values[Anum_pg_class_relhassubclass - 1] = BoolGetDatum(rd_rel->relhassubclass);
 	values[Anum_pg_class_relfrozenxid - 1] = TransactionIdGetDatum(rd_rel->relfrozenxid);
 	/* start out with empty permissions */
@@ -2366,7 +2363,7 @@ heap_truncate_check_FKs(List *relations, bool tempTables)
 	{
 		Relation	rel = lfirst(cell);
 
-		if (rel->rd_rel->reltriggers != 0)
+		if (rel->rd_rel->relhastriggers)
 			oids = lappend_oid(oids, RelationGetRelid(rel));
 	}
 
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 46023c60b04..07c0809609d 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -3,7 +3,7 @@
  *
  * Copyright (c) 1996-2008, PostgreSQL Global Development Group
  *
- * $PostgreSQL: pgsql/src/backend/catalog/system_views.sql,v 1.55 2008/09/21 19:38:56 tgl Exp $
+ * $PostgreSQL: pgsql/src/backend/catalog/system_views.sql,v 1.56 2008/11/09 21:24:32 tgl Exp $
  */
 
 CREATE VIEW pg_roles AS 
@@ -84,7 +84,7 @@ CREATE VIEW pg_tables AS
         T.spcname AS tablespace,
         C.relhasindex AS hasindexes, 
         C.relhasrules AS hasrules, 
-        (C.reltriggers > 0) AS hastriggers 
+        C.relhastriggers AS hastriggers 
     FROM pg_class C LEFT JOIN pg_namespace N ON (N.oid = C.relnamespace) 
          LEFT JOIN pg_tablespace T ON (T.oid = C.reltablespace)
     WHERE C.relkind = 'r';
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index 53fb1995fc8..07c686c91da 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/commands/trigger.c,v 1.239 2008/11/02 01:45:28 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/commands/trigger.c,v 1.240 2008/11/09 21:24:32 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -95,7 +95,6 @@ CreateTrigger(CreateTrigStmt *stmt, Oid constraintOid)
 	Oid			funcoid;
 	Oid			funcrettype;
 	Oid			trigoid;
-	int			found = 0;
 	int			i;
 	char		constrtrigname[NAMEDATALEN];
 	char	   *trigname;
@@ -280,10 +279,9 @@ CreateTrigger(CreateTrigStmt *stmt, Oid constraintOid)
 	}
 
 	/*
-	 * Scan pg_trigger for existing triggers on relation.  We do this mainly
-	 * because we must count them; a secondary benefit is to give a nice error
-	 * message if there's already a trigger of the same name. (The unique
-	 * index on tgrelid/tgname would complain anyway.)
+	 * Scan pg_trigger for existing triggers on relation.  We do this only
+	 * to give a nice error message if there's already a trigger of the same
+	 * name.  (The unique index on tgrelid/tgname would complain anyway.)
 	 *
 	 * NOTE that this is cool only because we have AccessExclusiveLock on the
 	 * relation, so the trigger set won't be changing underneath us.
@@ -303,7 +301,6 @@ CreateTrigger(CreateTrigStmt *stmt, Oid constraintOid)
 					(errcode(ERRCODE_DUPLICATE_OBJECT),
 				  errmsg("trigger \"%s\" for relation \"%s\" already exists",
 						 trigname, stmt->relation->relname)));
-		found++;
 	}
 	systable_endscan(tgscan);
 
@@ -405,7 +402,7 @@ CreateTrigger(CreateTrigStmt *stmt, Oid constraintOid)
 		elog(ERROR, "cache lookup failed for relation %u",
 			 RelationGetRelid(rel));
 
-	((Form_pg_class) GETSTRUCT(tuple))->reltriggers = found + 1;
+	((Form_pg_class) GETSTRUCT(tuple))->relhastriggers = true;
 
 	simple_heap_update(pgrel, &tuple->t_self, tuple);
 
@@ -818,9 +815,6 @@ RemoveTriggerById(Oid trigOid)
 	HeapTuple	tup;
 	Oid			relid;
 	Relation	rel;
-	Relation	pgrel;
-	HeapTuple	tuple;
-	Form_pg_class classForm;
 
 	tgrel = heap_open(TriggerRelationId, RowExclusiveLock);
 
@@ -867,33 +861,15 @@ RemoveTriggerById(Oid trigOid)
 	heap_close(tgrel, RowExclusiveLock);
 
 	/*
-	 * Update relation's pg_class entry.  Crucial side-effect: other backends
-	 * (and this one too!) are sent SI message to make them rebuild relcache
-	 * entries.
-	 *
-	 * Note this is OK only because we have AccessExclusiveLock on the rel, so
-	 * no one else is creating/deleting triggers on this rel at the same time.
+	 * We do not bother to try to determine whether any other triggers remain,
+	 * which would be needed in order to decide whether it's safe to clear
+	 * the relation's relhastriggers.  (In any case, there might be a
+	 * concurrent process adding new triggers.)  Instead, just force a
+	 * relcache inval to make other backends (and this one too!) rebuild
+	 * their relcache entries.  There's no great harm in leaving relhastriggers
+	 * true even if there are no triggers left.
 	 */
-	pgrel = heap_open(RelationRelationId, RowExclusiveLock);
-	tuple = SearchSysCacheCopy(RELOID,
-							   ObjectIdGetDatum(relid),
-							   0, 0, 0);
-	if (!HeapTupleIsValid(tuple))
-		elog(ERROR, "cache lookup failed for relation %u", relid);
-	classForm = (Form_pg_class) GETSTRUCT(tuple);
-
-	if (classForm->reltriggers == 0)	/* should not happen */
-		elog(ERROR, "relation \"%s\" has reltriggers = 0",
-			 RelationGetRelationName(rel));
-	classForm->reltriggers--;
-
-	simple_heap_update(pgrel, &tuple->t_self, tuple);
-
-	CatalogUpdateIndexes(pgrel, tuple);
-
-	heap_freetuple(tuple);
-
-	heap_close(pgrel, RowExclusiveLock);
+	CacheInvalidateRelcache(rel);
 
 	/* Keep lock on trigger's rel until end of xact */
 	heap_close(rel, NoLock);
@@ -1134,18 +1110,23 @@ void
 RelationBuildTriggers(Relation relation)
 {
 	TriggerDesc *trigdesc;
-	int			ntrigs = relation->rd_rel->reltriggers;
+	int			numtrigs;
+	int			maxtrigs;
 	Trigger    *triggers;
-	int			found = 0;
 	Relation	tgrel;
 	ScanKeyData skey;
 	SysScanDesc tgscan;
 	HeapTuple	htup;
 	MemoryContext oldContext;
+	int			i;
 
-	Assert(ntrigs > 0);			/* else I should not have been called */
-
-	triggers = (Trigger *) palloc(ntrigs * sizeof(Trigger));
+	/*
+	 * Allocate a working array to hold the triggers (the array is extended
+	 * if necessary)
+	 */
+	maxtrigs = 16;
+	triggers = (Trigger *) palloc(maxtrigs * sizeof(Trigger));
+	numtrigs = 0;
 
 	/*
 	 * Note: since we scan the triggers using TriggerRelidNameIndexId, we will
@@ -1167,10 +1148,12 @@ RelationBuildTriggers(Relation relation)
 		Form_pg_trigger pg_trigger = (Form_pg_trigger) GETSTRUCT(htup);
 		Trigger    *build;
 
-		if (found >= ntrigs)
-			elog(ERROR, "too many trigger records found for relation \"%s\"",
-				 RelationGetRelationName(relation));
-		build = &(triggers[found]);
+		if (numtrigs >= maxtrigs)
+		{
+			maxtrigs *= 2;
+			triggers = (Trigger *) repalloc(triggers, maxtrigs * sizeof(Trigger));
+		}
+		build = &(triggers[numtrigs]);
 
 		build->tgoid = HeapTupleGetOid(htup);
 		build->tgname = DatumGetCString(DirectFunctionCall1(nameout,
@@ -1199,7 +1182,6 @@ RelationBuildTriggers(Relation relation)
 			bytea	   *val;
 			bool		isnull;
 			char	   *p;
-			int			i;
 
 			val = DatumGetByteaP(fastgetattr(htup,
 											 Anum_pg_trigger_tgargs,
@@ -1218,23 +1200,25 @@ RelationBuildTriggers(Relation relation)
 		else
 			build->tgargs = NULL;
 
-		found++;
+		numtrigs++;
 	}
 
 	systable_endscan(tgscan);
 	heap_close(tgrel, AccessShareLock);
 
-	if (found != ntrigs)
-		elog(ERROR, "%d trigger record(s) not found for relation \"%s\"",
-			 ntrigs - found,
-			 RelationGetRelationName(relation));
+	/* There might not be any triggers */
+	if (numtrigs == 0)
+	{
+		pfree(triggers);
+		return;
+	}
 
 	/* Build trigdesc */
 	trigdesc = (TriggerDesc *) palloc0(sizeof(TriggerDesc));
 	trigdesc->triggers = triggers;
-	trigdesc->numtriggers = ntrigs;
-	for (found = 0; found < ntrigs; found++)
-		InsertTrigger(trigdesc, &(triggers[found]), found);
+	trigdesc->numtriggers = numtrigs;
+	for (i = 0; i < numtrigs; i++)
+		InsertTrigger(trigdesc, &(triggers[i]), i);
 
 	/* Copy completed trigdesc into cache storage */
 	oldContext = MemoryContextSwitchTo(CacheMemoryContext);
diff --git a/src/backend/rewrite/rewriteDefine.c b/src/backend/rewrite/rewriteDefine.c
index 6f40944ef26..6212add6bcb 100644
--- a/src/backend/rewrite/rewriteDefine.c
+++ b/src/backend/rewrite/rewriteDefine.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/rewrite/rewriteDefine.c,v 1.131 2008/11/02 01:45:28 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/rewrite/rewriteDefine.c,v 1.132 2008/11/09 21:24:32 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -371,7 +371,11 @@ DefineQueryRewrite(char *rulename,
 		 *
 		 * If so, check that the relation is empty because the storage for the
 		 * relation is going to be deleted.  Also insist that the rel not have
-		 * any triggers, indexes, or child tables.
+		 * any triggers, indexes, or child tables.  (Note: these tests are
+		 * too strict, because they will reject relations that once had such
+		 * but don't anymore.  But we don't really care, because this whole
+		 * business of converting relations to views is just a kluge to allow
+		 * loading ancient pg_dump files.)
 		 */
 		if (event_relation->rd_rel->relkind != RELKIND_VIEW)
 		{
@@ -385,7 +389,7 @@ DefineQueryRewrite(char *rulename,
 								RelationGetRelationName(event_relation))));
 			heap_endscan(scanDesc);
 
-			if (event_relation->rd_rel->reltriggers != 0)
+			if (event_relation->rd_rel->relhastriggers)
 				ereport(ERROR,
 						(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
 						 errmsg("could not convert table \"%s\" to a view because it has triggers",
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index cec75ada720..92adfd8300d 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/cache/relcache.c,v 1.274 2008/09/30 10:52:13 heikki Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/cache/relcache.c,v 1.275 2008/11/09 21:24:32 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -856,7 +856,7 @@ RelationBuildDesc(Oid targetRelId, Relation oldrelation)
 		relation->rd_rulescxt = NULL;
 	}
 
-	if (relation->rd_rel->reltriggers > 0)
+	if (relation->rd_rel->relhastriggers)
 		RelationBuildTriggers(relation);
 	else
 		relation->trigdesc = NULL;
@@ -2641,7 +2641,7 @@ RelationCacheInitializePhase2(void)
 		 */
 		if (relation->rd_rel->relhasrules && relation->rd_rules == NULL)
 			RelationBuildRuleLock(relation);
-		if (relation->rd_rel->reltriggers > 0 && relation->trigdesc == NULL)
+		if (relation->rd_rel->relhastriggers && relation->trigdesc == NULL)
 			RelationBuildTriggers(relation);
 	}