aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--src/backend/commands/tablecmds.c64
-rw-r--r--src/backend/commands/trigger.c47
-rw-r--r--src/backend/nodes/copyfuncs.c1
-rw-r--r--src/backend/nodes/equalfuncs.c1
-rw-r--r--src/include/commands/trigger.h3
-rw-r--r--src/include/nodes/parsenodes.h1
-rw-r--r--src/test/regress/expected/triggers.out40
-rw-r--r--src/test/regress/sql/triggers.sql11
8 files changed, 134 insertions, 34 deletions
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 59569848e60..217ac622c11 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -503,7 +503,8 @@ static void ATExecSetRelOptions(Relation rel, List *defList,
AlterTableType operation,
LOCKMODE lockmode);
static void ATExecEnableDisableTrigger(Relation rel, const char *trigname,
- char fires_when, bool skip_system, LOCKMODE lockmode);
+ char fires_when, bool skip_system, bool recurse,
+ LOCKMODE lockmode);
static void ATExecEnableDisableRule(Relation rel, const char *rulename,
char fires_when, LOCKMODE lockmode);
static void ATPrepAddInherit(Relation child_rel);
@@ -3693,9 +3694,7 @@ AlterTableLookupRelation(AlterTableStmt *stmt, LOCKMODE lockmode)
* expressions that need to be evaluated with respect to the old table
* schema.
*
- * ATRewriteCatalogs performs phase 2 for each affected table. (Note that
- * phases 2 and 3 normally do no explicit recursion, since phase 1 already
- * did it --- although some subcommands have to recurse in phase 2 instead.)
+ * ATRewriteCatalogs performs phase 2 for each affected table.
* Certain subcommands need to be performed before others to avoid
* unnecessary conflicts; for example, DROP COLUMN should come before
* ADD COLUMN. Therefore phase 1 divides the subcommands into multiple
@@ -3703,6 +3702,12 @@ AlterTableLookupRelation(AlterTableStmt *stmt, LOCKMODE lockmode)
*
* ATRewriteTables performs phase 3 for those tables that need it.
*
+ * For most subcommand types, phases 2 and 3 do no explicit recursion,
+ * since phase 1 already does it. However, for certain subcommand types
+ * it is only possible to determine how to recurse at phase 2 time; for
+ * those cases, phase 1 sets the cmd->recurse flag (or, in some older coding,
+ * changes the command subtype of a "Recurse" variant XXX to be cleaned up.)
+ *
* Thanks to the magic of MVCC, an error anywhere along the way rolls back
* the whole operation; we don't have to do anything special to clean up.
*
@@ -4090,10 +4095,10 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
tab = ATGetQueueEntry(wqueue, rel);
/*
- * Copy the original subcommand for each table. This avoids conflicts
- * when different child tables need to make different parse
- * transformations (for example, the same column may have different column
- * numbers in different children).
+ * Copy the original subcommand for each table, so we can scribble on it.
+ * This avoids conflicts when different child tables need to make
+ * different parse transformations (for example, the same column may have
+ * different column numbers in different children).
*/
cmd = copyObject(cmd);
@@ -4331,8 +4336,9 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
case AT_DisableTrigAll:
case AT_DisableTrigUser:
ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE);
- if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
- ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode);
+ /* Set up recursion for phase 2; no other prep needed */
+ if (recurse)
+ cmd->recurse = true;
pass = AT_PASS_MISC;
break;
case AT_EnableRule: /* ENABLE/DISABLE RULE variants */
@@ -4623,35 +4629,51 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
break;
case AT_EnableTrig: /* ENABLE TRIGGER name */
ATExecEnableDisableTrigger(rel, cmd->name,
- TRIGGER_FIRES_ON_ORIGIN, false, lockmode);
+ TRIGGER_FIRES_ON_ORIGIN, false,
+ cmd->recurse,
+ lockmode);
break;
case AT_EnableAlwaysTrig: /* ENABLE ALWAYS TRIGGER name */
ATExecEnableDisableTrigger(rel, cmd->name,
- TRIGGER_FIRES_ALWAYS, false, lockmode);
+ TRIGGER_FIRES_ALWAYS, false,
+ cmd->recurse,
+ lockmode);
break;
case AT_EnableReplicaTrig: /* ENABLE REPLICA TRIGGER name */
ATExecEnableDisableTrigger(rel, cmd->name,
- TRIGGER_FIRES_ON_REPLICA, false, lockmode);
+ TRIGGER_FIRES_ON_REPLICA, false,
+ cmd->recurse,
+ lockmode);
break;
case AT_DisableTrig: /* DISABLE TRIGGER name */
ATExecEnableDisableTrigger(rel, cmd->name,
- TRIGGER_DISABLED, false, lockmode);
+ TRIGGER_DISABLED, false,
+ cmd->recurse,
+ lockmode);
break;
case AT_EnableTrigAll: /* ENABLE TRIGGER ALL */
ATExecEnableDisableTrigger(rel, NULL,
- TRIGGER_FIRES_ON_ORIGIN, false, lockmode);
+ TRIGGER_FIRES_ON_ORIGIN, false,
+ cmd->recurse,
+ lockmode);
break;
case AT_DisableTrigAll: /* DISABLE TRIGGER ALL */
ATExecEnableDisableTrigger(rel, NULL,
- TRIGGER_DISABLED, false, lockmode);
+ TRIGGER_DISABLED, false,
+ cmd->recurse,
+ lockmode);
break;
case AT_EnableTrigUser: /* ENABLE TRIGGER USER */
ATExecEnableDisableTrigger(rel, NULL,
- TRIGGER_FIRES_ON_ORIGIN, true, lockmode);
+ TRIGGER_FIRES_ON_ORIGIN, true,
+ cmd->recurse,
+ lockmode);
break;
case AT_DisableTrigUser: /* DISABLE TRIGGER USER */
ATExecEnableDisableTrigger(rel, NULL,
- TRIGGER_DISABLED, true, lockmode);
+ TRIGGER_DISABLED, true,
+ cmd->recurse,
+ lockmode);
break;
case AT_EnableRule: /* ENABLE RULE name */
@@ -13321,9 +13343,11 @@ index_copy_data(Relation rel, RelFileNode newrnode)
*/
static void
ATExecEnableDisableTrigger(Relation rel, const char *trigname,
- char fires_when, bool skip_system, LOCKMODE lockmode)
+ char fires_when, bool skip_system, bool recurse,
+ LOCKMODE lockmode)
{
- EnableDisableTrigger(rel, trigname, fires_when, skip_system, lockmode);
+ EnableDisableTriggerNew(rel, trigname, fires_when, skip_system, recurse,
+ lockmode);
}
/*
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index eb1a13d3bc0..51a0af8d1c6 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -1806,14 +1806,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;
@@ -1879,6 +1881,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);
+ 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);
}
@@ -1902,6 +1932,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.
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 3703845b010..d3e42574b70 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -3160,6 +3160,7 @@ _copyAlterTableCmd(const AlterTableCmd *from)
COPY_NODE_FIELD(def);
COPY_SCALAR_FIELD(behavior);
COPY_SCALAR_FIELD(missing_ok);
+ COPY_SCALAR_FIELD(recurse);
return newnode;
}
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 88e17a3a604..d34b73b842b 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -1095,6 +1095,7 @@ _equalAlterTableCmd(const AlterTableCmd *a, const AlterTableCmd *b)
COMPARE_NODE_FIELD(def);
COMPARE_SCALAR_FIELD(behavior);
COMPARE_SCALAR_FIELD(missing_ok);
+ COMPARE_SCALAR_FIELD(recurse);
return true;
}
diff --git a/src/include/commands/trigger.h b/src/include/commands/trigger.h
index 5dbd74799d8..33002ef472d 100644
--- a/src/include/commands/trigger.h
+++ b/src/include/commands/trigger.h
@@ -172,6 +172,9 @@ extern Oid get_trigger_oid(Oid relid, const char *name, bool missing_ok);
extern ObjectAddress renametrig(RenameStmt *stmt);
+extern void EnableDisableTriggerNew(Relation rel, const char *tgname,
+ char fires_when, bool skip_system, bool recurse,
+ LOCKMODE lockmode);
extern void EnableDisableTrigger(Relation rel, const char *tgname,
char fires_when, bool skip_system, LOCKMODE lockmode);
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index f93aff96ac7..c9209ae16f6 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -1853,6 +1853,7 @@ typedef struct AlterTableCmd /* one subcommand of an ALTER TABLE */
* constraint, or parent table */
DropBehavior behavior; /* RESTRICT or CASCADE for DROP cases */
bool missing_ok; /* skip error if missing? */
+ bool recurse; /* exec-time recursion */
} AlterTableCmd;
diff --git a/src/test/regress/expected/triggers.out b/src/test/regress/expected/triggers.out
index 4c02407f6b9..49c5ccfbd6e 100644
--- a/src/test/regress/expected/triggers.out
+++ b/src/test/regress/expected/triggers.out
@@ -2532,24 +2532,42 @@ create table parent (a int) partition by list (a);
create table child1 partition of parent for values in (1);
create trigger tg after insert on parent
for each row execute procedure trig_nothing();
+create trigger tg_stmt after insert on parent
+ for statement execute procedure trig_nothing();
select tgrelid::regclass, tgname, tgenabled from pg_trigger
where tgrelid in ('parent'::regclass, 'child1'::regclass)
order by tgrelid::regclass::text;
- tgrelid | tgname | tgenabled
----------+--------+-----------
- child1 | tg | O
- parent | tg | O
-(2 rows)
+ tgrelid | tgname | tgenabled
+---------+---------+-----------
+ child1 | tg | O
+ parent | tg | O
+ parent | tg_stmt | O
+(3 rows)
-alter table only parent enable always trigger tg;
+alter table only parent enable always trigger tg; -- no recursion because ONLY
+alter table parent enable always trigger tg_stmt; -- no recursion because statement trigger
select tgrelid::regclass, tgname, tgenabled from pg_trigger
where tgrelid in ('parent'::regclass, 'child1'::regclass)
order by tgrelid::regclass::text;
- tgrelid | tgname | tgenabled
----------+--------+-----------
- child1 | tg | O
- parent | tg | A
-(2 rows)
+ tgrelid | tgname | tgenabled
+---------+---------+-----------
+ child1 | tg | O
+ parent | tg | A
+ parent | tg_stmt | A
+(3 rows)
+
+-- The following is a no-op for the parent trigger but not so
+-- for the child trigger, so recursion should be applied.
+alter table parent enable always trigger tg;
+select tgrelid::regclass, tgname, tgenabled from pg_trigger
+ where tgrelid in ('parent'::regclass, 'child1'::regclass)
+ order by tgrelid::regclass::text;
+ tgrelid | tgname | tgenabled
+---------+---------+-----------
+ child1 | tg | A
+ parent | tg | A
+ parent | tg_stmt | A
+(3 rows)
drop table parent, child1;
-- Verify that firing state propagates correctly on creation, too
diff --git a/src/test/regress/sql/triggers.sql b/src/test/regress/sql/triggers.sql
index 9cb19c53dfe..fa0a2455524 100644
--- a/src/test/regress/sql/triggers.sql
+++ b/src/test/regress/sql/triggers.sql
@@ -1750,10 +1750,19 @@ create table parent (a int) partition by list (a);
create table child1 partition of parent for values in (1);
create trigger tg after insert on parent
for each row execute procedure trig_nothing();
+create trigger tg_stmt after insert on parent
+ for statement execute procedure trig_nothing();
select tgrelid::regclass, tgname, tgenabled from pg_trigger
where tgrelid in ('parent'::regclass, 'child1'::regclass)
order by tgrelid::regclass::text;
-alter table only parent enable always trigger tg;
+alter table only parent enable always trigger tg; -- no recursion because ONLY
+alter table parent enable always trigger tg_stmt; -- no recursion because statement trigger
+select tgrelid::regclass, tgname, tgenabled from pg_trigger
+ where tgrelid in ('parent'::regclass, 'child1'::regclass)
+ order by tgrelid::regclass::text;
+-- The following is a no-op for the parent trigger but not so
+-- for the child trigger, so recursion should be applied.
+alter table parent enable always trigger tg;
select tgrelid::regclass, tgname, tgenabled from pg_trigger
where tgrelid in ('parent'::regclass, 'child1'::regclass)
order by tgrelid::regclass::text;