Skip to content

Commit cc340af

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 c31516a commit cc340af

File tree

6 files changed

+167
-13
lines changed

6 files changed

+167
-13
lines changed

‎src/bin/pg_dump/pg_dump.c

Lines changed: 88 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7816,6 +7816,7 @@ getTriggers(Archive *fout, TableInfo tblinfo[], int numTables)
78167816
i_tgconstrrelid,
78177817
i_tgconstrrelname,
78187818
i_tgenabled,
7819+
i_tgisinternal,
78197820
i_tgdeferrable,
78207821
i_tginitdeferred,
78217822
i_tgdef;
@@ -7834,18 +7835,63 @@ getTriggers(Archive *fout, TableInfo tblinfo[], int numTables)
78347835
tbinfo->dobj.name);
78357836

78367837
resetPQExpBuffer(query);
7837-
if (fout->remoteVersion >= 90000)
7838+
if (fout->remoteVersion >= 130000)
78387839
{
78397840
/*
78407841
* NB: think not to use pretty=true in pg_get_triggerdef. It
78417842
* could result in non-forward-compatible dumps of WHEN clauses
78427843
* due to under-parenthesization.
7844+
*
7845+
* NB: We need to see tgisinternal triggers in partitions, in case
7846+
* the tgenabled flag has been changed from the parent.
78437847
*/
78447848
appendPQExpBuffer(query,
7845-
"SELECT tgname, "
7846-
"tgfoid::pg_catalog.regproc AS tgfname, "
7847-
"pg_catalog.pg_get_triggerdef(oid, false) AS tgdef, "
7848-
"tgenabled, tableoid, oid "
7849+
"SELECT t.tgname, "
7850+
"t.tgfoid::pg_catalog.regproc AS tgfname, "
7851+
"pg_catalog.pg_get_triggerdef(t.oid, false) AS tgdef, "
7852+
"t.tgenabled, t.tableoid, t.oid, t.tgisinternal "
7853+
"FROM pg_catalog.pg_trigger t "
7854+
"LEFT JOIN pg_catalog.pg_trigger u ON u.oid = t.tgparentid "
7855+
"WHERE t.tgrelid = '%u'::pg_catalog.oid "
7856+
"AND (NOT t.tgisinternal OR t.tgenabled != u.tgenabled)",
7857+
tbinfo->dobj.catId.oid);
7858+
}
7859+
else if (fout->remoteVersion >= 110000)
7860+
{
7861+
/*
7862+
* NB: We need to see tgisinternal triggers in partitions, in case
7863+
* the tgenabled flag has been changed from the parent. No
7864+
* tgparentid in version 11-12, so we have to match them via
7865+
* pg_depend.
7866+
*
7867+
* See above about pretty=true in pg_get_triggerdef.
7868+
*/
7869+
appendPQExpBuffer(query,
7870+
"SELECT t.tgname, "
7871+
"t.tgfoid::pg_catalog.regproc AS tgfname, "
7872+
"pg_catalog.pg_get_triggerdef(t.oid, false) AS tgdef, "
7873+
"t.tgenabled, t.tableoid, t.oid, t.tgisinternal "
7874+
"FROM pg_catalog.pg_trigger t "
7875+
"LEFT JOIN pg_catalog.pg_depend AS d ON "
7876+
" d.classid = 'pg_catalog.pg_trigger'::pg_catalog.regclass AND "
7877+
" d.refclassid = 'pg_catalog.pg_trigger'::pg_catalog.regclass AND "
7878+
" d.objid = t.oid "
7879+
"LEFT JOIN pg_catalog.pg_trigger AS pt ON pt.oid = refobjid "
7880+
"WHERE t.tgrelid = '%u'::pg_catalog.oid "
7881+
"AND (NOT t.tgisinternal%s)",
7882+
tbinfo->dobj.catId.oid,
7883+
tbinfo->ispartition ?
7884+
" OR t.tgenabled != pt.tgenabled" : "");
7885+
}
7886+
else if (fout->remoteVersion >= 90000)
7887+
{
7888+
/* See above about pretty=true in pg_get_triggerdef */
7889+
appendPQExpBuffer(query,
7890+
"SELECT t.tgname, "
7891+
"t.tgfoid::pg_catalog.regproc AS tgfname, "
7892+
"pg_catalog.pg_get_triggerdef(t.oid, false) AS tgdef, "
7893+
"t.tgenabled, false as tgisinternal, "
7894+
"t.tableoid, t.oid "
78497895
"FROM pg_catalog.pg_trigger t "
78507896
"WHERE tgrelid = '%u'::pg_catalog.oid "
78517897
"AND NOT tgisinternal",
@@ -7860,6 +7906,7 @@ getTriggers(Archive *fout, TableInfo tblinfo[], int numTables)
78607906
"SELECT tgname, "
78617907
"tgfoid::pg_catalog.regproc AS tgfname, "
78627908
"tgtype, tgnargs, tgargs, tgenabled, "
7909+
"false as tgisinternal, "
78637910
"tgisconstraint, tgconstrname, tgdeferrable, "
78647911
"tgconstrrelid, tginitdeferred, tableoid, oid, "
78657912
"tgconstrrelid::pg_catalog.regclass AS tgconstrrelname "
@@ -7908,6 +7955,7 @@ getTriggers(Archive *fout, TableInfo tblinfo[], int numTables)
79087955
i_tgconstrrelid = PQfnumber(res, "tgconstrrelid");
79097956
i_tgconstrrelname = PQfnumber(res, "tgconstrrelname");
79107957
i_tgenabled = PQfnumber(res, "tgenabled");
7958+
i_tgisinternal = PQfnumber(res, "tgisinternal");
79117959
i_tgdeferrable = PQfnumber(res, "tgdeferrable");
79127960
i_tginitdeferred = PQfnumber(res, "tginitdeferred");
79137961
i_tgdef = PQfnumber(res, "tgdef");
@@ -7927,6 +7975,7 @@ getTriggers(Archive *fout, TableInfo tblinfo[], int numTables)
79277975
tginfo[j].dobj.namespace = tbinfo->dobj.namespace;
79287976
tginfo[j].tgtable = tbinfo;
79297977
tginfo[j].tgenabled = *(PQgetvalue(res, j, i_tgenabled));
7978+
tginfo[j].tgisinternal = *(PQgetvalue(res, j, i_tgisinternal)) == 't';
79307979
if (i_tgdef >= 0)
79317980
{
79327981
tginfo[j].tgdef = pg_strdup(PQgetvalue(res, j, i_tgdef));
@@ -17681,7 +17730,40 @@ dumpTrigger(Archive *fout, TriggerInfo *tginfo)
1768117730
"pg_catalog.pg_trigger", "TRIGGER",
1768217731
trigidentity->data);
1768317732

17684-
if (tginfo->tgenabled != 't' && tginfo->tgenabled != 'O')
17733+
if (tginfo->tgisinternal)
17734+
{
17735+
/*
17736+
* Triggers marked internal only appear here because their 'tgenabled'
17737+
* flag differs from its parent's. The trigger is created already, so
17738+
* remove the CREATE and replace it with an ALTER. (Clear out the
17739+
* DROP query too, so that pg_dump --create does not cause errors.)
17740+
*/
17741+
resetPQExpBuffer(query);
17742+
resetPQExpBuffer(delqry);
17743+
appendPQExpBuffer(query, "\nALTER %sTABLE %s ",
17744+
tbinfo->relkind == RELKIND_FOREIGN_TABLE ? "FOREIGN " : "",
17745+
fmtQualifiedDumpable(tbinfo));
17746+
switch (tginfo->tgenabled)
17747+
{
17748+
case 'f':
17749+
case 'D':
17750+
appendPQExpBufferStr(query, "DISABLE");
17751+
break;
17752+
case 't':
17753+
case 'O':
17754+
appendPQExpBufferStr(query, "ENABLE");
17755+
break;
17756+
case 'R':
17757+
appendPQExpBufferStr(query, "ENABLE REPLICA");
17758+
break;
17759+
case 'A':
17760+
appendPQExpBufferStr(query, "ENABLE ALWAYS");
17761+
break;
17762+
}
17763+
appendPQExpBuffer(query, " TRIGGER %s;\n",
17764+
fmtId(tginfo->dobj.name));
17765+
}
17766+
else if (tginfo->tgenabled != 't' && tginfo->tgenabled != 'O')
1768517767
{
1768617768
appendPQExpBuffer(query, "\nALTER %sTABLE %s ",
1768717769
tbinfo->relkind == RELKIND_FOREIGN_TABLE ? "FOREIGN " : "",

‎src/bin/pg_dump/pg_dump.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -413,6 +413,7 @@ typedef struct _triggerInfo
413413
Oid tgconstrrelid;
414414
char *tgconstrrelname;
415415
char tgenabled;
416+
bool tgisinternal;
416417
bool tgdeferrable;
417418
bool tginitdeferred;
418419
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
@@ -2400,12 +2400,68 @@
24002400
},
24012401
},
24022402
2403-
# this shouldn't ever get emitted
2404-
'Creation of row-level trigger in partition' => {
2403+
'Disabled trigger on partition is altered' => {
2404+
create_order => 93,
2405+
create_sql =>
2406+
'CREATE TABLE dump_test_second_schema.measurement_y2006m3
2407+
PARTITION OF dump_test.measurement
2408+
FOR VALUES FROM (\'2006-03-01\') TO (\'2006-04-01\');
2409+
ALTER TABLE dump_test_second_schema.measurement_y2006m3 DISABLE TRIGGER test_trigger;
2410+
CREATE TABLE dump_test_second_schema.measurement_y2006m4
2411+
PARTITION OF dump_test.measurement
2412+
FOR VALUES FROM (\'2006-04-01\') TO (\'2006-05-01\');
2413+
ALTER TABLE dump_test_second_schema.measurement_y2006m4 ENABLE REPLICA TRIGGER test_trigger;
2414+
CREATE TABLE dump_test_second_schema.measurement_y2006m5
2415+
PARTITION OF dump_test.measurement
2416+
FOR VALUES FROM (\'2006-05-01\') TO (\'2006-06-01\');
2417+
ALTER TABLE dump_test_second_schema.measurement_y2006m5 ENABLE ALWAYS TRIGGER test_trigger;
2418+
',
2419+
regexp => qr/^
2420+
\QALTER TABLE dump_test_second_schema.measurement_y2006m3 DISABLE TRIGGER test_trigger;\E
2421+
/xm,
2422+
like => {
2423+
%full_runs,
2424+
section_post_data => 1,
2425+
role => 1,
2426+
binary_upgrade => 1,
2427+
},
2428+
},
2429+
2430+
'Replica trigger on partition is altered' => {
24052431
regexp => qr/^
2406-
\QCREATE TRIGGER test_trigger AFTER INSERT ON dump_test_second_schema.measurement\E
2432+
\QALTER TABLE dump_test_second_schema.measurement_y2006m4 ENABLE REPLICA TRIGGER test_trigger;\E
24072433
/xm,
2408-
like => {},
2434+
like => {
2435+
%full_runs,
2436+
section_post_data => 1,
2437+
role => 1,
2438+
binary_upgrade => 1,
2439+
},
2440+
},
2441+
2442+
'Always trigger on partition is altered' => {
2443+
regexp => qr/^
2444+
\QALTER TABLE dump_test_second_schema.measurement_y2006m5 ENABLE ALWAYS TRIGGER test_trigger;\E
2445+
/xm,
2446+
like => {
2447+
%full_runs,
2448+
section_post_data => 1,
2449+
role => 1,
2450+
binary_upgrade => 1,
2451+
},
2452+
},
2453+
2454+
# We should never see the creation of a trigger on a partition
2455+
'Disabled trigger on partition is not created' => {
2456+
regexp => qr/CREATE TRIGGER test_trigger.*ON dump_test_second_schema/,
2457+
like => {},
2458+
unlike => { %full_runs, %dump_test_schema_runs },
2459+
},
2460+
2461+
# Triggers on partitions should not be dropped individually
2462+
'Triggers on partitions are not dropped' => {
2463+
regexp => qr/DROP TRIGGER test_trigger.*ON dump_test_second_schema/,
2464+
like => {}
24092465
},
24102466
24112467
'CREATE TABLE test_fourth_table_zero_col' => {
@@ -3044,9 +3100,12 @@
30443100
},
30453101
30463102
'GRANT SELECT ON TABLE measurement_y2006m2' => {
3047-
create_order => 92,
3048-
create_sql => 'GRANT SELECT ON
3049-
TABLE dump_test_second_schema.measurement_y2006m2
3103+
create_order => 94,
3104+
create_sql => 'GRANT SELECT ON TABLE
3105+
dump_test_second_schema.measurement_y2006m2,
3106+
dump_test_second_schema.measurement_y2006m3,
3107+
dump_test_second_schema.measurement_y2006m4,
3108+
dump_test_second_schema.measurement_y2006m5
30503109
TO regress_dump_test_role;',
30513110
regexp =>
30523111
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: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,8 @@ tmp|f
211211
trigger_parted|t
212212
trigger_parted_p1|t
213213
trigger_parted_p1_1|t
214+
trigger_parted_p2|t
215+
trigger_parted_p2_2|t
214216
varchar_tbl|f
215217
view_base_table|t
216218
-- restore normal output mode

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3257,6 +3257,11 @@ create trigger aft_row after insert or update on trigger_parted
32573257
create table trigger_parted_p1 partition of trigger_parted for values in (1)
32583258
partition by list (a);
32593259
create table trigger_parted_p1_1 partition of trigger_parted_p1 for values in (1);
3260+
create table trigger_parted_p2 partition of trigger_parted for values in (2)
3261+
partition by list (a);
3262+
create table trigger_parted_p2_2 partition of trigger_parted_p2 for values in (2);
3263+
alter table only trigger_parted_p2 disable trigger aft_row;
3264+
alter table trigger_parted_p2_2 enable always trigger aft_row;
32603265
-- verify transition table conversion slot's lifetime
32613266
-- https://postgr.es/m/39a71864-b120-5a5c-8cc5-c632b6f16761@amazon.com
32623267
create table convslot_test_parent (col1 text primary key);

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2415,6 +2415,11 @@ create trigger aft_row after insert or update on trigger_parted
24152415
create table trigger_parted_p1 partition of trigger_parted for values in (1)
24162416
partition by list (a);
24172417
create table trigger_parted_p1_1 partition of trigger_parted_p1 for values in (1);
2418+
create table trigger_parted_p2 partition of trigger_parted for values in (2)
2419+
partition by list (a);
2420+
create table trigger_parted_p2_2 partition of trigger_parted_p2 for values in (2);
2421+
alter table only trigger_parted_p2 disable trigger aft_row;
2422+
alter table trigger_parted_p2_2 enable always trigger aft_row;
24182423

24192424
-- verify transition table conversion slot's lifetime
24202425
-- https://postgr.es/m/39a71864-b120-5a5c-8cc5-c632b6f16761@amazon.com

0 commit comments

Comments
 (0)