Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commite78fd90

Browse files
alvherreamitlan
andcommitted
Fix ENABLE/DISABLE TRIGGER to handle recursion correctly
Using ATSimpleRecursion() in ATPrepCmd() to do so asbbb927b did isnot correct, because ATPrepCmd() can't distinguish between triggers thatmay be cloned and those that may not, so would wrongly try to recursefor the latter category of triggers.So this commit restores the code in EnableDisableTrigger() that86f5759 had added to do the recursion, which would do it only fortriggers that may be cloned, that is, row-level triggers. This alsochanges tablecmds.c such that ATExecCmd() is able to pass the value ofONLY flag down to EnableDisableTrigger() using its new 'recurse'parameter.This also fixes what seems like an oversight of86f5759 that therecursion to partition triggers would only occur if EnableDisableTrigger()had actually changed the trigger. It is more apt to recurse to inspectpartition triggers even if the parent's trigger didn't need to bechanged: only then can we be certain that all descendants share the samestate afterwards.Backpatch all the way back to 11, likebbb927b. Care is taken notto 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
1 parent270eb4b commite78fd90

File tree

8 files changed

+134
-34
lines changed

8 files changed

+134
-34
lines changed

‎src/backend/commands/tablecmds.c

Lines changed: 44 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -580,7 +580,8 @@ static void ATExecSetRelOptions(Relation rel, List *defList,
580580
AlterTableType operation,
581581
LOCKMODE lockmode);
582582
static void ATExecEnableDisableTrigger(Relation rel, const char *trigname,
583-
char fires_when, bool skip_system, LOCKMODE lockmode);
583+
char fires_when, bool skip_system, bool recurse,
584+
LOCKMODE lockmode);
584585
static void ATExecEnableDisableRule(Relation rel, const char *rulename,
585586
char fires_when, LOCKMODE lockmode);
586587
static void ATPrepAddInherit(Relation child_rel);
@@ -4053,16 +4054,20 @@ AlterTableLookupRelation(AlterTableStmt *stmt, LOCKMODE lockmode)
40534054
* be done in this phase. Generally, this phase acquires table locks,
40544055
* checks permissions and relkind, and recurses to find child tables.
40554056
*
4056-
* ATRewriteCatalogs performs phase 2 for each affected table. (Note that
4057-
* phases 2 and 3 normally do no explicit recursion, since phase 1 already
4058-
* did it --- although some subcommands have to recurse in phase 2 instead.)
4057+
* ATRewriteCatalogs performs phase 2 for each affected table.
40594058
* Certain subcommands need to be performed before others to avoid
40604059
* unnecessary conflicts; for example, DROP COLUMN should come before
40614060
* ADD COLUMN. Therefore phase 1 divides the subcommands into multiple
40624061
* lists, one for each logical "pass" of phase 2.
40634062
*
40644063
* ATRewriteTables performs phase 3 for those tables that need it.
40654064
*
4065+
* For most subcommand types, phases 2 and 3 do no explicit recursion,
4066+
* since phase 1 already does it. However, for certain subcommand types
4067+
* it is only possible to determine how to recurse at phase 2 time; for
4068+
* those cases, phase 1 sets the cmd->recurse flag (or, in some older coding,
4069+
* changes the command subtype of a "Recurse" variant XXX to be cleaned up.)
4070+
*
40664071
* Thanks to the magic of MVCC, an error anywhere along the way rolls back
40674072
* the whole operation; we don't have to do anything special to clean up.
40684073
*
@@ -4483,10 +4488,10 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
44834488
errhint("Use ALTER TABLE ... DETACH PARTITION ... FINALIZE to complete the pending detach operation."));
44844489

44854490
/*
4486-
* Copy the original subcommand for each table. This avoids conflicts
4487-
* when different child tables need to make different parse
4488-
* transformations (for example, the same column may have different column
4489-
* numbers in different children).
4491+
* Copy the original subcommand for each table, so we can scribble on it.
4492+
*This avoids conflictswhen different child tables need to make
4493+
*different parsetransformations (for example, the same column may have
4494+
*different columnnumbers in different children).
44904495
*/
44914496
cmd = copyObject(cmd);
44924497

@@ -4773,8 +4778,9 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
47734778
case AT_DisableTrigAll:
47744779
case AT_DisableTrigUser:
47754780
ATSimplePermissions(cmd->subtype, rel, ATT_TABLE | ATT_FOREIGN_TABLE);
4776-
if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
4777-
ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode, context);
4781+
/* Set up recursion for phase 2; no other prep needed */
4782+
if (recurse)
4783+
cmd->recurse = true;
47784784
pass = AT_PASS_MISC;
47794785
break;
47804786
case AT_EnableRule:/* ENABLE/DISABLE RULE variants */
@@ -5115,35 +5121,51 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab,
51155121
break;
51165122
case AT_EnableTrig:/* ENABLE TRIGGER name */
51175123
ATExecEnableDisableTrigger(rel, cmd->name,
5118-
TRIGGER_FIRES_ON_ORIGIN, false, lockmode);
5124+
TRIGGER_FIRES_ON_ORIGIN, false,
5125+
cmd->recurse,
5126+
lockmode);
51195127
break;
51205128
case AT_EnableAlwaysTrig:/* ENABLE ALWAYS TRIGGER name */
51215129
ATExecEnableDisableTrigger(rel, cmd->name,
5122-
TRIGGER_FIRES_ALWAYS, false, lockmode);
5130+
TRIGGER_FIRES_ALWAYS, false,
5131+
cmd->recurse,
5132+
lockmode);
51235133
break;
51245134
case AT_EnableReplicaTrig:/* ENABLE REPLICA TRIGGER name */
51255135
ATExecEnableDisableTrigger(rel, cmd->name,
5126-
TRIGGER_FIRES_ON_REPLICA, false, lockmode);
5136+
TRIGGER_FIRES_ON_REPLICA, false,
5137+
cmd->recurse,
5138+
lockmode);
51275139
break;
51285140
case AT_DisableTrig:/* DISABLE TRIGGER name */
51295141
ATExecEnableDisableTrigger(rel, cmd->name,
5130-
TRIGGER_DISABLED, false, lockmode);
5142+
TRIGGER_DISABLED, false,
5143+
cmd->recurse,
5144+
lockmode);
51315145
break;
51325146
case AT_EnableTrigAll:/* ENABLE TRIGGER ALL */
51335147
ATExecEnableDisableTrigger(rel, NULL,
5134-
TRIGGER_FIRES_ON_ORIGIN, false, lockmode);
5148+
TRIGGER_FIRES_ON_ORIGIN, false,
5149+
cmd->recurse,
5150+
lockmode);
51355151
break;
51365152
case AT_DisableTrigAll: /* DISABLE TRIGGER ALL */
51375153
ATExecEnableDisableTrigger(rel, NULL,
5138-
TRIGGER_DISABLED, false, lockmode);
5154+
TRIGGER_DISABLED, false,
5155+
cmd->recurse,
5156+
lockmode);
51395157
break;
51405158
case AT_EnableTrigUser: /* ENABLE TRIGGER USER */
51415159
ATExecEnableDisableTrigger(rel, NULL,
5142-
TRIGGER_FIRES_ON_ORIGIN, true, lockmode);
5160+
TRIGGER_FIRES_ON_ORIGIN, true,
5161+
cmd->recurse,
5162+
lockmode);
51435163
break;
51445164
case AT_DisableTrigUser:/* DISABLE TRIGGER USER */
51455165
ATExecEnableDisableTrigger(rel, NULL,
5146-
TRIGGER_DISABLED, true, lockmode);
5166+
TRIGGER_DISABLED, true,
5167+
cmd->recurse,
5168+
lockmode);
51475169
break;
51485170

51495171
case AT_EnableRule:/* ENABLE RULE name */
@@ -14699,9 +14721,11 @@ index_copy_data(Relation rel, RelFileNode newrnode)
1469914721
*/
1470014722
static void
1470114723
ATExecEnableDisableTrigger(Relation rel, const char *trigname,
14702-
char fires_when, bool skip_system, LOCKMODE lockmode)
14724+
char fires_when, bool skip_system, bool recurse,
14725+
LOCKMODE lockmode)
1470314726
{
14704-
EnableDisableTrigger(rel, trigname, fires_when, skip_system, lockmode);
14727+
EnableDisableTriggerNew(rel, trigname, fires_when, skip_system, recurse,
14728+
lockmode);
1470514729
}
1470614730

1470714731
/*

‎src/backend/commands/trigger.c

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1759,14 +1759,16 @@ renametrig_partition(Relation tgrel, Oid partitionId, Oid parentTriggerOid,
17591759
* enablement/disablement, this also defines when the trigger
17601760
* should be fired in session replication roles.
17611761
* skip_system: if true, skip "system" triggers (constraint triggers)
1762+
* recurse: if true, recurse to partitions
17621763
*
17631764
* Caller should have checked permissions for the table; here we also
17641765
* enforce that superuser privilege is required to alter the state of
17651766
* system triggers
17661767
*/
17671768
void
1768-
EnableDisableTrigger(Relationrel,constchar*tgname,
1769-
charfires_when,boolskip_system,LOCKMODElockmode)
1769+
EnableDisableTriggerNew(Relationrel,constchar*tgname,
1770+
charfires_when,boolskip_system,boolrecurse,
1771+
LOCKMODElockmode)
17701772
{
17711773
Relationtgrel;
17721774
intnkeys;
@@ -1832,6 +1834,34 @@ EnableDisableTrigger(Relation rel, const char *tgname,
18321834
changed= true;
18331835
}
18341836

1837+
/*
1838+
* When altering FOR EACH ROW triggers on a partitioned table, do the
1839+
* same on the partitions as well, unless ONLY is specified.
1840+
*
1841+
* Note that we recurse even if we didn't change the trigger above,
1842+
* because the partitions' copy of the trigger may have a different
1843+
* value of tgenabled than the parent's trigger and thus might need to
1844+
* be changed.
1845+
*/
1846+
if (recurse&&
1847+
rel->rd_rel->relkind==RELKIND_PARTITIONED_TABLE&&
1848+
(TRIGGER_FOR_ROW(oldtrig->tgtype)))
1849+
{
1850+
PartitionDescpartdesc=RelationGetPartitionDesc(rel, true);
1851+
inti;
1852+
1853+
for (i=0;i<partdesc->nparts;i++)
1854+
{
1855+
Relationpart;
1856+
1857+
part=relation_open(partdesc->oids[i],lockmode);
1858+
EnableDisableTriggerNew(part,NameStr(oldtrig->tgname),
1859+
fires_when,skip_system,recurse,
1860+
lockmode);
1861+
table_close(part,NoLock);/* keep lock till commit */
1862+
}
1863+
}
1864+
18351865
InvokeObjectPostAlterHook(TriggerRelationId,
18361866
oldtrig->oid,0);
18371867
}
@@ -1855,6 +1885,19 @@ EnableDisableTrigger(Relation rel, const char *tgname,
18551885
CacheInvalidateRelcache(rel);
18561886
}
18571887

1888+
/*
1889+
* ABI-compatible wrapper for the above. To keep as close possible to the old
1890+
* behavior, this never recurses. Do not call this function in new code.
1891+
*/
1892+
void
1893+
EnableDisableTrigger(Relationrel,constchar*tgname,
1894+
charfires_when,boolskip_system,
1895+
LOCKMODElockmode)
1896+
{
1897+
EnableDisableTriggerNew(rel,tgname,fires_when,skip_system,
1898+
true,lockmode);
1899+
}
1900+
18581901

18591902
/*
18601903
* Build trigger data to attach to the given relcache entry.

‎src/backend/nodes/copyfuncs.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3893,6 +3893,7 @@ _copyAlterTableCmd(const AlterTableCmd *from)
38933893
COPY_NODE_FIELD(def);
38943894
COPY_SCALAR_FIELD(behavior);
38953895
COPY_SCALAR_FIELD(missing_ok);
3896+
COPY_SCALAR_FIELD(recurse);
38963897

38973898
returnnewnode;
38983899
}

‎src/backend/nodes/equalfuncs.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1521,6 +1521,7 @@ _equalAlterTableCmd(const AlterTableCmd *a, const AlterTableCmd *b)
15211521
COMPARE_NODE_FIELD(def);
15221522
COMPARE_SCALAR_FIELD(behavior);
15231523
COMPARE_SCALAR_FIELD(missing_ok);
1524+
COMPARE_SCALAR_FIELD(recurse);
15241525

15251526
return true;
15261527
}

‎src/include/commands/trigger.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,9 @@ extern Oidget_trigger_oid(Oid relid, const char *name, bool missing_ok);
170170

171171
externObjectAddressrenametrig(RenameStmt*stmt);
172172

173+
externvoidEnableDisableTriggerNew(Relationrel,constchar*tgname,
174+
charfires_when,boolskip_system,boolrecurse,
175+
LOCKMODElockmode);
173176
externvoidEnableDisableTrigger(Relationrel,constchar*tgname,
174177
charfires_when,boolskip_system,LOCKMODElockmode);
175178

‎src/include/nodes/parsenodes.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2317,6 +2317,7 @@ typedef struct AlterTableCmd/* one subcommand of an ALTER TABLE */
23172317
* constraint, or parent table */
23182318
DropBehaviorbehavior;/* RESTRICT or CASCADE for DROP cases */
23192319
boolmissing_ok;/* skip error if missing? */
2320+
boolrecurse;/* exec-time recursion */
23202321
}AlterTableCmd;
23212322

23222323

‎src/test/regress/expected/triggers.out

Lines changed: 29 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2681,24 +2681,42 @@ create table parent (a int) partition by list (a);
26812681
create table child1 partition of parent for values in (1);
26822682
create trigger tg after insert on parent
26832683
for each row execute procedure trig_nothing();
2684+
create trigger tg_stmt after insert on parent
2685+
for statement execute procedure trig_nothing();
26842686
select tgrelid::regclass, tgname, tgenabled from pg_trigger
26852687
where tgrelid in ('parent'::regclass, 'child1'::regclass)
26862688
order by tgrelid::regclass::text;
2687-
tgrelid | tgname | tgenabled
2688-
---------+--------+-----------
2689-
child1 | tg | O
2690-
parent | tg | O
2691-
(2 rows)
2689+
tgrelid | tgname | tgenabled
2690+
---------+---------+-----------
2691+
child1 | tg | O
2692+
parent | tg | O
2693+
parent | tg_stmt | O
2694+
(3 rows)
26922695

2693-
alter table only parent enable always trigger tg;
2696+
alter table only parent enable always trigger tg;-- no recursion because ONLY
2697+
alter table parent enable always trigger tg_stmt;-- no recursion because statement trigger
26942698
select tgrelid::regclass, tgname, tgenabled from pg_trigger
26952699
where tgrelid in ('parent'::regclass, 'child1'::regclass)
26962700
order by tgrelid::regclass::text;
2697-
tgrelid | tgname | tgenabled
2698-
---------+--------+-----------
2699-
child1 | tg | O
2700-
parent | tg | A
2701-
(2 rows)
2701+
tgrelid | tgname | tgenabled
2702+
---------+---------+-----------
2703+
child1 | tg | O
2704+
parent | tg | A
2705+
parent | tg_stmt | A
2706+
(3 rows)
2707+
2708+
-- The following is a no-op for the parent trigger but not so
2709+
-- for the child trigger, so recursion should be applied.
2710+
alter table parent enable always trigger tg;
2711+
select tgrelid::regclass, tgname, tgenabled from pg_trigger
2712+
where tgrelid in ('parent'::regclass, 'child1'::regclass)
2713+
order by tgrelid::regclass::text;
2714+
tgrelid | tgname | tgenabled
2715+
---------+---------+-----------
2716+
child1 | tg | A
2717+
parent | tg | A
2718+
parent | tg_stmt | A
2719+
(3 rows)
27022720

27032721
drop table parent, child1;
27042722
-- Verify that firing state propagates correctly on creation, too

‎src/test/regress/sql/triggers.sql

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1865,10 +1865,19 @@ create table parent (a int) partition by list (a);
18651865
createtablechild1 partition of parent forvaluesin (1);
18661866
createtriggertg after inserton parent
18671867
for each row execute procedure trig_nothing();
1868+
createtriggertg_stmt after inserton parent
1869+
for statement execute procedure trig_nothing();
18681870
select tgrelid::regclass, tgname, tgenabledfrom pg_trigger
18691871
where tgrelidin ('parent'::regclass,'child1'::regclass)
18701872
order by tgrelid::regclass::text;
1871-
altertable only parent enable always trigger tg;
1873+
altertable only parent enable always trigger tg;-- no recursion because ONLY
1874+
altertable parent enable always trigger tg_stmt;-- no recursion because statement trigger
1875+
select tgrelid::regclass, tgname, tgenabledfrom pg_trigger
1876+
where tgrelidin ('parent'::regclass,'child1'::regclass)
1877+
order by tgrelid::regclass::text;
1878+
-- The following is a no-op for the parent trigger but not so
1879+
-- for the child trigger, so recursion should be applied.
1880+
altertable parent enable always trigger tg;
18721881
select tgrelid::regclass, tgname, tgenabledfrom pg_trigger
18731882
where tgrelidin ('parent'::regclass,'child1'::regclass)
18741883
order by tgrelid::regclass::text;

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp