Skip to content

Commit 5992c94

Browse files
alvherrejustinpryzby
alvherre
and
justinpryzby
committed
Fix pg_dump for disabled triggers on partitioned tables
pg_dump failed to preserve the 'enabled' flag (which can be not only disabled, but also REPLICA or ALWAYS) for partitions which had it changed from their respective parents. Attempt to handle that by including a definition for such triggers in the dump, but replace the standard CREATE TRIGGER line with an ALTER TRIGGER line. Backpatch to 11, where these triggers can exist. In branches 11 and 12, pick up a few test lines from commit b9b408c to verify that pg_upgrade is okay with these arrangements. Co-authored-by: Justin Pryzby <pryzby@telsasoft.com> Co-authored-by: Álvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://postgr.es/m/20200930223450.GA14848@telsasoft.com
1 parent 7584ec1 commit 5992c94

File tree

6 files changed

+189
-13
lines changed

6 files changed

+189
-13
lines changed

‎src/bin/pg_dump/pg_dump.c

Lines changed: 88 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7696,6 +7696,7 @@ getTriggers(Archive *fout, TableInfo tblinfo[], int numTables)
76967696
i_tgconstrrelid,
76977697
i_tgconstrrelname,
76987698
i_tgenabled,
7699+
i_tgisinternal,
76997700
i_tgdeferrable,
77007701
i_tginitdeferred,
77017702
i_tgdef;
@@ -7714,18 +7715,63 @@ getTriggers(Archive *fout, TableInfo tblinfo[], int numTables)
77147715
tbinfo->dobj.name);
77157716

77167717
resetPQExpBuffer(query);
7717-
if (fout->remoteVersion >= 90000)
7718+
if (fout->remoteVersion >= 130000)
77187719
{
77197720
/*
77207721
* NB: think not to use pretty=true in pg_get_triggerdef. It
77217722
* could result in non-forward-compatible dumps of WHEN clauses
77227723
* due to under-parenthesization.
7724+
*
7725+
* NB: We need to see tgisinternal triggers in partitions, in case
7726+
* the tgenabled flag has been changed from the parent.
77237727
*/
77247728
appendPQExpBuffer(query,
7725-
"SELECT tgname, "
7726-
"tgfoid::pg_catalog.regproc AS tgfname, "
7727-
"pg_catalog.pg_get_triggerdef(oid, false) AS tgdef, "
7728-
"tgenabled, tableoid, oid "
7729+
"SELECT t.tgname, "
7730+
"t.tgfoid::pg_catalog.regproc AS tgfname, "
7731+
"pg_catalog.pg_get_triggerdef(t.oid, false) AS tgdef, "
7732+
"t.tgenabled, t.tableoid, t.oid, t.tgisinternal "
7733+
"FROM pg_catalog.pg_trigger t "
7734+
"LEFT JOIN pg_catalog.pg_trigger u ON u.oid = t.tgparentid "
7735+
"WHERE t.tgrelid = '%u'::pg_catalog.oid "
7736+
"AND (NOT t.tgisinternal OR t.tgenabled != u.tgenabled)",
7737+
tbinfo->dobj.catId.oid);
7738+
}
7739+
else if (fout->remoteVersion >= 110000)
7740+
{
7741+
/*
7742+
* NB: We need to see tgisinternal triggers in partitions, in case
7743+
* the tgenabled flag has been changed from the parent. No
7744+
* tgparentid in version 11-12, so we have to match them via
7745+
* pg_depend.
7746+
*
7747+
* See above about pretty=true in pg_get_triggerdef.
7748+
*/
7749+
appendPQExpBuffer(query,
7750+
"SELECT t.tgname, "
7751+
"t.tgfoid::pg_catalog.regproc AS tgfname, "
7752+
"pg_catalog.pg_get_triggerdef(t.oid, false) AS tgdef, "
7753+
"t.tgenabled, t.tableoid, t.oid, t.tgisinternal "
7754+
"FROM pg_catalog.pg_trigger t "
7755+
"LEFT JOIN pg_catalog.pg_depend AS d ON "
7756+
" d.classid = 'pg_catalog.pg_trigger'::pg_catalog.regclass AND "
7757+
" d.refclassid = 'pg_catalog.pg_trigger'::pg_catalog.regclass AND "
7758+
" d.objid = t.oid "
7759+
"LEFT JOIN pg_catalog.pg_trigger AS pt ON pt.oid = refobjid "
7760+
"WHERE t.tgrelid = '%u'::pg_catalog.oid "
7761+
"AND (NOT t.tgisinternal%s)",
7762+
tbinfo->dobj.catId.oid,
7763+
tbinfo->ispartition ?
7764+
" OR t.tgenabled != pt.tgenabled" : "");
7765+
}
7766+
else if (fout->remoteVersion >= 90000)
7767+
{
7768+
/* See above about pretty=true in pg_get_triggerdef */
7769+
appendPQExpBuffer(query,
7770+
"SELECT t.tgname, "
7771+
"t.tgfoid::pg_catalog.regproc AS tgfname, "
7772+
"pg_catalog.pg_get_triggerdef(t.oid, false) AS tgdef, "
7773+
"t.tgenabled, false as tgisinternal, "
7774+
"t.tableoid, t.oid "
77297775
"FROM pg_catalog.pg_trigger t "
77307776
"WHERE tgrelid = '%u'::pg_catalog.oid "
77317777
"AND NOT tgisinternal",
@@ -7740,6 +7786,7 @@ getTriggers(Archive *fout, TableInfo tblinfo[], int numTables)
77407786
"SELECT tgname, "
77417787
"tgfoid::pg_catalog.regproc AS tgfname, "
77427788
"tgtype, tgnargs, tgargs, tgenabled, "
7789+
"false as tgisinternal, "
77437790
"tgisconstraint, tgconstrname, tgdeferrable, "
77447791
"tgconstrrelid, tginitdeferred, tableoid, oid, "
77457792
"tgconstrrelid::pg_catalog.regclass AS tgconstrrelname "
@@ -7788,6 +7835,7 @@ getTriggers(Archive *fout, TableInfo tblinfo[], int numTables)
77887835
i_tgconstrrelid = PQfnumber(res, "tgconstrrelid");
77897836
i_tgconstrrelname = PQfnumber(res, "tgconstrrelname");
77907837
i_tgenabled = PQfnumber(res, "tgenabled");
7838+
i_tgisinternal = PQfnumber(res, "tgisinternal");
77917839
i_tgdeferrable = PQfnumber(res, "tgdeferrable");
77927840
i_tginitdeferred = PQfnumber(res, "tginitdeferred");
77937841
i_tgdef = PQfnumber(res, "tgdef");
@@ -7807,6 +7855,7 @@ getTriggers(Archive *fout, TableInfo tblinfo[], int numTables)
78077855
tginfo[j].dobj.namespace = tbinfo->dobj.namespace;
78087856
tginfo[j].tgtable = tbinfo;
78097857
tginfo[j].tgenabled = *(PQgetvalue(res, j, i_tgenabled));
7858+
tginfo[j].tgisinternal = *(PQgetvalue(res, j, i_tgisinternal)) == 't';
78107859
if (i_tgdef >= 0)
78117860
{
78127861
tginfo[j].tgdef = pg_strdup(PQgetvalue(res, j, i_tgdef));
@@ -17558,7 +17607,40 @@ dumpTrigger(Archive *fout, TriggerInfo *tginfo)
1755817607
"pg_catalog.pg_trigger", "TRIGGER",
1755917608
trigidentity->data);
1756017609

17561-
if (tginfo->tgenabled != 't' && tginfo->tgenabled != 'O')
17610+
if (tginfo->tgisinternal)
17611+
{
17612+
/*
17613+
* Triggers marked internal only appear here because their 'tgenabled'
17614+
* flag differs from its parent's. The trigger is created already, so
17615+
* remove the CREATE and replace it with an ALTER. (Clear out the
17616+
* DROP query too, so that pg_dump --create does not cause errors.)
17617+
*/
17618+
resetPQExpBuffer(query);
17619+
resetPQExpBuffer(delqry);
17620+
appendPQExpBuffer(query, "\nALTER %sTABLE %s ",
17621+
tbinfo->relkind == RELKIND_FOREIGN_TABLE ? "FOREIGN " : "",
17622+
fmtQualifiedDumpable(tbinfo));
17623+
switch (tginfo->tgenabled)
17624+
{
17625+
case 'f':
17626+
case 'D':
17627+
appendPQExpBufferStr(query, "DISABLE");
17628+
break;
17629+
case 't':
17630+
case 'O':
17631+
appendPQExpBufferStr(query, "ENABLE");
17632+
break;
17633+
case 'R':
17634+
appendPQExpBufferStr(query, "ENABLE REPLICA");
17635+
break;
17636+
case 'A':
17637+
appendPQExpBufferStr(query, "ENABLE ALWAYS");
17638+
break;
17639+
}
17640+
appendPQExpBuffer(query, " TRIGGER %s;\n",
17641+
fmtId(tginfo->dobj.name));
17642+
}
17643+
else if (tginfo->tgenabled != 't' && tginfo->tgenabled != 'O')
1756217644
{
1756317645
appendPQExpBuffer(query, "\nALTER TABLE %s ",
1756417646
fmtQualifiedDumpable(tbinfo));

‎src/bin/pg_dump/pg_dump.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -415,6 +415,7 @@ typedef struct _triggerInfo
415415
Oid tgconstrrelid;
416416
char *tgconstrrelname;
417417
char tgenabled;
418+
bool tgisinternal;
418419
bool tgdeferrable;
419420
bool tginitdeferred;
420421
char *tgdef;

‎src/bin/pg_dump/t/002_pg_dump.pl

Lines changed: 66 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2386,12 +2386,68 @@
23862386
},
23872387
},
23882388
2389-
# this shouldn't ever get emitted
2390-
'Creation of row-level trigger in partition' => {
2389+
'Disabled trigger on partition is altered' => {
2390+
create_order => 93,
2391+
create_sql =>
2392+
'CREATE TABLE dump_test_second_schema.measurement_y2006m3
2393+
PARTITION OF dump_test.measurement
2394+
FOR VALUES FROM (\'2006-03-01\') TO (\'2006-04-01\');
2395+
ALTER TABLE dump_test_second_schema.measurement_y2006m3 DISABLE TRIGGER test_trigger;
2396+
CREATE TABLE dump_test_second_schema.measurement_y2006m4
2397+
PARTITION OF dump_test.measurement
2398+
FOR VALUES FROM (\'2006-04-01\') TO (\'2006-05-01\');
2399+
ALTER TABLE dump_test_second_schema.measurement_y2006m4 ENABLE REPLICA TRIGGER test_trigger;
2400+
CREATE TABLE dump_test_second_schema.measurement_y2006m5
2401+
PARTITION OF dump_test.measurement
2402+
FOR VALUES FROM (\'2006-05-01\') TO (\'2006-06-01\');
2403+
ALTER TABLE dump_test_second_schema.measurement_y2006m5 ENABLE ALWAYS TRIGGER test_trigger;
2404+
',
2405+
regexp => qr/^
2406+
\QALTER TABLE dump_test_second_schema.measurement_y2006m3 DISABLE TRIGGER test_trigger;\E
2407+
/xm,
2408+
like => {
2409+
%full_runs,
2410+
section_post_data => 1,
2411+
role => 1,
2412+
binary_upgrade => 1,
2413+
},
2414+
},
2415+
2416+
'Replica trigger on partition is altered' => {
23912417
regexp => qr/^
2392-
\QCREATE TRIGGER test_trigger AFTER INSERT ON dump_test_second_schema.measurement\E
2418+
\QALTER TABLE dump_test_second_schema.measurement_y2006m4 ENABLE REPLICA TRIGGER test_trigger;\E
23932419
/xm,
2394-
like => {},
2420+
like => {
2421+
%full_runs,
2422+
section_post_data => 1,
2423+
role => 1,
2424+
binary_upgrade => 1,
2425+
},
2426+
},
2427+
2428+
'Always trigger on partition is altered' => {
2429+
regexp => qr/^
2430+
\QALTER TABLE dump_test_second_schema.measurement_y2006m5 ENABLE ALWAYS TRIGGER test_trigger;\E
2431+
/xm,
2432+
like => {
2433+
%full_runs,
2434+
section_post_data => 1,
2435+
role => 1,
2436+
binary_upgrade => 1,
2437+
},
2438+
},
2439+
2440+
# We should never see the creation of a trigger on a partition
2441+
'Disabled trigger on partition is not created' => {
2442+
regexp => qr/CREATE TRIGGER test_trigger.*ON dump_test_second_schema/,
2443+
like => {},
2444+
unlike => { %full_runs, %dump_test_schema_runs },
2445+
},
2446+
2447+
# Triggers on partitions should not be dropped individually
2448+
'Triggers on partitions are not dropped' => {
2449+
regexp => qr/DROP TRIGGER test_trigger.*ON dump_test_second_schema/,
2450+
like => {}
23952451
},
23962452
23972453
'CREATE TABLE test_fourth_table_zero_col' => {
@@ -3018,9 +3074,12 @@
30183074
},
30193075
30203076
'GRANT SELECT ON TABLE measurement_y2006m2' => {
3021-
create_order => 92,
3022-
create_sql => 'GRANT SELECT ON
3023-
TABLE dump_test_second_schema.measurement_y2006m2
3077+
create_order => 94,
3078+
create_sql => 'GRANT SELECT ON TABLE
3079+
dump_test_second_schema.measurement_y2006m2,
3080+
dump_test_second_schema.measurement_y2006m3,
3081+
dump_test_second_schema.measurement_y2006m4,
3082+
dump_test_second_schema.measurement_y2006m5
30243083
TO regress_dump_test_role;',
30253084
regexp =>
30263085
qr/^\QGRANT SELECT ON TABLE dump_test_second_schema.measurement_y2006m2 TO regress_dump_test_role;\E/m,

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,11 @@ timestamp_tbl|f
210210
timestamptz_tbl|f
211211
timetz_tbl|f
212212
tmp|f
213+
trigger_parted|t
214+
trigger_parted_p1|t
215+
trigger_parted_p1_1|t
216+
trigger_parted_p2|t
217+
trigger_parted_p2_2|t
213218
varchar_tbl|f
214219
view_base_table|t
215220
-- restore normal output mode

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3139,6 +3139,20 @@ drop table self_ref;
31393139
drop function dump_insert();
31403140
drop function dump_update();
31413141
drop function dump_delete();
3142+
-- Leave around some objects for other tests
3143+
create table trigger_parted (a int primary key) partition by list (a);
3144+
create function trigger_parted_trigfunc() returns trigger language plpgsql as
3145+
$$ begin end; $$;
3146+
create trigger aft_row after insert or update on trigger_parted
3147+
for each row execute function trigger_parted_trigfunc();
3148+
create table trigger_parted_p1 partition of trigger_parted for values in (1)
3149+
partition by list (a);
3150+
create table trigger_parted_p1_1 partition of trigger_parted_p1 for values in (1);
3151+
create table trigger_parted_p2 partition of trigger_parted for values in (2)
3152+
partition by list (a);
3153+
create table trigger_parted_p2_2 partition of trigger_parted_p2 for values in (2);
3154+
alter table only trigger_parted_p2 disable trigger aft_row;
3155+
alter table trigger_parted_p2_2 enable always trigger aft_row;
31423156
-- verify transition table conversion slot's lifetime
31433157
-- https://postgr.es/m/39a71864-b120-5a5c-8cc5-c632b6f16761@amazon.com
31443158
create table convslot_test_parent (col1 text primary key);

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2329,6 +2329,21 @@ drop function dump_insert();
23292329
drop function dump_update();
23302330
drop function dump_delete();
23312331

2332+
-- Leave around some objects for other tests
2333+
create table trigger_parted (a int primary key) partition by list (a);
2334+
create function trigger_parted_trigfunc() returns trigger language plpgsql as
2335+
$$ begin end; $$;
2336+
create trigger aft_row after insert or update on trigger_parted
2337+
for each row execute function trigger_parted_trigfunc();
2338+
create table trigger_parted_p1 partition of trigger_parted for values in (1)
2339+
partition by list (a);
2340+
create table trigger_parted_p1_1 partition of trigger_parted_p1 for values in (1);
2341+
create table trigger_parted_p2 partition of trigger_parted for values in (2)
2342+
partition by list (a);
2343+
create table trigger_parted_p2_2 partition of trigger_parted_p2 for values in (2);
2344+
alter table only trigger_parted_p2 disable trigger aft_row;
2345+
alter table trigger_parted_p2_2 enable always trigger aft_row;
2346+
23322347
-- verify transition table conversion slot's lifetime
23332348
-- https://postgr.es/m/39a71864-b120-5a5c-8cc5-c632b6f16761@amazon.com
23342349
create table convslot_test_parent (col1 text primary key);

0 commit comments

Comments
 (0)