From e4718f2c9eff30dedd022fac53a1f874aa1955d8 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sun, 9 Nov 2008 21:24:33 +0000 Subject: 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 --- src/backend/catalog/heap.c | 9 ++-- src/backend/catalog/system_views.sql | 4 +- src/backend/commands/trigger.c | 94 +++++++++++++++--------------------- src/backend/rewrite/rewriteDefine.c | 10 ++-- src/backend/utils/cache/relcache.c | 6 +-- 5 files changed, 54 insertions(+), 69 deletions(-) (limited to 'src/backend') 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); } -- cgit v1.2.3