Skip to content

Commit e4718f2

Browse files
tglsfdctglsfdc
committed
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
1 parent 1d577f5 commit e4718f2

File tree

13 files changed

+162
-197
lines changed

13 files changed

+162
-197
lines changed

‎doc/src/sgml/catalogs.sgml

Lines changed: 15 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
<!-- $PostgreSQL: pgsql/doc/src/sgml/catalogs.sgml,v 2.180 2008/10/31 08:39:19 heikki Exp $ -->
1+
<!-- $PostgreSQL: pgsql/doc/src/sgml/catalogs.sgml,v 2.181 2008/11/09 21:24:32 tgl Exp $ -->
22
<!--
33
Documentation of the system catalogs, directed toward PostgreSQL developers
44
-->
@@ -1711,37 +1711,6 @@
17111711
</entry>
17121712
</row>
17131713

1714-
<row>
1715-
<entry><structfield>reltriggers</structfield></entry>
1716-
<entry><type>int2</type></entry>
1717-
<entry></entry>
1718-
<entry>
1719-
Number of triggers on the table; see
1720-
<link linkend="catalog-pg-trigger"><structname>pg_trigger</structname></link> catalog
1721-
</entry>
1722-
</row>
1723-
1724-
<row>
1725-
<entry><structfield>relukeys</structfield></entry>
1726-
<entry><type>int2</type></entry>
1727-
<entry></entry>
1728-
<entry>Unused (<emphasis>not</emphasis> the number of unique keys)</entry>
1729-
</row>
1730-
1731-
<row>
1732-
<entry><structfield>relfkeys</structfield></entry>
1733-
<entry><type>int2</type></entry>
1734-
<entry></entry>
1735-
<entry>Unused (<emphasis>not</emphasis> the number of foreign keys on the table)</entry>
1736-
</row>
1737-
1738-
<row>
1739-
<entry><structfield>relrefs</structfield></entry>
1740-
<entry><type>int2</type></entry>
1741-
<entry></entry>
1742-
<entry>Unused</entry>
1743-
</row>
1744-
17451714
<row>
17461715
<entry><structfield>relhasoids</structfield></entry>
17471716
<entry><type>bool</type></entry>
@@ -1765,11 +1734,21 @@
17651734
<entry><type>bool</type></entry>
17661735
<entry></entry>
17671736
<entry>
1768-
True if table has rules; see
1737+
True if table has (or once had) rules; see
17691738
<link linkend="catalog-pg-rewrite"><structname>pg_rewrite</structname></link> catalog
17701739
</entry>
17711740
</row>
17721741

1742+
<row>
1743+
<entry><structfield>relhastriggers</structfield></entry>
1744+
<entry><type>bool</type></entry>
1745+
<entry></entry>
1746+
<entry>
1747+
True if table has (or once had) triggers; see
1748+
<link linkend="catalog-pg-trigger"><structname>pg_trigger</structname></link> catalog
1749+
</entry>
1750+
</row>
1751+
17731752
<row>
17741753
<entry><structfield>relhassubclass</structfield></entry>
17751754
<entry><type>bool</type></entry>
@@ -4499,13 +4478,6 @@
44994478
</para>
45004479
</note>
45014480

4502-
<note>
4503-
<para>
4504-
<literal>pg_class.reltriggers</literal> needs to agree with the
4505-
number of triggers found in this table for each relation.
4506-
</para>
4507-
</note>
4508-
45094481
</sect1>
45104482

45114483

@@ -6818,13 +6790,13 @@
68186790
<entry><structfield>hasrules</structfield></entry>
68196791
<entry><type>boolean</type></entry>
68206792
<entry><literal><link linkend="catalog-pg-class"><structname>pg_class</structname></link>.relhasrules</literal></entry>
6821-
<entry>true if table has rules</entry>
6793+
<entry>true if table has (or once had) rules</entry>
68226794
</row>
68236795
<row>
68246796
<entry><structfield>hastriggers</structfield></entry>
68256797
<entry><type>boolean</type></entry>
6826-
<entry><literal><link linkend="catalog-pg-class"><structname>pg_class</structname></link>.reltriggers</literal></entry>
6827-
<entry>true if table has triggers</entry>
6798+
<entry><literal><link linkend="catalog-pg-class"><structname>pg_class</structname></link>.relhastriggers</literal></entry>
6799+
<entry>true if table has (or once had) triggers</entry>
68286800
</row>
68296801
</tbody>
68306802
</tgroup>

‎src/backend/catalog/heap.c

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/catalog/heap.c,v 1.342 2008/11/02 01:45:27 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/catalog/heap.c,v 1.343 2008/11/09 21:24:32 tgl Exp $
1212
*
1313
*
1414
* INTERFACE ROUTINES
@@ -641,13 +641,10 @@ InsertPgClassTuple(Relation pg_class_desc,
641641
values[Anum_pg_class_relkind - 1] = CharGetDatum(rd_rel->relkind);
642642
values[Anum_pg_class_relnatts - 1] = Int16GetDatum(rd_rel->relnatts);
643643
values[Anum_pg_class_relchecks - 1] = Int16GetDatum(rd_rel->relchecks);
644-
values[Anum_pg_class_reltriggers - 1] = Int16GetDatum(rd_rel->reltriggers);
645-
values[Anum_pg_class_relukeys - 1] = Int16GetDatum(rd_rel->relukeys);
646-
values[Anum_pg_class_relfkeys - 1] = Int16GetDatum(rd_rel->relfkeys);
647-
values[Anum_pg_class_relrefs - 1] = Int16GetDatum(rd_rel->relrefs);
648644
values[Anum_pg_class_relhasoids - 1] = BoolGetDatum(rd_rel->relhasoids);
649645
values[Anum_pg_class_relhaspkey - 1] = BoolGetDatum(rd_rel->relhaspkey);
650646
values[Anum_pg_class_relhasrules - 1] = BoolGetDatum(rd_rel->relhasrules);
647+
values[Anum_pg_class_relhastriggers - 1] = BoolGetDatum(rd_rel->relhastriggers);
651648
values[Anum_pg_class_relhassubclass - 1] = BoolGetDatum(rd_rel->relhassubclass);
652649
values[Anum_pg_class_relfrozenxid - 1] = TransactionIdGetDatum(rd_rel->relfrozenxid);
653650
/* start out with empty permissions */
@@ -2366,7 +2363,7 @@ heap_truncate_check_FKs(List *relations, bool tempTables)
23662363
{
23672364
Relation rel = lfirst(cell);
23682365

2369-
if (rel->rd_rel->reltriggers != 0)
2366+
if (rel->rd_rel->relhastriggers)
23702367
oids = lappend_oid(oids, RelationGetRelid(rel));
23712368
}
23722369

‎src/backend/catalog/system_views.sql

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
*
44
* Copyright (c) 1996-2008, PostgreSQL Global Development Group
55
*
6-
* $PostgreSQL: pgsql/src/backend/catalog/system_views.sql,v 1.55 2008/09/21 19:38:56 tgl Exp $
6+
* $PostgreSQL: pgsql/src/backend/catalog/system_views.sql,v 1.56 2008/11/09 21:24:32 tgl Exp $
77
*/
88

99
CREATE VIEW pg_roles AS
@@ -84,7 +84,7 @@ CREATE VIEW pg_tables AS
8484
T.spcname AS tablespace,
8585
C.relhasindex AS hasindexes,
8686
C.relhasrules AS hasrules,
87-
(C.reltriggers > 0) AS hastriggers
87+
C.relhastriggers AS hastriggers
8888
FROM pg_class C LEFT JOIN pg_namespace N ON (N.oid = C.relnamespace)
8989
LEFT JOIN pg_tablespace T ON (T.oid = C.reltablespace)
9090
WHERE C.relkind = 'r';

‎src/backend/commands/trigger.c

Lines changed: 39 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1994, Regents of the University of California
88
*
99
* IDENTIFICATION
10-
* $PostgreSQL: pgsql/src/backend/commands/trigger.c,v 1.239 2008/11/02 01:45:28 tgl Exp $
10+
* $PostgreSQL: pgsql/src/backend/commands/trigger.c,v 1.240 2008/11/09 21:24:32 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -95,7 +95,6 @@ CreateTrigger(CreateTrigStmt *stmt, Oid constraintOid)
9595
Oid funcoid;
9696
Oid funcrettype;
9797
Oid trigoid;
98-
int found = 0;
9998
int i;
10099
char constrtrigname[NAMEDATALEN];
101100
char *trigname;
@@ -280,10 +279,9 @@ CreateTrigger(CreateTrigStmt *stmt, Oid constraintOid)
280279
}
281280

282281
/*
283-
* Scan pg_trigger for existing triggers on relation. We do this mainly
284-
* because we must count them; a secondary benefit is to give a nice error
285-
* message if there's already a trigger of the same name. (The unique
286-
* index on tgrelid/tgname would complain anyway.)
282+
* Scan pg_trigger for existing triggers on relation. We do this only
283+
* to give a nice error message if there's already a trigger of the same
284+
* name. (The unique index on tgrelid/tgname would complain anyway.)
287285
*
288286
* NOTE that this is cool only because we have AccessExclusiveLock on the
289287
* relation, so the trigger set won't be changing underneath us.
@@ -303,7 +301,6 @@ CreateTrigger(CreateTrigStmt *stmt, Oid constraintOid)
303301
(errcode(ERRCODE_DUPLICATE_OBJECT),
304302
errmsg("trigger \"%s\" for relation \"%s\" already exists",
305303
trigname, stmt->relation->relname)));
306-
found++;
307304
}
308305
systable_endscan(tgscan);
309306

@@ -405,7 +402,7 @@ CreateTrigger(CreateTrigStmt *stmt, Oid constraintOid)
405402
elog(ERROR, "cache lookup failed for relation %u",
406403
RelationGetRelid(rel));
407404

408-
((Form_pg_class) GETSTRUCT(tuple))->reltriggers = found + 1;
405+
((Form_pg_class) GETSTRUCT(tuple))->relhastriggers = true;
409406

410407
simple_heap_update(pgrel, &tuple->t_self, tuple);
411408

@@ -818,9 +815,6 @@ RemoveTriggerById(Oid trigOid)
818815
HeapTuple tup;
819816
Oid relid;
820817
Relation rel;
821-
Relation pgrel;
822-
HeapTuple tuple;
823-
Form_pg_class classForm;
824818

825819
tgrel = heap_open(TriggerRelationId, RowExclusiveLock);
826820

@@ -867,33 +861,15 @@ RemoveTriggerById(Oid trigOid)
867861
heap_close(tgrel, RowExclusiveLock);
868862

869863
/*
870-
* Update relation's pg_class entry. Crucial side-effect: other backends
871-
* (and this one too!) are sent SI message to make them rebuild relcache
872-
* entries.
873-
*
874-
* Note this is OK only because we have AccessExclusiveLock on the rel, so
875-
* no one else is creating/deleting triggers on this rel at the same time.
864+
* We do not bother to try to determine whether any other triggers remain,
865+
* which would be needed in order to decide whether it's safe to clear
866+
* the relation's relhastriggers. (In any case, there might be a
867+
* concurrent process adding new triggers.) Instead, just force a
868+
* relcache inval to make other backends (and this one too!) rebuild
869+
* their relcache entries. There's no great harm in leaving relhastriggers
870+
* true even if there are no triggers left.
876871
*/
877-
pgrel = heap_open(RelationRelationId, RowExclusiveLock);
878-
tuple = SearchSysCacheCopy(RELOID,
879-
ObjectIdGetDatum(relid),
880-
0, 0, 0);
881-
if (!HeapTupleIsValid(tuple))
882-
elog(ERROR, "cache lookup failed for relation %u", relid);
883-
classForm = (Form_pg_class) GETSTRUCT(tuple);
884-
885-
if (classForm->reltriggers == 0) /* should not happen */
886-
elog(ERROR, "relation \"%s\" has reltriggers = 0",
887-
RelationGetRelationName(rel));
888-
classForm->reltriggers--;
889-
890-
simple_heap_update(pgrel, &tuple->t_self, tuple);
891-
892-
CatalogUpdateIndexes(pgrel, tuple);
893-
894-
heap_freetuple(tuple);
895-
896-
heap_close(pgrel, RowExclusiveLock);
872+
CacheInvalidateRelcache(rel);
897873

898874
/* Keep lock on trigger's rel until end of xact */
899875
heap_close(rel, NoLock);
@@ -1134,18 +1110,23 @@ void
11341110
RelationBuildTriggers(Relation relation)
11351111
{
11361112
TriggerDesc *trigdesc;
1137-
int ntrigs = relation->rd_rel->reltriggers;
1113+
int numtrigs;
1114+
int maxtrigs;
11381115
Trigger *triggers;
1139-
int found = 0;
11401116
Relation tgrel;
11411117
ScanKeyData skey;
11421118
SysScanDesc tgscan;
11431119
HeapTuple htup;
11441120
MemoryContext oldContext;
1121+
int i;
11451122

1146-
Assert(ntrigs > 0); /* else I should not have been called */
1147-
1148-
triggers = (Trigger *) palloc(ntrigs * sizeof(Trigger));
1123+
/*
1124+
* Allocate a working array to hold the triggers (the array is extended
1125+
* if necessary)
1126+
*/
1127+
maxtrigs = 16;
1128+
triggers = (Trigger *) palloc(maxtrigs * sizeof(Trigger));
1129+
numtrigs = 0;
11491130

11501131
/*
11511132
* Note: since we scan the triggers using TriggerRelidNameIndexId, we will
@@ -1167,10 +1148,12 @@ RelationBuildTriggers(Relation relation)
11671148
Form_pg_trigger pg_trigger = (Form_pg_trigger) GETSTRUCT(htup);
11681149
Trigger *build;
11691150

1170-
if (found >= ntrigs)
1171-
elog(ERROR, "too many trigger records found for relation \"%s\"",
1172-
RelationGetRelationName(relation));
1173-
build = &(triggers[found]);
1151+
if (numtrigs >= maxtrigs)
1152+
{
1153+
maxtrigs *= 2;
1154+
triggers = (Trigger *) repalloc(triggers, maxtrigs * sizeof(Trigger));
1155+
}
1156+
build = &(triggers[numtrigs]);
11741157

11751158
build->tgoid = HeapTupleGetOid(htup);
11761159
build->tgname = DatumGetCString(DirectFunctionCall1(nameout,
@@ -1199,7 +1182,6 @@ RelationBuildTriggers(Relation relation)
11991182
bytea *val;
12001183
bool isnull;
12011184
char *p;
1202-
int i;
12031185

12041186
val = DatumGetByteaP(fastgetattr(htup,
12051187
Anum_pg_trigger_tgargs,
@@ -1218,23 +1200,25 @@ RelationBuildTriggers(Relation relation)
12181200
else
12191201
build->tgargs = NULL;
12201202

1221-
found++;
1203+
numtrigs++;
12221204
}
12231205

12241206
systable_endscan(tgscan);
12251207
heap_close(tgrel, AccessShareLock);
12261208

1227-
if (found != ntrigs)
1228-
elog(ERROR, "%d trigger record(s) not found for relation \"%s\"",
1229-
ntrigs - found,
1230-
RelationGetRelationName(relation));
1209+
/* There might not be any triggers */
1210+
if (numtrigs == 0)
1211+
{
1212+
pfree(triggers);
1213+
return;
1214+
}
12311215

12321216
/* Build trigdesc */
12331217
trigdesc = (TriggerDesc *) palloc0(sizeof(TriggerDesc));
12341218
trigdesc->triggers = triggers;
1235-
trigdesc->numtriggers = ntrigs;
1236-
for (found = 0; found < ntrigs; found++)
1237-
InsertTrigger(trigdesc, &(triggers[found]), found);
1219+
trigdesc->numtriggers = numtrigs;
1220+
for (i = 0; i < numtrigs; i++)
1221+
InsertTrigger(trigdesc, &(triggers[i]), i);
12381222

12391223
/* Copy completed trigdesc into cache storage */
12401224
oldContext = MemoryContextSwitchTo(CacheMemoryContext);

‎src/backend/rewrite/rewriteDefine.c

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/rewrite/rewriteDefine.c,v 1.131 2008/11/02 01:45:28 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/rewrite/rewriteDefine.c,v 1.132 2008/11/09 21:24:32 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -371,7 +371,11 @@ DefineQueryRewrite(char *rulename,
371371
*
372372
* If so, check that the relation is empty because the storage for the
373373
* relation is going to be deleted. Also insist that the rel not have
374-
* any triggers, indexes, or child tables.
374+
* any triggers, indexes, or child tables. (Note: these tests are
375+
* too strict, because they will reject relations that once had such
376+
* but don't anymore. But we don't really care, because this whole
377+
* business of converting relations to views is just a kluge to allow
378+
* loading ancient pg_dump files.)
375379
*/
376380
if (event_relation->rd_rel->relkind != RELKIND_VIEW)
377381
{
@@ -385,7 +389,7 @@ DefineQueryRewrite(char *rulename,
385389
RelationGetRelationName(event_relation))));
386390
heap_endscan(scanDesc);
387391

388-
if (event_relation->rd_rel->reltriggers != 0)
392+
if (event_relation->rd_rel->relhastriggers)
389393
ereport(ERROR,
390394
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
391395
errmsg("could not convert table \"%s\" to a view because it has triggers",

‎src/backend/utils/cache/relcache.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/utils/cache/relcache.c,v 1.274 2008/09/30 10:52:13 heikki Exp $
11+
* $PostgreSQL: pgsql/src/backend/utils/cache/relcache.c,v 1.275 2008/11/09 21:24:32 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -856,7 +856,7 @@ RelationBuildDesc(Oid targetRelId, Relation oldrelation)
856856
relation->rd_rulescxt = NULL;
857857
}
858858

859-
if (relation->rd_rel->reltriggers > 0)
859+
if (relation->rd_rel->relhastriggers)
860860
RelationBuildTriggers(relation);
861861
else
862862
relation->trigdesc = NULL;
@@ -2641,7 +2641,7 @@ RelationCacheInitializePhase2(void)
26412641
*/
26422642
if (relation->rd_rel->relhasrules && relation->rd_rules == NULL)
26432643
RelationBuildRuleLock(relation);
2644-
if (relation->rd_rel->reltriggers > 0 && relation->trigdesc == NULL)
2644+
if (relation->rd_rel->relhastriggers && relation->trigdesc == NULL)
26452645
RelationBuildTriggers(relation);
26462646
}
26472647

0 commit comments

Comments
 (0)