Here's another version of this patch. It is virtually identical to the previous one, except for a small doc update and whitespace changes.
To recap: when a row-level trigger is created on a partitioned table, it is marked tginherits; partitions all have their pg_class row modified with relhastriggers=true. No clone of the pg_trigger row is created for the partitions. Instead, when the relcache entry for the partition is created, pg_trigger is scanned to look for entries for its ancestors. So the trigger list for a partition is created by repeatedly scanning pg_trigger and pg_inherits, until only entries with relhastriggers=f are found. I reserve the right to revise this further, as I'm going to spend a couple of hours looking at it this afternoon, particularly to see how concurrent DDL behaves, but I don't see anything obviously wrong with it. Robert Haas wrote: > Elsewhere, we've put a lot of blood, sweat, and tears into making sure > that we only traverse the inheritance hierarchy from top to bottom. > Otherwise, we're adding deadlock hazards. I think it's categorically > unacceptable to do traversals in the opposite order -- if you do, then > an UPDATE on a child could deadlock with a LOCK TABLE on the parent. > That will not win us any awards. We don't actually open relations or acquire locks in the traversal I was talking about, though; the only thing we do is scan pg_trigger using first the partition relid, then seek the ancestor(s) by scanning pg_inherits and recurse. We don't acquire locks on the involved relations, so there should be no danger of deadlocks. Changes in the definitions ought to be handled by the cache invalidations that are sent, although I admit to not having tested this specifically. I'll do that later today. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml index a0e6d7062b..4887878eec 100644 --- a/doc/src/sgml/catalogs.sgml +++ b/doc/src/sgml/catalogs.sgml @@ -1873,7 +1873,9 @@ SCRAM-SHA-256$<replaceable><iteration count></replaceable>:<replaceable>&l <entry></entry> <entry> True if table has (or once had) triggers; see - <link linkend="catalog-pg-trigger"><structname>pg_trigger</structname></link> catalog + <link linkend="catalog-pg-trigger"><structname>pg_trigger</structname></link> catalog. + If this is a partition, triggers on its partitioned ancestors are also + considered </entry> </row> @@ -6988,6 +6990,13 @@ SCRAM-SHA-256$<replaceable><iteration count></replaceable>:<replaceable>&l </row> <row> + <entry><structfield>tginherits</structfield></entry> + <entry><type>bool</type></entry> + <entry></entry> + <entry>True if trigger applies to children relations too</entry> + </row> + + <row> <entry><structfield>tgnargs</structfield></entry> <entry><type>int2</type></entry> <entry></entry> diff --git a/doc/src/sgml/ref/create_trigger.sgml b/doc/src/sgml/ref/create_trigger.sgml index 3d6b9f033c..901264c6d2 100644 --- a/doc/src/sgml/ref/create_trigger.sgml +++ b/doc/src/sgml/ref/create_trigger.sgml @@ -504,7 +504,9 @@ UPDATE OF <replaceable>column_name1</replaceable> [, <replaceable>column_name2</ statement-level triggers attached to the explicitly named table, but not statement-level triggers for its partitions or child tables. In contrast, row-level triggers are fired on the rows in affected partitions or - child tables, even if they are not explicitly named in the query. + child tables, even if they are not explicitly named in the query; + also, row-level triggers defined on partitioned tables are fired when + rows are modified in its partitions. If a statement-level trigger has been defined with transition relations named by a <literal>REFERENCING</literal> clause, then before and after images of rows are visible from all affected partitions or child tables. diff --git a/src/backend/bootstrap/bootparse.y b/src/backend/bootstrap/bootparse.y index ed7a55596f..7ad0126df5 100644 --- a/src/backend/bootstrap/bootparse.y +++ b/src/backend/bootstrap/bootparse.y @@ -230,6 +230,7 @@ Boot_CreateStmt: RELPERSISTENCE_PERMANENT, shared_relation, mapped_relation, + false, true); elog(DEBUG4, "bootstrap relation created"); } @@ -252,6 +253,7 @@ Boot_CreateStmt: mapped_relation, true, 0, + false, ONCOMMIT_NOOP, (Datum) 0, false, diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index cf36ce4add..815f371ac2 100644 --- a/src/backend/catalog/heap.c +++ b/src/backend/catalog/heap.c @@ -257,6 +257,7 @@ heap_create(const char *relname, char relpersistence, bool shared_relation, bool mapped_relation, + bool has_triggers, bool allow_system_table_mods) { bool create_storage; @@ -351,7 +352,8 @@ heap_create(const char *relname, shared_relation, mapped_relation, relpersistence, - relkind); + relkind, + has_triggers); /* * Have the storage manager create the relation's disk file, if needed. @@ -1005,6 +1007,7 @@ AddNewRelationType(const char *typeName, * mapped_relation: true if the relation will use the relfilenode map * oidislocal: true if oid column (if any) should be marked attislocal * oidinhcount: attinhcount to assign to oid column (if any) + * hastriggers: value to set relhastriggers to * oncommit: ON COMMIT marking (only relevant if it's a temp table) * reloptions: reloptions in Datum form, or (Datum) 0 if none * use_user_acl: true if should look for user-defined default permissions; @@ -1034,6 +1037,7 @@ heap_create_with_catalog(const char *relname, bool mapped_relation, bool oidislocal, int oidinhcount, + bool hastriggers, OnCommitAction oncommit, Datum reloptions, bool use_user_acl, @@ -1173,6 +1177,7 @@ heap_create_with_catalog(const char *relname, relpersistence, shared_relation, mapped_relation, + hastriggers, allow_system_table_mods); Assert(relid == RelationGetRelid(new_rel_desc)); diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index 431bc31969..c33d2570bd 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -903,6 +903,7 @@ index_create(Relation heapRelation, relpersistence, shared_relation, mapped_relation, + false, allow_system_table_mods); Assert(indexRelationId == RelationGetRelid(indexRelation)); diff --git a/src/backend/catalog/toasting.c b/src/backend/catalog/toasting.c index 8bf2698545..363f39e7fe 100644 --- a/src/backend/catalog/toasting.c +++ b/src/backend/catalog/toasting.c @@ -274,6 +274,7 @@ create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid, mapped_relation, true, 0, + false, ONCOMMIT_NOOP, reloptions, false, diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index 5d481dd50d..cd78f12f19 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -687,6 +687,7 @@ make_new_heap(Oid OIDOldHeap, Oid NewTableSpace, char relpersistence, RelationIsMapped(OldHeap), true, 0, + OldHeap->rd_rel->relhastriggers, /* XXX why? */ ONCOMMIT_NOOP, reloptions, false, diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 74e020bffc..344eecda7a 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -303,7 +303,7 @@ struct DropRelationCallbackState static void truncate_check_rel(Relation rel); static List *MergeAttributes(List *schema, List *supers, char relpersistence, bool is_partition, List **supOids, List **supconstr, - int *supOidCount); + int *supOidCount, bool *hastriggers); static bool MergeCheckConstraint(List *constraints, char *name, Node *expr); static void MergeAttributesIntoExisting(Relation child_rel, Relation parent_rel); static void MergeConstraintsIntoExisting(Relation child_rel, Relation parent_rel); @@ -527,8 +527,10 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId, List *old_constraints; bool localHasOids; int parentOidCount; + bool hastriggers; List *rawDefaults; List *cookedDefaults; + bool is_partition; Datum reloptions; ListCell *listptr; AttrNumber attnum; @@ -559,6 +561,8 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId, relkind = RELKIND_PARTITIONED_TABLE; } + is_partition = stmt->partbound != NULL; + /* * Look up the namespace in which we are supposed to create the relation, * check we have permission to create there, lock it against concurrent @@ -647,8 +651,9 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId, stmt->tableElts = MergeAttributes(stmt->tableElts, stmt->inhRelations, stmt->relation->relpersistence, - stmt->partbound != NULL, - &inheritOids, &old_constraints, &parentOidCount); + is_partition, + &inheritOids, &old_constraints, &parentOidCount, + &hastriggers); /* * Create a tuple descriptor from the relation schema. Note that this @@ -675,7 +680,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId, * If a partitioned table doesn't have the system OID column, then none of * its partitions should have it. */ - if (stmt->partbound && parentOidCount == 0 && localHasOids) + if (is_partition && parentOidCount == 0 && localHasOids) ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), errmsg("cannot create table with OIDs as partition of table without OIDs"))); @@ -759,6 +764,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId, false, localHasOids, parentOidCount, + hastriggers, stmt->oncommit, reloptions, true, @@ -767,7 +773,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId, typaddress); /* Store inheritance information for new rel. */ - StoreCatalogInheritance(relationId, inheritOids, stmt->partbound != NULL); + StoreCatalogInheritance(relationId, inheritOids, is_partition); /* * We must bump the command counter to make the newly-created relation @@ -784,7 +790,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId, rel = relation_open(relationId, AccessExclusiveLock); /* Process and store partition bound, if any. */ - if (stmt->partbound) + if (is_partition) { PartitionBoundSpec *bound; ParseState *pstate; @@ -920,7 +926,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId, * the parent. We can't do it earlier, because DefineIndex wants to know * the partition key which we just stored. */ - if (stmt->partbound) + if (is_partition) { Oid parentId = linitial_oid(inheritOids); Relation parent; @@ -1692,6 +1698,8 @@ storage_name(char c) * 'supconstr' receives a list of constraints belonging to the parents, * updated as necessary to be valid for the child. * 'supOidCount' is set to the number of parents that have OID columns. + * 'hasTriggers' is set to true if any parent has inheritable triggers, + * false otherwise. * * Return value: * Completed schema list. @@ -1738,7 +1746,7 @@ storage_name(char c) static List * MergeAttributes(List *schema, List *supers, char relpersistence, bool is_partition, List **supOids, List **supconstr, - int *supOidCount) + int *supOidCount, bool *hasTriggers) { ListCell *entry; List *inhSchema = NIL; @@ -1750,6 +1758,8 @@ MergeAttributes(List *schema, List *supers, char relpersistence, static Node bogus_marker = {0}; /* marks conflicting defaults */ List *saved_schema = NIL; + *hasTriggers = false; + /* * Check for and reject tables with too many columns. We perform this * check relatively early for two reasons: (a) we don't run the risk of @@ -2147,6 +2157,23 @@ MergeAttributes(List *schema, List *supers, char relpersistence, pfree(newattno); /* + * If this parent has triggers, and any of them is marked inheritable, + * set *hastriggers. + */ + if (relation->rd_rel->relhastriggers && + relation->trigdesc != NULL && + !*hasTriggers) + { + int trg; + + for (trg = 0; trg < relation->trigdesc->numtriggers; trg++) + { + if (relation->trigdesc->triggers[trg].tginherits) + *hasTriggers = true; + } + } + + /* * Close the parent rel, but keep our lock on it until xact commit. * That will prevent someone else from deleting or ALTERing the parent * before the child is committed. @@ -14248,6 +14275,9 @@ AttachPartitionEnsureIndexes(Relation rel, Relation attachrel) index_close(idxRel, AccessShareLock); } + /* Make this all visible */ + CommandCounterIncrement(); + /* Clean up. */ for (i = 0; i < list_length(attachRelIdxs); i++) index_close(attachrelIdxRels[i], AccessShareLock); diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index fbd176b5d0..e3e46814f9 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -24,6 +24,7 @@ #include "catalog/objectaccess.h" #include "catalog/pg_constraint.h" #include "catalog/pg_constraint_fn.h" +#include "catalog/pg_inherits.h" #include "catalog/pg_inherits_fn.h" #include "catalog/pg_proc.h" #include "catalog/pg_trigger.h" @@ -100,7 +101,17 @@ static void AfterTriggerSaveEvent(EState *estate, ResultRelInfo *relinfo, List *recheckIndexes, Bitmapset *modifiedCols, TransitionCaptureState *transition_capture); static void AfterTriggerEnlargeQueryState(void); +static void SendTriggerRelcacheInval(Relation inheritsRel, Relation rel); static bool before_stmt_triggers_fired(Oid relid, CmdType cmdType); +static void recursively_update_relhastriggers(Relation pg_class, Oid relid, + bool recurse); +static void add_triggers_to_array(Relation tgrel, Relation inhrel, + Oid tgrelid, bool all_triggers, + Trigger **triggers, int *numtrigs, int *maxtrigs, + bool *must_sort); +static void add_trigger_to_array(TupleDesc tgdesc, HeapTuple tgtup, + Trigger **triggers, int *numtrigs, int *maxtrigs); +static int qsort_trigger_cmp(const void *a, const void *b); /* @@ -133,6 +144,9 @@ static bool before_stmt_triggers_fired(Oid relid, CmdType cmdType); * relation, as well as ACL_EXECUTE on the trigger function. For internal * triggers the caller must apply any required permission checks. * + * When called on partitioned tables, FOR EACH ROW triggers are marked as + * applying on partitions too (ie. tginherits), except if isInternal. + * * Note: can return InvalidObjectAddress if we decided to not create a trigger * at all, but a foreign-key constraint. This is a kluge for backwards * compatibility. @@ -149,6 +163,7 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString, Node *whenClause; List *whenRtable; char *qual; + bool tginherits; Datum values[Natts_pg_trigger]; bool nulls[Natts_pg_trigger]; Relation rel; @@ -179,8 +194,7 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString, * Triggers must be on tables or views, and there are additional * relation-type-specific restrictions. */ - if (rel->rd_rel->relkind == RELKIND_RELATION || - rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) + if (rel->rd_rel->relkind == RELKIND_RELATION) { /* Tables can't have INSTEAD OF triggers */ if (stmt->timing != TRIGGER_TYPE_BEFORE && @@ -190,13 +204,69 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString, errmsg("\"%s\" is a table", RelationGetRelationName(rel)), errdetail("Tables cannot have INSTEAD OF triggers."))); - /* Disallow ROW triggers on partitioned tables */ - if (stmt->row && rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) + } + else if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) + { + /* Partitioned tables can't have INSTEAD OF triggers */ + if (stmt->timing != TRIGGER_TYPE_BEFORE && + stmt->timing != TRIGGER_TYPE_AFTER) ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), - errmsg("\"%s\" is a partitioned table", + errmsg("\"%s\" is a table", RelationGetRelationName(rel)), - errdetail("Partitioned tables cannot have ROW triggers."))); + errdetail("Tables cannot have INSTEAD OF triggers."))); + /* + * FOR EACH ROW triggers have further restrictions + */ + if (stmt->row) + { + /* + * Disallow WHEN clauses; I think it's okay, but disallow for now + * to reduce testing surface. + */ + if (stmt->whenClause) + ereport(ERROR, + (errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg("\"%s\" is a partitioned table", + RelationGetRelationName(rel)), + errdetail("Triggers FOR EACH ROW on partitioned table cannot have WHEN clauses."))); + + /* + * BEFORE triggers FOR EACH ROW are forbidden, because they would + * allow the user to direct the row to another partition, which + * isn't implemented in the executor. + */ + if (stmt->timing != TRIGGER_TYPE_AFTER) + ereport(ERROR, + (errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg("\"%s\" is a partitioned table", + RelationGetRelationName(rel)), + errdetail("Partitioned tables cannot have BEFORE / FOR EACH ROW triggers."))); + + /* + * Constraint triggers are not allowed, either. + */ + if (stmt->isconstraint) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("\"%s\" is a partitioned table", + RelationGetRelationName(rel)), + errdetail("Partitioned tables cannot have CONSTRAINT triggers FOR EACH ROW."))); + + /* + * Disallow use of transition tables. If this partitioned table + * has any partitions, the error would occur below; but if it + * doesn't then we would only hit that code when the first CREATE + * TABLE ... PARTITION OF is executed, which is too late. Check + * early to avoid the problem. + */ + if (stmt->transitionRels != NIL) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("\"%s\" is a partitioned table", + RelationGetRelationName(rel)), + errdetail("Triggers on partitioned tables cannot have transition tables."))); + } } else if (rel->rd_rel->relkind == RELKIND_VIEW) { @@ -676,6 +746,12 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString, } /* + * FOR EACH ROW triggers in partitioned tables are marked inheritable. + */ + tginherits = (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE && + TRIGGER_FOR_ROW(tgtype)); + + /* * Generate the trigger's OID now, so that we can use it in the name if * needed. */ @@ -748,6 +824,7 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString, values[Anum_pg_trigger_tgconstraint - 1] = ObjectIdGetDatum(constraintOid); values[Anum_pg_trigger_tgdeferrable - 1] = BoolGetDatum(stmt->deferrable); values[Anum_pg_trigger_tginitdeferred - 1] = BoolGetDatum(stmt->initdeferred); + values[Anum_pg_trigger_tginherits - 1] = BoolGetDatum(tginherits); if (stmt->args) { @@ -872,22 +949,15 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString, pfree(DatumGetPointer(values[Anum_pg_trigger_tgnewtable - 1])); /* - * Update relation's pg_class entry. Crucial side-effect: other backends + * Update relation's pg_class entry -- and that of each child relation, + * if the trigger is inheritable. Crucial side-effect: other backends * (and this one too!) are sent SI message to make them rebuild relcache * entries. */ pgrel = heap_open(RelationRelationId, RowExclusiveLock); - tuple = SearchSysCacheCopy1(RELOID, - ObjectIdGetDatum(RelationGetRelid(rel))); - if (!HeapTupleIsValid(tuple)) - elog(ERROR, "cache lookup failed for relation %u", - RelationGetRelid(rel)); - ((Form_pg_class) GETSTRUCT(tuple))->relhastriggers = true; + recursively_update_relhastriggers(pgrel, RelationGetRelid(rel), tginherits); - CatalogTupleUpdate(pgrel, &tuple->t_self, tuple); - - heap_freetuple(tuple); heap_close(pgrel, RowExclusiveLock); /* @@ -933,6 +1003,7 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString, referenced.objectId = RelationGetRelid(rel); referenced.objectSubId = 0; recordDependencyOn(&myself, &referenced, DEPENDENCY_AUTO); + if (OidIsValid(constrrelid)) { referenced.classId = RelationRelationId; @@ -988,7 +1059,6 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString, return myself; } - /* * Convert legacy (pre-7.3) CREATE CONSTRAINT TRIGGER commands into * full-fledged foreign key constraints. @@ -1337,7 +1407,7 @@ RemoveTriggerById(Oid trigOid) * There's no great harm in leaving relhastriggers true even if there are * no triggers left. */ - CacheInvalidateRelcache(rel); + SendTriggerRelcacheInval(NULL, rel); /* Keep lock on trigger's rel until end of xact */ heap_close(rel, NoLock); @@ -1535,7 +1605,7 @@ renametrig(RenameStmt *stmt) * this one too!) are sent SI message to make them rebuild relcache * entries. (Ideally this should happen automatically...) */ - CacheInvalidateRelcache(targetrel); + SendTriggerRelcacheInval(NULL, targetrel); } else { @@ -1665,7 +1735,7 @@ EnableDisableTrigger(Relation rel, const char *tgname, * Otherwise they will fail to apply the change promptly. */ if (changed) - CacheInvalidateRelcache(rel); + SendTriggerRelcacheInval(NULL, rel); } @@ -1687,11 +1757,10 @@ RelationBuildTriggers(Relation relation) int maxtrigs; Trigger *triggers; Relation tgrel; - ScanKeyData skey; - SysScanDesc tgscan; - HeapTuple htup; + Relation inhrel; MemoryContext oldContext; int i; + bool must_sort = false; /* * Allocate a working array to hold the triggers (the array is extended if @@ -1701,108 +1770,14 @@ RelationBuildTriggers(Relation relation) triggers = (Trigger *) palloc(maxtrigs * sizeof(Trigger)); numtrigs = 0; - /* - * Note: since we scan the triggers using TriggerRelidNameIndexId, we will - * be reading the triggers in name order, except possibly during - * emergency-recovery operations (ie, IgnoreSystemIndexes). This in turn - * ensures that triggers will be fired in name order. - */ - ScanKeyInit(&skey, - Anum_pg_trigger_tgrelid, - BTEqualStrategyNumber, F_OIDEQ, - ObjectIdGetDatum(RelationGetRelid(relation))); - tgrel = heap_open(TriggerRelationId, AccessShareLock); - tgscan = systable_beginscan(tgrel, TriggerRelidNameIndexId, true, - NULL, 1, &skey); + inhrel = heap_open(InheritsRelationId, AccessShareLock); - while (HeapTupleIsValid(htup = systable_getnext(tgscan))) - { - Form_pg_trigger pg_trigger = (Form_pg_trigger) GETSTRUCT(htup); - Trigger *build; - Datum datum; - bool isnull; + add_triggers_to_array(tgrel, inhrel, RelationGetRelid(relation), + true, &triggers, &numtrigs, &maxtrigs, &must_sort); - if (numtrigs >= maxtrigs) - { - maxtrigs *= 2; - triggers = (Trigger *) repalloc(triggers, maxtrigs * sizeof(Trigger)); - } - build = &(triggers[numtrigs]); - - build->tgoid = HeapTupleGetOid(htup); - build->tgname = DatumGetCString(DirectFunctionCall1(nameout, - NameGetDatum(&pg_trigger->tgname))); - build->tgfoid = pg_trigger->tgfoid; - build->tgtype = pg_trigger->tgtype; - build->tgenabled = pg_trigger->tgenabled; - build->tgisinternal = pg_trigger->tgisinternal; - build->tgconstrrelid = pg_trigger->tgconstrrelid; - build->tgconstrindid = pg_trigger->tgconstrindid; - build->tgconstraint = pg_trigger->tgconstraint; - build->tgdeferrable = pg_trigger->tgdeferrable; - build->tginitdeferred = pg_trigger->tginitdeferred; - build->tgnargs = pg_trigger->tgnargs; - /* tgattr is first var-width field, so OK to access directly */ - build->tgnattr = pg_trigger->tgattr.dim1; - if (build->tgnattr > 0) - { - build->tgattr = (int16 *) palloc(build->tgnattr * sizeof(int16)); - memcpy(build->tgattr, &(pg_trigger->tgattr.values), - build->tgnattr * sizeof(int16)); - } - else - build->tgattr = NULL; - if (build->tgnargs > 0) - { - bytea *val; - char *p; - - val = DatumGetByteaPP(fastgetattr(htup, - Anum_pg_trigger_tgargs, - tgrel->rd_att, &isnull)); - if (isnull) - elog(ERROR, "tgargs is null in trigger for relation \"%s\"", - RelationGetRelationName(relation)); - p = (char *) VARDATA_ANY(val); - build->tgargs = (char **) palloc(build->tgnargs * sizeof(char *)); - for (i = 0; i < build->tgnargs; i++) - { - build->tgargs[i] = pstrdup(p); - p += strlen(p) + 1; - } - } - else - build->tgargs = NULL; - - datum = fastgetattr(htup, Anum_pg_trigger_tgoldtable, - tgrel->rd_att, &isnull); - if (!isnull) - build->tgoldtable = - DatumGetCString(DirectFunctionCall1(nameout, datum)); - else - build->tgoldtable = NULL; - - datum = fastgetattr(htup, Anum_pg_trigger_tgnewtable, - tgrel->rd_att, &isnull); - if (!isnull) - build->tgnewtable = - DatumGetCString(DirectFunctionCall1(nameout, datum)); - else - build->tgnewtable = NULL; - - datum = fastgetattr(htup, Anum_pg_trigger_tgqual, - tgrel->rd_att, &isnull); - if (!isnull) - build->tgqual = TextDatumGetCString(datum); - else - build->tgqual = NULL; - - numtrigs++; - } - - systable_endscan(tgscan); heap_close(tgrel, AccessShareLock); + heap_close(inhrel, AccessShareLock); /* There might not be any triggers */ if (numtrigs == 0) @@ -1811,6 +1786,10 @@ RelationBuildTriggers(Relation relation) return; } + /* apply a final sort step, if needed */ + if (must_sort) + qsort(triggers, numtrigs, sizeof(Trigger), qsort_trigger_cmp); + /* Build trigdesc */ trigdesc = (TriggerDesc *) palloc0(sizeof(TriggerDesc)); trigdesc->triggers = triggers; @@ -5742,6 +5721,51 @@ AfterTriggerSaveEvent(EState *estate, ResultRelInfo *relinfo, } /* + * SendTriggerRelcacheInvals + * Send inval signals for this rel and all its descendants. + */ +static void +SendTriggerRelcacheInval(Relation inheritsRel, Relation rel) +{ + ScanKeyData skey; + bool opened = false; + SysScanDesc scan; + HeapTuple inhtup; + + CacheInvalidateRelcache(rel); + + if (!rel->rd_rel->relhassubclass) + return; + + if (inheritsRel == NULL) + { + inheritsRel = heap_open(InheritsRelationId, AccessShareLock); + opened = true; + } + + ScanKeyInit(&skey, + Anum_pg_inherits_inhparent, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(RelationGetRelid(rel))); + scan = systable_beginscan(inheritsRel, InheritsParentIndexId, true, NULL, + 1, &skey); + while ((inhtup = systable_getnext(scan)) != NULL) + { + Oid child = ((Form_pg_inherits) GETSTRUCT(inhtup))->inhrelid; + Relation childrel; + + childrel = heap_open(child, AccessShareLock); + SendTriggerRelcacheInval(inheritsRel, childrel); + heap_close(childrel, AccessShareLock); + } + + systable_endscan(scan); + + if (opened) + heap_close(inheritsRel, AccessShareLock); +} + +/* * Detect whether we already queued BEFORE STATEMENT triggers for the given * relation + operation, and set the flag so the next call will report "true". */ @@ -5864,6 +5888,259 @@ done: } /* + * Update the relhastriggers for the relation with 'relid'; and there are any + * descendents, recurse to update it on those too, if the 'recurse' flag is + * true. + */ +static void +recursively_update_relhastriggers(Relation pg_class, Oid relid, bool recurse) +{ + HeapTuple classTup; + Form_pg_class classForm; + + classTup = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(relid)); + if (!HeapTupleIsValid(classTup)) + elog(ERROR, "cache lookup failed for relation %u", relid); + classForm = (Form_pg_class) GETSTRUCT(classTup); + + /* + * Update the relation's relhastriggers flag, if necessary. If we don't + * do it, make sure to send an inval message anyway. + */ + if (!classForm->relhastriggers) + { + classForm->relhastriggers = true; + CatalogTupleUpdate(pg_class, &classTup->t_self, classTup); + } + else + CacheInvalidateRelcacheByTuple(classTup); + + /* + * Recurse to update the children flag, if there are any. + * + * You may be tempted to merge this with the above, thinking that if the + * parent already had relhastriggers then children must be OK too -- but + * that is wrong if the parent already had a non-inheritable trigger and + * now has an inheritable one. + */ + if (recurse && classForm->relhassubclass) + { + ScanKeyData key; + Relation inhRel; + SysScanDesc scan; + HeapTuple inhTup; + + inhRel = heap_open(InheritsRelationId, AccessShareLock); + ScanKeyInit(&key, + Anum_pg_inherits_inhparent, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(relid)); + scan = systable_beginscan(inhRel, InheritsParentIndexId, true, + NULL, 1, &key); + + while (HeapTupleIsValid(inhTup = systable_getnext(scan))) + { + Form_pg_inherits inhForm = (Form_pg_inherits) GETSTRUCT(inhTup); + + recursively_update_relhastriggers(pg_class, inhForm->inhrelid, recurse); + } + systable_endscan(scan); + heap_close(inhRel, AccessShareLock); + } + + heap_freetuple(classTup); +} + +/* + * Search for triggers on the relation with oid 'tgrelid', and add any that are + * found to the array 'triggers' ('numtrigs' is the number of used elements, + * 'maxtrigs' the number of allocated elements). If 'all_triggers' is false, + * only consider triggers that have tginherits true; otherwise include all + * triggers. + * + * For each parent of this relation, recurse to do the same, passing + * 'all_triggers' false. + * + * 'tgrel' is the pg_triggers relation. + * + * 'must_sort' is set to true if the order of the triggers in the output array + * is not guaranteed; caller must sort afterwards in that case. + */ +static void +add_triggers_to_array(Relation tgrel, Relation inhrel, + Oid tgrelid, bool all_triggers, + Trigger **triggers, int *numtrigs, int *maxtrigs, + bool *must_sort) +{ + SysScanDesc scan; + ScanKeyData skey; + HeapTuple tgtup; + HeapTuple inhtup; + + /* + * Note: since we scan the triggers using TriggerRelidNameIndexId, we will + * be reading the triggers in name order, except possibly during + * emergency-recovery operations (ie, IgnoreSystemIndexes). This in turn + * ensures that triggers will be fired in name order. However, when + * recursing to search for triggers in parent relations, the order is no + * longer guaranteed (since multiple scans are involved), so we signal our + * caller to sort afterwards. + */ + + /* + * Scan pg_trigger for the given relation, appending relevant triggers + * to our output array. + */ + ScanKeyInit(&skey, + Anum_pg_trigger_tgrelid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(tgrelid)); + scan = systable_beginscan(tgrel, TriggerRelidNameIndexId, true, + NULL, 1, &skey); + while (HeapTupleIsValid(tgtup = systable_getnext(scan))) + { + Form_pg_trigger trig = (Form_pg_trigger) GETSTRUCT(tgtup); + + /* + * triggers in ancestor rels are not considered unless they are marked + * tginherits. + */ + if (!all_triggers && !trig->tginherits) + continue; + + add_trigger_to_array(RelationGetDescr(tgrel), tgtup, + triggers, numtrigs, maxtrigs); + } + systable_endscan(scan); + + /* + * Now scan pg_inherits to find parents of this relation and recurse, in + * case any inheritable triggers are present there. + */ + ScanKeyInit(&skey, + Anum_pg_inherits_inhrelid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(tgrelid)); + scan = systable_beginscan(inhrel, InheritsRelidSeqnoIndexId, true, NULL, + 1, &skey); + while ((inhtup = systable_getnext(scan)) != NULL) + { + Oid ancestor = ((Form_pg_inherits) GETSTRUCT(inhtup))->inhparent; + int local_numtrigs = *numtrigs; + + /* recurse to this parent */ + add_triggers_to_array(tgrel, inhrel, ancestor, false, + triggers, numtrigs, maxtrigs, must_sort); + + /* if any triggers were added, set the sort flag */ + if (*numtrigs > local_numtrigs) + *must_sort = true; + } + + systable_endscan(scan); +} + +/* + * Subroutine for add_triggers_to_array to add a single trigger to the array. + */ +static void +add_trigger_to_array(TupleDesc tgdesc, HeapTuple htup, + Trigger **triggers, int *numtrigs, int *maxtrigs) +{ + Form_pg_trigger pg_trigger = (Form_pg_trigger) GETSTRUCT(htup); + Trigger *build; + Datum datum; + bool isnull; + + if (*numtrigs >= *maxtrigs) + { + *maxtrigs *= 2; + *triggers = (Trigger *) repalloc(*triggers, *maxtrigs * sizeof(Trigger)); + } + build = *triggers + *numtrigs; + + build->tgoid = HeapTupleGetOid(htup); + build->tgname = DatumGetCString(DirectFunctionCall1(nameout, + NameGetDatum(&pg_trigger->tgname))); + build->tgfoid = pg_trigger->tgfoid; + build->tgtype = pg_trigger->tgtype; + build->tgenabled = pg_trigger->tgenabled; + build->tgisinternal = pg_trigger->tgisinternal; + build->tginherits = pg_trigger->tginherits; + build->tgconstrrelid = pg_trigger->tgconstrrelid; + build->tgconstrindid = pg_trigger->tgconstrindid; + build->tgconstraint = pg_trigger->tgconstraint; + build->tgdeferrable = pg_trigger->tgdeferrable; + build->tginitdeferred = pg_trigger->tginitdeferred; + build->tgnargs = pg_trigger->tgnargs; + /* tgattr is first var-width field, so OK to access directly */ + build->tgnattr = pg_trigger->tgattr.dim1; + if (build->tgnattr > 0) + { + build->tgattr = (int16 *) palloc(build->tgnattr * sizeof(int16)); + memcpy(build->tgattr, &(pg_trigger->tgattr.values), + build->tgnattr * sizeof(int16)); + } + else + build->tgattr = NULL; + if (build->tgnargs > 0) + { + bytea *val; + char *p; + int i; + + val = DatumGetByteaPP(fastgetattr(htup, + Anum_pg_trigger_tgargs, + tgdesc, &isnull)); + if (isnull) + elog(ERROR, "tgargs is null in trigger \"%u\"", + HeapTupleGetOid(htup)); + p = (char *) VARDATA_ANY(val); + build->tgargs = (char **) palloc(build->tgnargs * sizeof(char *)); + for (i = 0; i < build->tgnargs; i++) + { + build->tgargs[i] = pstrdup(p); + p += strlen(p) + 1; + } + } + else + build->tgargs = NULL; + + datum = fastgetattr(htup, Anum_pg_trigger_tgoldtable, + tgdesc, &isnull); + if (!isnull) + build->tgoldtable = + DatumGetCString(DirectFunctionCall1(nameout, datum)); + else + build->tgoldtable = NULL; + + datum = fastgetattr(htup, Anum_pg_trigger_tgnewtable, + tgdesc, &isnull); + if (!isnull) + build->tgnewtable = + DatumGetCString(DirectFunctionCall1(nameout, datum)); + else + build->tgnewtable = NULL; + + datum = fastgetattr(htup, Anum_pg_trigger_tgqual, + tgdesc, &isnull); + if (!isnull) + build->tgqual = TextDatumGetCString(datum); + else + build->tgqual = NULL; + + (*numtrigs)++; +} + +static int +qsort_trigger_cmp(const void *a, const void *b) +{ + const Trigger *ta = (const Trigger *) a; + const Trigger *tb = (const Trigger *) b; + + return strcmp(ta->tgname, tb->tgname); +} +/* * SQL function pg_trigger_depth() */ Datum diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index 9ee78f885f..8e494bb70b 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -3161,7 +3161,8 @@ RelationBuildLocalRelation(const char *relname, bool shared_relation, bool mapped_relation, char relpersistence, - char relkind) + char relkind, + bool has_triggers) { Relation rel; MemoryContext oldcxt; @@ -3272,6 +3273,7 @@ RelationBuildLocalRelation(const char *relname, rel->rd_rel->relhasoids = rel->rd_att->tdhasoid; rel->rd_rel->relnatts = natts; rel->rd_rel->reltype = InvalidOid; + rel->rd_rel->relhastriggers = has_triggers; /* needed when bootstrapping: */ rel->rd_rel->relowner = BOOTSTRAP_SUPERUSERID; diff --git a/src/include/catalog/heap.h b/src/include/catalog/heap.h index 9bdc63ceb5..8961a557f7 100644 --- a/src/include/catalog/heap.h +++ b/src/include/catalog/heap.h @@ -47,6 +47,7 @@ extern Relation heap_create(const char *relname, TupleDesc tupDesc, char relkind, char relpersistence, + bool has_triggers, bool shared_relation, bool mapped_relation, bool allow_system_table_mods); @@ -66,6 +67,7 @@ extern Oid heap_create_with_catalog(const char *relname, bool mapped_relation, bool oidislocal, int oidinhcount, + bool hastriggers, OnCommitAction oncommit, Datum reloptions, bool use_user_acl, diff --git a/src/include/catalog/pg_trigger.h b/src/include/catalog/pg_trigger.h index c80a3aa54d..1f3d1a8ebe 100644 --- a/src/include/catalog/pg_trigger.h +++ b/src/include/catalog/pg_trigger.h @@ -48,6 +48,7 @@ CATALOG(pg_trigger,2620) Oid tgconstraint; /* associated pg_constraint entry, if any */ bool tgdeferrable; /* constraint trigger is deferrable */ bool tginitdeferred; /* constraint trigger is deferred initially */ + bool tginherits; /* trigger applies to children relations */ int16 tgnargs; /* # of extra arguments in tgargs */ /* @@ -75,7 +76,7 @@ typedef FormData_pg_trigger *Form_pg_trigger; * compiler constants for pg_trigger * ---------------- */ -#define Natts_pg_trigger 17 +#define Natts_pg_trigger 18 #define Anum_pg_trigger_tgrelid 1 #define Anum_pg_trigger_tgname 2 #define Anum_pg_trigger_tgfoid 3 @@ -87,12 +88,13 @@ typedef FormData_pg_trigger *Form_pg_trigger; #define Anum_pg_trigger_tgconstraint 9 #define Anum_pg_trigger_tgdeferrable 10 #define Anum_pg_trigger_tginitdeferred 11 -#define Anum_pg_trigger_tgnargs 12 -#define Anum_pg_trigger_tgattr 13 -#define Anum_pg_trigger_tgargs 14 -#define Anum_pg_trigger_tgqual 15 -#define Anum_pg_trigger_tgoldtable 16 -#define Anum_pg_trigger_tgnewtable 17 +#define Anum_pg_trigger_tginherits 12 +#define Anum_pg_trigger_tgnargs 13 +#define Anum_pg_trigger_tgattr 14 +#define Anum_pg_trigger_tgargs 15 +#define Anum_pg_trigger_tgqual 16 +#define Anum_pg_trigger_tgoldtable 17 +#define Anum_pg_trigger_tgnewtable 18 /* Bits within tgtype */ #define TRIGGER_TYPE_ROW (1 << 0) diff --git a/src/include/utils/relcache.h b/src/include/utils/relcache.h index 8a546aba28..4ee77bfb3e 100644 --- a/src/include/utils/relcache.h +++ b/src/include/utils/relcache.h @@ -103,7 +103,8 @@ extern Relation RelationBuildLocalRelation(const char *relname, bool shared_relation, bool mapped_relation, char relpersistence, - char relkind); + char relkind, + bool has_triggers); /* * Routine to manage assignment of new relfilenode to a relation diff --git a/src/include/utils/reltrigger.h b/src/include/utils/reltrigger.h index 9b4dc7f810..b1f0354263 100644 --- a/src/include/utils/reltrigger.h +++ b/src/include/utils/reltrigger.h @@ -29,6 +29,7 @@ typedef struct Trigger int16 tgtype; char tgenabled; bool tgisinternal; + bool tginherits; Oid tgconstrrelid; Oid tgconstrindid; Oid tgconstraint; diff --git a/src/test/regress/expected/triggers.out b/src/test/regress/expected/triggers.out index 99be9ac6e9..03c9410b19 100644 --- a/src/test/regress/expected/triggers.out +++ b/src/test/regress/expected/triggers.out @@ -1847,7 +1847,66 @@ drop function my_trigger_function(); drop view my_view; drop table my_table; -- --- Verify that per-statement triggers are fired for partitioned tables +-- Verify cases that are unsupported with partitioned tables +-- +create table parted_trig (a int) partition by list (a); +create function trigger_nothing() returns trigger + language plpgsql as $$ begin end; $$; +create trigger failed before insert or update or delete on parted_trig + for each row execute procedure trigger_nothing(); +ERROR: "parted_trig" is a partitioned table +DETAIL: Partitioned tables cannot have BEFORE / FOR EACH ROW triggers. +create trigger failed after update on parted_trig + for each row when (OLD.a <> NEW.a) execute procedure trigger_nothing(); +ERROR: "parted_trig" is a partitioned table +DETAIL: Triggers FOR EACH ROW on partitioned table cannot have WHEN clauses. +create trigger failed instead of update on parted_trig + for each row execute procedure trigger_nothing(); +ERROR: "parted_trig" is a table +DETAIL: Tables cannot have INSTEAD OF triggers. +create trigger failed after update on parted_trig + referencing old table as old_table + for each statement execute procedure trigger_nothing(); +create constraint trigger failed after insert on parted_trig + for each row execute procedure trigger_nothing(); +ERROR: "parted_trig" is a partitioned table +DETAIL: Partitioned tables cannot have CONSTRAINT triggers FOR EACH ROW. +drop table parted_trig; +-- +-- Verify trigger creation for partitioned tables, and drop behavior +-- +create table trigpart (a int, b int) partition by range (a); +create table trigpart1 partition of trigpart for values from (0) to (1000); +create trigger f after insert on trigpart for each row execute procedure trigger_nothing(); +create table trigpart2 partition of trigpart for values from (1000) to (2000); +create table trigpart3 (like trigpart); +alter table trigpart attach partition trigpart3 for values from (2000) to (3000); +select tgrelid::regclass, tgname, tgfoid::regproc from pg_trigger + where tgrelid::regclass::text like 'trigpart%' order by tgrelid::regclass::text; + tgrelid | tgname | tgfoid +----------+--------+----------------- + trigpart | f | trigger_nothing +(1 row) + +drop table trigpart2; +select tgrelid::regclass, tgname, tgfoid::regproc from pg_trigger + where tgrelid::regclass::text like 'trigpart%' order by tgrelid::regclass::text; + tgrelid | tgname | tgfoid +----------+--------+----------------- + trigpart | f | trigger_nothing +(1 row) + +drop trigger f on trigpart; -- ok, all gone +select tgrelid::regclass, tgname, tgfoid::regproc from pg_trigger + where tgrelid::regclass::text like 'trigpart%' order by tgrelid::regclass::text; + tgrelid | tgname | tgfoid +---------+--------+-------- +(0 rows) + +drop table trigpart; +drop function trigger_nothing(); +-- +-- Verify that triggers are fired for partitioned tables -- create table parted_stmt_trig (a int) partition by list (a); create table parted_stmt_trig1 partition of parted_stmt_trig for values in (1); @@ -1864,7 +1923,7 @@ create or replace function trigger_notice() returns trigger as $$ return null; end; $$ language plpgsql; --- insert/update/delete statment-level triggers on the parent +-- insert/update/delete statement-level triggers on the parent create trigger trig_ins_before before insert on parted_stmt_trig for each statement execute procedure trigger_notice(); create trigger trig_ins_after after insert on parted_stmt_trig @@ -1877,36 +1936,62 @@ create trigger trig_del_before before delete on parted_stmt_trig for each statement execute procedure trigger_notice(); create trigger trig_del_after after delete on parted_stmt_trig for each statement execute procedure trigger_notice(); +-- these cases are disallowed +create trigger trig_ins_before_1 before insert on parted_stmt_trig + for each row execute procedure trigger_notice(); +ERROR: "parted_stmt_trig" is a partitioned table +DETAIL: Partitioned tables cannot have BEFORE / FOR EACH ROW triggers. +create trigger trig_upd_before_1 before update on parted_stmt_trig + for each row execute procedure trigger_notice(); +ERROR: "parted_stmt_trig" is a partitioned table +DETAIL: Partitioned tables cannot have BEFORE / FOR EACH ROW triggers. +create trigger trig_del_before_1 before delete on parted_stmt_trig + for each row execute procedure trigger_notice(); +ERROR: "parted_stmt_trig" is a partitioned table +DETAIL: Partitioned tables cannot have BEFORE / FOR EACH ROW triggers. +-- insert/update/delete row-level triggers on the parent +create trigger trig_ins_after_parent after insert on parted_stmt_trig + for each row execute procedure trigger_notice(); +create trigger trig_upd_after_parent after update on parted_stmt_trig + for each row execute procedure trigger_notice(); +create trigger trig_del_after_parent after delete on parted_stmt_trig + for each row execute procedure trigger_notice(); -- insert/update/delete row-level triggers on the first partition -create trigger trig_ins_before before insert on parted_stmt_trig1 +create trigger trig_ins_before_child before insert on parted_stmt_trig1 for each row execute procedure trigger_notice(); -create trigger trig_ins_after after insert on parted_stmt_trig1 +create trigger trig_ins_after_child after insert on parted_stmt_trig1 for each row execute procedure trigger_notice(); -create trigger trig_upd_before before update on parted_stmt_trig1 +create trigger trig_upd_before_child before update on parted_stmt_trig1 for each row execute procedure trigger_notice(); -create trigger trig_upd_after after update on parted_stmt_trig1 +create trigger trig_upd_after_child after update on parted_stmt_trig1 + for each row execute procedure trigger_notice(); +create trigger trig_del_before_child before delete on parted_stmt_trig1 + for each row execute procedure trigger_notice(); +create trigger trig_del_after_child after delete on parted_stmt_trig1 for each row execute procedure trigger_notice(); -- insert/update/delete statement-level triggers on the parent -create trigger trig_ins_before before insert on parted2_stmt_trig +create trigger trig_ins_before_3 before insert on parted2_stmt_trig for each statement execute procedure trigger_notice(); -create trigger trig_ins_after after insert on parted2_stmt_trig +create trigger trig_ins_after_3 after insert on parted2_stmt_trig for each statement execute procedure trigger_notice(); -create trigger trig_upd_before before update on parted2_stmt_trig +create trigger trig_upd_before_3 before update on parted2_stmt_trig for each statement execute procedure trigger_notice(); -create trigger trig_upd_after after update on parted2_stmt_trig +create trigger trig_upd_after_3 after update on parted2_stmt_trig for each statement execute procedure trigger_notice(); -create trigger trig_del_before before delete on parted2_stmt_trig +create trigger trig_del_before_3 before delete on parted2_stmt_trig for each statement execute procedure trigger_notice(); -create trigger trig_del_after after delete on parted2_stmt_trig +create trigger trig_del_after_3 after delete on parted2_stmt_trig for each statement execute procedure trigger_notice(); with ins (a) as ( insert into parted2_stmt_trig values (1), (2) returning a ) insert into parted_stmt_trig select a from ins returning tableoid::regclass, a; NOTICE: trigger trig_ins_before on parted_stmt_trig BEFORE INSERT for STATEMENT -NOTICE: trigger trig_ins_before on parted2_stmt_trig BEFORE INSERT for STATEMENT -NOTICE: trigger trig_ins_before on parted_stmt_trig1 BEFORE INSERT for ROW -NOTICE: trigger trig_ins_after on parted_stmt_trig1 AFTER INSERT for ROW -NOTICE: trigger trig_ins_after on parted2_stmt_trig AFTER INSERT for STATEMENT +NOTICE: trigger trig_ins_before_3 on parted2_stmt_trig BEFORE INSERT for STATEMENT +NOTICE: trigger trig_ins_before_child on parted_stmt_trig1 BEFORE INSERT for ROW +NOTICE: trigger trig_ins_after_child on parted_stmt_trig1 AFTER INSERT for ROW +NOTICE: trigger trig_ins_after_parent on parted_stmt_trig1 AFTER INSERT for ROW +NOTICE: trigger trig_ins_after_parent on parted_stmt_trig2 AFTER INSERT for ROW +NOTICE: trigger trig_ins_after_3 on parted2_stmt_trig AFTER INSERT for STATEMENT NOTICE: trigger trig_ins_after on parted_stmt_trig AFTER INSERT for STATEMENT tableoid | a -------------------+--- @@ -1918,25 +2003,63 @@ with upd as ( update parted2_stmt_trig set a = a ) update parted_stmt_trig set a = a; NOTICE: trigger trig_upd_before on parted_stmt_trig BEFORE UPDATE for STATEMENT -NOTICE: trigger trig_upd_before on parted_stmt_trig1 BEFORE UPDATE for ROW -NOTICE: trigger trig_upd_before on parted2_stmt_trig BEFORE UPDATE for STATEMENT -NOTICE: trigger trig_upd_after on parted_stmt_trig1 AFTER UPDATE for ROW +NOTICE: trigger trig_upd_before_child on parted_stmt_trig1 BEFORE UPDATE for ROW +NOTICE: trigger trig_upd_before_3 on parted2_stmt_trig BEFORE UPDATE for STATEMENT +NOTICE: trigger trig_upd_after_child on parted_stmt_trig1 AFTER UPDATE for ROW +NOTICE: trigger trig_upd_after_parent on parted_stmt_trig1 AFTER UPDATE for ROW +NOTICE: trigger trig_upd_after_parent on parted_stmt_trig2 AFTER UPDATE for ROW NOTICE: trigger trig_upd_after on parted_stmt_trig AFTER UPDATE for STATEMENT -NOTICE: trigger trig_upd_after on parted2_stmt_trig AFTER UPDATE for STATEMENT +NOTICE: trigger trig_upd_after_3 on parted2_stmt_trig AFTER UPDATE for STATEMENT delete from parted_stmt_trig; NOTICE: trigger trig_del_before on parted_stmt_trig BEFORE DELETE for STATEMENT +NOTICE: trigger trig_del_before_child on parted_stmt_trig1 BEFORE DELETE for ROW +NOTICE: trigger trig_del_after_parent on parted_stmt_trig2 AFTER DELETE for ROW NOTICE: trigger trig_del_after on parted_stmt_trig AFTER DELETE for STATEMENT -- insert via copy on the parent copy parted_stmt_trig(a) from stdin; NOTICE: trigger trig_ins_before on parted_stmt_trig BEFORE INSERT for STATEMENT -NOTICE: trigger trig_ins_before on parted_stmt_trig1 BEFORE INSERT for ROW -NOTICE: trigger trig_ins_after on parted_stmt_trig1 AFTER INSERT for ROW +NOTICE: trigger trig_ins_before_child on parted_stmt_trig1 BEFORE INSERT for ROW +NOTICE: trigger trig_ins_after_child on parted_stmt_trig1 AFTER INSERT for ROW +NOTICE: trigger trig_ins_after_parent on parted_stmt_trig1 AFTER INSERT for ROW +NOTICE: trigger trig_ins_after_parent on parted_stmt_trig2 AFTER INSERT for ROW NOTICE: trigger trig_ins_after on parted_stmt_trig AFTER INSERT for STATEMENT -- insert via copy on the first partition copy parted_stmt_trig1(a) from stdin; -NOTICE: trigger trig_ins_before on parted_stmt_trig1 BEFORE INSERT for ROW -NOTICE: trigger trig_ins_after on parted_stmt_trig1 AFTER INSERT for ROW +NOTICE: trigger trig_ins_before_child on parted_stmt_trig1 BEFORE INSERT for ROW +NOTICE: trigger trig_ins_after_child on parted_stmt_trig1 AFTER INSERT for ROW +NOTICE: trigger trig_ins_after_parent on parted_stmt_trig1 AFTER INSERT for ROW +-- Disabling a trigger in the parent table should disable children triggers too +alter table parted_stmt_trig disable trigger trig_ins_after_parent; +insert into parted_stmt_trig values (1); +NOTICE: trigger trig_ins_before on parted_stmt_trig BEFORE INSERT for STATEMENT +NOTICE: trigger trig_ins_before_child on parted_stmt_trig1 BEFORE INSERT for ROW +NOTICE: trigger trig_ins_after_child on parted_stmt_trig1 AFTER INSERT for ROW +NOTICE: trigger trig_ins_after on parted_stmt_trig AFTER INSERT for STATEMENT +alter table parted_stmt_trig enable trigger trig_ins_after_parent; +insert into parted_stmt_trig values (1); +NOTICE: trigger trig_ins_before on parted_stmt_trig BEFORE INSERT for STATEMENT +NOTICE: trigger trig_ins_before_child on parted_stmt_trig1 BEFORE INSERT for ROW +NOTICE: trigger trig_ins_after_child on parted_stmt_trig1 AFTER INSERT for ROW +NOTICE: trigger trig_ins_after_parent on parted_stmt_trig1 AFTER INSERT for ROW +NOTICE: trigger trig_ins_after on parted_stmt_trig AFTER INSERT for STATEMENT drop table parted_stmt_trig, parted2_stmt_trig; +-- Verify that triggers fire in alphabetical order +create table parted_trig (a int) partition by range (a); +create table parted_trig_1 partition of parted_trig for values from (0) to (1000) + partition by range (a); +create table parted_trig_1_1 partition of parted_trig_1 for values from (0) to (100); +create trigger zzz after insert on parted_trig for each row execute procedure trigger_notice(); +create trigger mmm after insert on parted_trig_1_1 for each row execute procedure trigger_notice(); +create trigger aaa after insert on parted_trig_1 for each row execute procedure trigger_notice(); +create trigger bbb after insert on parted_trig for each row execute procedure trigger_notice(); +create trigger qqq after insert on parted_trig_1_1 for each row execute procedure trigger_notice(); +insert into parted_trig values (50); +NOTICE: trigger aaa on parted_trig_1_1 AFTER INSERT for ROW +NOTICE: trigger bbb on parted_trig_1_1 AFTER INSERT for ROW +NOTICE: trigger mmm on parted_trig_1_1 AFTER INSERT for ROW +NOTICE: trigger qqq on parted_trig_1_1 AFTER INSERT for ROW +NOTICE: trigger zzz on parted_trig_1_1 AFTER INSERT for ROW +drop table parted_trig; -- -- Test the interaction between transition tables and both kinds of -- inheritance. We'll dump the contents of the transition tables in a diff --git a/src/test/regress/sql/triggers.sql b/src/test/regress/sql/triggers.sql index 3354f4899f..e32055e94e 100644 --- a/src/test/regress/sql/triggers.sql +++ b/src/test/regress/sql/triggers.sql @@ -1286,7 +1286,47 @@ drop view my_view; drop table my_table; -- --- Verify that per-statement triggers are fired for partitioned tables +-- Verify cases that are unsupported with partitioned tables +-- +create table parted_trig (a int) partition by list (a); +create function trigger_nothing() returns trigger + language plpgsql as $$ begin end; $$; +create trigger failed before insert or update or delete on parted_trig + for each row execute procedure trigger_nothing(); +create trigger failed after update on parted_trig + for each row when (OLD.a <> NEW.a) execute procedure trigger_nothing(); +create trigger failed instead of update on parted_trig + for each row execute procedure trigger_nothing(); +create trigger failed after update on parted_trig + referencing old table as old_table + for each statement execute procedure trigger_nothing(); +create constraint trigger failed after insert on parted_trig + for each row execute procedure trigger_nothing(); +drop table parted_trig; + +-- +-- Verify trigger creation for partitioned tables, and drop behavior +-- +create table trigpart (a int, b int) partition by range (a); +create table trigpart1 partition of trigpart for values from (0) to (1000); +create trigger f after insert on trigpart for each row execute procedure trigger_nothing(); +create table trigpart2 partition of trigpart for values from (1000) to (2000); +create table trigpart3 (like trigpart); +alter table trigpart attach partition trigpart3 for values from (2000) to (3000); +select tgrelid::regclass, tgname, tgfoid::regproc from pg_trigger + where tgrelid::regclass::text like 'trigpart%' order by tgrelid::regclass::text; +drop table trigpart2; +select tgrelid::regclass, tgname, tgfoid::regproc from pg_trigger + where tgrelid::regclass::text like 'trigpart%' order by tgrelid::regclass::text; +drop trigger f on trigpart; -- ok, all gone +select tgrelid::regclass, tgname, tgfoid::regproc from pg_trigger + where tgrelid::regclass::text like 'trigpart%' order by tgrelid::regclass::text; + +drop table trigpart; +drop function trigger_nothing(); + +-- +-- Verify that triggers are fired for partitioned tables -- create table parted_stmt_trig (a int) partition by list (a); create table parted_stmt_trig1 partition of parted_stmt_trig for values in (1); @@ -1306,7 +1346,7 @@ create or replace function trigger_notice() returns trigger as $$ end; $$ language plpgsql; --- insert/update/delete statment-level triggers on the parent +-- insert/update/delete statement-level triggers on the parent create trigger trig_ins_before before insert on parted_stmt_trig for each statement execute procedure trigger_notice(); create trigger trig_ins_after after insert on parted_stmt_trig @@ -1320,28 +1360,48 @@ create trigger trig_del_before before delete on parted_stmt_trig create trigger trig_del_after after delete on parted_stmt_trig for each statement execute procedure trigger_notice(); +-- these cases are disallowed +create trigger trig_ins_before_1 before insert on parted_stmt_trig + for each row execute procedure trigger_notice(); +create trigger trig_upd_before_1 before update on parted_stmt_trig + for each row execute procedure trigger_notice(); +create trigger trig_del_before_1 before delete on parted_stmt_trig + for each row execute procedure trigger_notice(); + +-- insert/update/delete row-level triggers on the parent +create trigger trig_ins_after_parent after insert on parted_stmt_trig + for each row execute procedure trigger_notice(); +create trigger trig_upd_after_parent after update on parted_stmt_trig + for each row execute procedure trigger_notice(); +create trigger trig_del_after_parent after delete on parted_stmt_trig + for each row execute procedure trigger_notice(); + -- insert/update/delete row-level triggers on the first partition -create trigger trig_ins_before before insert on parted_stmt_trig1 +create trigger trig_ins_before_child before insert on parted_stmt_trig1 for each row execute procedure trigger_notice(); -create trigger trig_ins_after after insert on parted_stmt_trig1 +create trigger trig_ins_after_child after insert on parted_stmt_trig1 for each row execute procedure trigger_notice(); -create trigger trig_upd_before before update on parted_stmt_trig1 +create trigger trig_upd_before_child before update on parted_stmt_trig1 for each row execute procedure trigger_notice(); -create trigger trig_upd_after after update on parted_stmt_trig1 +create trigger trig_upd_after_child after update on parted_stmt_trig1 + for each row execute procedure trigger_notice(); +create trigger trig_del_before_child before delete on parted_stmt_trig1 + for each row execute procedure trigger_notice(); +create trigger trig_del_after_child after delete on parted_stmt_trig1 for each row execute procedure trigger_notice(); -- insert/update/delete statement-level triggers on the parent -create trigger trig_ins_before before insert on parted2_stmt_trig +create trigger trig_ins_before_3 before insert on parted2_stmt_trig for each statement execute procedure trigger_notice(); -create trigger trig_ins_after after insert on parted2_stmt_trig +create trigger trig_ins_after_3 after insert on parted2_stmt_trig for each statement execute procedure trigger_notice(); -create trigger trig_upd_before before update on parted2_stmt_trig +create trigger trig_upd_before_3 before update on parted2_stmt_trig for each statement execute procedure trigger_notice(); -create trigger trig_upd_after after update on parted2_stmt_trig +create trigger trig_upd_after_3 after update on parted2_stmt_trig for each statement execute procedure trigger_notice(); -create trigger trig_del_before before delete on parted2_stmt_trig +create trigger trig_del_before_3 before delete on parted2_stmt_trig for each statement execute procedure trigger_notice(); -create trigger trig_del_after after delete on parted2_stmt_trig +create trigger trig_del_after_3 after delete on parted2_stmt_trig for each statement execute procedure trigger_notice(); with ins (a) as ( @@ -1365,8 +1425,27 @@ copy parted_stmt_trig1(a) from stdin; 1 \. +-- Disabling a trigger in the parent table should disable children triggers too +alter table parted_stmt_trig disable trigger trig_ins_after_parent; +insert into parted_stmt_trig values (1); +alter table parted_stmt_trig enable trigger trig_ins_after_parent; +insert into parted_stmt_trig values (1); + drop table parted_stmt_trig, parted2_stmt_trig; +-- Verify that triggers fire in alphabetical order +create table parted_trig (a int) partition by range (a); +create table parted_trig_1 partition of parted_trig for values from (0) to (1000) + partition by range (a); +create table parted_trig_1_1 partition of parted_trig_1 for values from (0) to (100); +create trigger zzz after insert on parted_trig for each row execute procedure trigger_notice(); +create trigger mmm after insert on parted_trig_1_1 for each row execute procedure trigger_notice(); +create trigger aaa after insert on parted_trig_1 for each row execute procedure trigger_notice(); +create trigger bbb after insert on parted_trig for each row execute procedure trigger_notice(); +create trigger qqq after insert on parted_trig_1_1 for each row execute procedure trigger_notice(); +insert into parted_trig values (50); +drop table parted_trig; + -- -- Test the interaction between transition tables and both kinds of -- inheritance. We'll dump the contents of the transition tables in a