database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-06-30 21:42:05 +03:00
Code Activity

Fix ENABLE/DISABLE TRIGGER to handle recursion correctly

Browse Source 
Using ATSimpleRecursion() in ATPrepCmd() to do so as bbb927b4db did is
not correct, because ATPrepCmd() can't distinguish between triggers that
may be cloned and those that may not, so would wrongly try to recurse
for the latter category of triggers.

So this commit restores the code in EnableDisableTrigger() that
86f575948c had added to do the recursion, which would do it only for
triggers that may be cloned, that is, row-level triggers.  This also
changes tablecmds.c such that ATExecCmd() is able to pass the value of
ONLY flag down to EnableDisableTrigger() using its new 'recurse'
parameter.

This also fixes what seems like an oversight of 86f575948c that the
recursion to partition triggers would only occur if EnableDisableTrigger()
had actually changed the trigger.  It is more apt to recurse to inspect
partition triggers even if the parent's trigger didn't need to be
changed: only then can we be certain that all descendants share the same
state afterwards.

Backpatch all the way back to 11, like bbb927b4db.  Care is taken not
to break ABI compatibility (and that no catversion bump is needed.)

Co-authored-by: Amit Langote <amitlangote09@gmail.com>
Reviewed-by: Dmitry Koval <d.koval@postgrespro.ru>
Discussion: https://postgr.es/m/CA+HiwqG-cZT3XzGAnEgZQLoQbyfJApVwOTQaCaas1mhpf+4V5A@mail.gmail.com
This commit is contained in:
Alvaro Herrera
2022-08-04 20:02:02 +02:00
parent efba7a63ff
commit 731d514ae5
8 changed files with 134 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files

47
src/backend/commands/trigger.c
View File

@ -1543,14 +1543,16 @@ renametrig(RenameStmt *stmt)
* enablement/disablement, this also defines when the trigger
* should be fired in session replication roles.
* skip_system: if true, skip "system" triggers (constraint triggers)
* recurse: if true, recurse to partitions
*
* Caller should have checked permissions for the table; here we also
* enforce that superuser privilege is required to alter the state of
* system triggers
*/
void
EnableDisableTrigger(Relation rel, const char *tgname,
char fires_when, bool skip_system, LOCKMODE lockmode)
EnableDisableTriggerNew(Relation rel, const char *tgname,
char fires_when, bool skip_system, bool recurse,
LOCKMODE lockmode)
{
Relation tgrel;
int nkeys;
@ -1616,6 +1618,34 @@ EnableDisableTrigger(Relation rel, const char *tgname,
changed = true;
}
/*
* When altering FOR EACH ROW triggers on a partitioned table, do the
* same on the partitions as well, unless ONLY is specified.
*
* Note that we recurse even if we didn't change the trigger above,
* because the partitions' copy of the trigger may have a different
* value of tgenabled than the parent's trigger and thus might need to
* be changed.
*/
if (recurse &&
rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE &&
(TRIGGER_FOR_ROW(oldtrig->tgtype)))
{
PartitionDesc partdesc = RelationGetPartitionDesc(rel, true);
int i;
for (i = 0; i < partdesc->nparts; i++)
{
Relation part;
part = relation_open(partdesc->oids[i], lockmode);
EnableDisableTriggerNew(part, NameStr(oldtrig->tgname),
fires_when, skip_system, recurse,
lockmode);
table_close(part, NoLock); /* keep lock till commit */
}
}
InvokeObjectPostAlterHook(TriggerRelationId,
oldtrig->oid, 0);
}
@ -1639,6 +1669,19 @@ EnableDisableTrigger(Relation rel, const char *tgname,
CacheInvalidateRelcache(rel);
}
/*
* ABI-compatible wrapper for the above. To keep as close possible to the old
* behavior, this never recurses. Do not call this function in new code.
*/
void
EnableDisableTrigger(Relation rel, const char *tgname,
char fires_when, bool skip_system,
LOCKMODE lockmode)
{
EnableDisableTriggerNew(rel, tgname, fires_when, skip_system,
true, lockmode);
}
/*
* Build trigger data to attach to the given relcache entry.