Skip to content

Commit 3c5b7c6

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 eef92de commit 3c5b7c6

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
@@ -7946,6 +7946,7 @@ getTriggers(Archive *fout, TableInfo tblinfo[], int numTables)
79467946
i_tgconstrrelid,
79477947
i_tgconstrrelname,
79487948
i_tgenabled,
7949+
i_tgisinternal,
79497950
i_tgdeferrable,
79507951
i_tginitdeferred,
79517952
i_tgdef;
@@ -7964,18 +7965,63 @@ getTriggers(Archive *fout, TableInfo tblinfo[], int numTables)
79647965
tbinfo->dobj.name);
79657966

79667967
resetPQExpBuffer(query);
7967-
if (fout->remoteVersion >= 90000)
7968+
if (fout->remoteVersion >= 130000)
79687969
{
79697970
/*
79707971
* NB: think not to use pretty=true in pg_get_triggerdef. It
79717972
* could result in non-forward-compatible dumps of WHEN clauses
79727973
* due to under-parenthesization.
7974+
*
7975+
* NB: We need to see tgisinternal triggers in partitions, in case
7976+
* the tgenabled flag has been changed from the parent.
79737977
*/
79747978
appendPQExpBuffer(query,
7975-
"SELECT tgname, "
7976-
"tgfoid::pg_catalog.regproc AS tgfname, "
7977-
"pg_catalog.pg_get_triggerdef(oid, false) AS tgdef, "
7978-
"tgenabled, tableoid, oid "
7979+
"SELECT t.tgname, "
7980+
"t.tgfoid::pg_catalog.regproc AS tgfname, "
7981+
"pg_catalog.pg_get_triggerdef(t.oid, false) AS tgdef, "
7982+
"t.tgenabled, t.tableoid, t.oid, t.tgisinternal "
7983+
"FROM pg_catalog.pg_trigger t "
7984+
"LEFT JOIN pg_catalog.pg_trigger u ON u.oid = t.tgparentid "
7985+
"WHERE t.tgrelid = '%u'::pg_catalog.oid "
7986+
"AND (NOT t.tgisinternal OR t.tgenabled != u.tgenabled)",
7987+
tbinfo->dobj.catId.oid);
7988+
}
7989+
else if (fout->remoteVersion >= 110000)
7990+
{
7991+
/*
7992+
* NB: We need to see tgisinternal triggers in partitions, in case
7993+
* the tgenabled flag has been changed from the parent. No
7994+
* tgparentid in version 11-12, so we have to match them via
7995+
* pg_depend.
7996+
*
7997+
* See above about pretty=true in pg_get_triggerdef.
7998+
*/
7999+
appendPQExpBuffer(query,
8000+
"SELECT t.tgname, "
8001+
"t.tgfoid::pg_catalog.regproc AS tgfname, "
8002+
"pg_catalog.pg_get_triggerdef(t.oid, false) AS tgdef, "
8003+
"t.tgenabled, t.tableoid, t.oid, t.tgisinternal "
8004+
"FROM pg_catalog.pg_trigger t "
8005+
"LEFT JOIN pg_catalog.pg_depend AS d ON "
8006+
" d.classid = 'pg_catalog.pg_trigger'::pg_catalog.regclass AND "
8007+
" d.refclassid = 'pg_catalog.pg_trigger'::pg_catalog.regclass AND "
8008+
" d.objid = t.oid "
8009+
"LEFT JOIN pg_catalog.pg_trigger AS pt ON pt.oid = refobjid "
8010+
"WHERE t.tgrelid = '%u'::pg_catalog.oid "
8011+
"AND (NOT t.tgisinternal%s)",
8012+
tbinfo->dobj.catId.oid,
8013+
tbinfo->ispartition ?
8014+
" OR t.tgenabled != pt.tgenabled" : "");
8015+
}
8016+
else if (fout->remoteVersion >= 90000)
8017+
{
8018+
/* See above about pretty=true in pg_get_triggerdef */
8019+
appendPQExpBuffer(query,
8020+
"SELECT t.tgname, "
8021+
"t.tgfoid::pg_catalog.regproc AS tgfname, "
8022+
"pg_catalog.pg_get_triggerdef(t.oid, false) AS tgdef, "
8023+
"t.tgenabled, false as tgisinternal, "
8024+
"t.tableoid, t.oid "
79798025
"FROM pg_catalog.pg_trigger t "
79808026
"WHERE tgrelid = '%u'::pg_catalog.oid "
79818027
"AND NOT tgisinternal",
@@ -7990,6 +8036,7 @@ getTriggers(Archive *fout, TableInfo tblinfo[], int numTables)
79908036
"SELECT tgname, "
79918037
"tgfoid::pg_catalog.regproc AS tgfname, "
79928038
"tgtype, tgnargs, tgargs, tgenabled, "
8039+
"false as tgisinternal, "
79938040
"tgisconstraint, tgconstrname, tgdeferrable, "
79948041
"tgconstrrelid, tginitdeferred, tableoid, oid, "
79958042
"tgconstrrelid::pg_catalog.regclass AS tgconstrrelname "
@@ -8038,6 +8085,7 @@ getTriggers(Archive *fout, TableInfo tblinfo[], int numTables)
80388085
i_tgconstrrelid = PQfnumber(res, "tgconstrrelid");
80398086
i_tgconstrrelname = PQfnumber(res, "tgconstrrelname");
80408087
i_tgenabled = PQfnumber(res, "tgenabled");
8088+
i_tgisinternal = PQfnumber(res, "tgisinternal");
80418089
i_tgdeferrable = PQfnumber(res, "tgdeferrable");
80428090
i_tginitdeferred = PQfnumber(res, "tginitdeferred");
80438091
i_tgdef = PQfnumber(res, "tgdef");
@@ -8057,6 +8105,7 @@ getTriggers(Archive *fout, TableInfo tblinfo[], int numTables)
80578105
tginfo[j].dobj.namespace = tbinfo->dobj.namespace;
80588106
tginfo[j].tgtable = tbinfo;
80598107
tginfo[j].tgenabled = *(PQgetvalue(res, j, i_tgenabled));
8108+
tginfo[j].tgisinternal = *(PQgetvalue(res, j, i_tgisinternal)) == 't';
80608109
if (i_tgdef >= 0)
80618110
{
80628111
tginfo[j].tgdef = pg_strdup(PQgetvalue(res, j, i_tgdef));
@@ -17691,7 +17740,40 @@ dumpTrigger(Archive *fout, const TriggerInfo *tginfo)
1769117740
"pg_catalog.pg_trigger", "TRIGGER",
1769217741
trigidentity->data);
1769317742

17694-
if (tginfo->tgenabled != 't' && tginfo->tgenabled != 'O')
17743+
if (tginfo->tgisinternal)
17744+
{
17745+
/*
17746+
* Triggers marked internal only appear here because their 'tgenabled'
17747+
* flag differs from its parent's. The trigger is created already, so
17748+
* remove the CREATE and replace it with an ALTER. (Clear out the
17749+
* DROP query too, so that pg_dump --create does not cause errors.)
17750+
*/
17751+
resetPQExpBuffer(query);
17752+
resetPQExpBuffer(delqry);
17753+
appendPQExpBuffer(query, "\nALTER %sTABLE %s ",
17754+
tbinfo->relkind == RELKIND_FOREIGN_TABLE ? "FOREIGN " : "",
17755+
fmtQualifiedDumpable(tbinfo));
17756+
switch (tginfo->tgenabled)
17757+
{
17758+
case 'f':
17759+
case 'D':
17760+
appendPQExpBufferStr(query, "DISABLE");
17761+
break;
17762+
case 't':
17763+
case 'O':
17764+
appendPQExpBufferStr(query, "ENABLE");
17765+
break;
17766+
case 'R':
17767+
appendPQExpBufferStr(query, "ENABLE REPLICA");
17768+
break;
17769+
case 'A':
17770+
appendPQExpBufferStr(query, "ENABLE ALWAYS");
17771+
break;
17772+
}
17773+
appendPQExpBuffer(query, " TRIGGER %s;\n",
17774+
fmtId(tginfo->dobj.name));
17775+
}
17776+
else if (tginfo->tgenabled != 't' && tginfo->tgenabled != 'O')
1769517777
{
1769617778
appendPQExpBuffer(query, "\nALTER %sTABLE %s ",
1769717779
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
@@ -423,6 +423,7 @@ typedef struct _triggerInfo
423423
Oid tgconstrrelid;
424424
char *tgconstrrelname;
425425
char tgenabled;
426+
bool tgisinternal;
426427
bool tgdeferrable;
427428
bool tginitdeferred;
428429
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
@@ -2464,12 +2464,68 @@
24642464
},
24652465
},
24662466
2467-
# this shouldn't ever get emitted
2468-
'Creation of row-level trigger in partition' => {
2467+
'Disabled trigger on partition is altered' => {
2468+
create_order => 93,
2469+
create_sql =>
2470+
'CREATE TABLE dump_test_second_schema.measurement_y2006m3
2471+
PARTITION OF dump_test.measurement
2472+
FOR VALUES FROM (\'2006-03-01\') TO (\'2006-04-01\');
2473+
ALTER TABLE dump_test_second_schema.measurement_y2006m3 DISABLE TRIGGER test_trigger;
2474+
CREATE TABLE dump_test_second_schema.measurement_y2006m4
2475+
PARTITION OF dump_test.measurement
2476+
FOR VALUES FROM (\'2006-04-01\') TO (\'2006-05-01\');
2477+
ALTER TABLE dump_test_second_schema.measurement_y2006m4 ENABLE REPLICA TRIGGER test_trigger;
2478+
CREATE TABLE dump_test_second_schema.measurement_y2006m5
2479+
PARTITION OF dump_test.measurement
2480+
FOR VALUES FROM (\'2006-05-01\') TO (\'2006-06-01\');
2481+
ALTER TABLE dump_test_second_schema.measurement_y2006m5 ENABLE ALWAYS TRIGGER test_trigger;
2482+
',
2483+
regexp => qr/^
2484+
\QALTER TABLE dump_test_second_schema.measurement_y2006m3 DISABLE TRIGGER test_trigger;\E
2485+
/xm,
2486+
like => {
2487+
%full_runs,
2488+
section_post_data => 1,
2489+
role => 1,
2490+
binary_upgrade => 1,
2491+
},
2492+
},
2493+
2494+
'Replica trigger on partition is altered' => {
24692495
regexp => qr/^
2470-
\QCREATE TRIGGER test_trigger AFTER INSERT ON dump_test_second_schema.measurement\E
2496+
\QALTER TABLE dump_test_second_schema.measurement_y2006m4 ENABLE REPLICA TRIGGER test_trigger;\E
24712497
/xm,
2472-
like => {},
2498+
like => {
2499+
%full_runs,
2500+
section_post_data => 1,
2501+
role => 1,
2502+
binary_upgrade => 1,
2503+
},
2504+
},
2505+
2506+
'Always trigger on partition is altered' => {
2507+
regexp => qr/^
2508+
\QALTER TABLE dump_test_second_schema.measurement_y2006m5 ENABLE ALWAYS TRIGGER test_trigger;\E
2509+
/xm,
2510+
like => {
2511+
%full_runs,
2512+
section_post_data => 1,
2513+
role => 1,
2514+
binary_upgrade => 1,
2515+
},
2516+
},
2517+
2518+
# We should never see the creation of a trigger on a partition
2519+
'Disabled trigger on partition is not created' => {
2520+
regexp => qr/CREATE TRIGGER test_trigger.*ON dump_test_second_schema/,
2521+
like => {},
2522+
unlike => { %full_runs, %dump_test_schema_runs },
2523+
},
2524+
2525+
# Triggers on partitions should not be dropped individually
2526+
'Triggers on partitions are not dropped' => {
2527+
regexp => qr/DROP TRIGGER test_trigger.*ON dump_test_second_schema/,
2528+
like => {}
24732529
},
24742530
24752531
'CREATE TABLE test_fourth_table_zero_col' => {
@@ -3122,9 +3178,12 @@
31223178
},
31233179
31243180
'GRANT SELECT ON TABLE measurement_y2006m2' => {
3125-
create_order => 92,
3126-
create_sql => 'GRANT SELECT ON
3127-
TABLE dump_test_second_schema.measurement_y2006m2
3181+
create_order => 94,
3182+
create_sql => 'GRANT SELECT ON TABLE
3183+
dump_test_second_schema.measurement_y2006m2,
3184+
dump_test_second_schema.measurement_y2006m3,
3185+
dump_test_second_schema.measurement_y2006m4,
3186+
dump_test_second_schema.measurement_y2006m5
31283187
TO regress_dump_test_role;',
31293188
regexp =>
31303189
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
@@ -213,6 +213,8 @@ tmp|f
213213
trigger_parted|t
214214
trigger_parted_p1|t
215215
trigger_parted_p1_1|t
216+
trigger_parted_p2|t
217+
trigger_parted_p2_2|t
216218
varchar_tbl|f
217219
view_base_table|t
218220
-- restore normal output mode

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3346,6 +3346,11 @@ create trigger aft_row after insert or update on trigger_parted
33463346
create table trigger_parted_p1 partition of trigger_parted for values in (1)
33473347
partition by list (a);
33483348
create table trigger_parted_p1_1 partition of trigger_parted_p1 for values in (1);
3349+
create table trigger_parted_p2 partition of trigger_parted for values in (2)
3350+
partition by list (a);
3351+
create table trigger_parted_p2_2 partition of trigger_parted_p2 for values in (2);
3352+
alter table only trigger_parted_p2 disable trigger aft_row;
3353+
alter table trigger_parted_p2_2 enable always trigger aft_row;
33493354
-- verify transition table conversion slot's lifetime
33503355
-- https://postgr.es/m/39a71864-b120-5a5c-8cc5-c632b6f16761@amazon.com
33513356
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
@@ -2502,6 +2502,11 @@ create trigger aft_row after insert or update on trigger_parted
25022502
create table trigger_parted_p1 partition of trigger_parted for values in (1)
25032503
partition by list (a);
25042504
create table trigger_parted_p1_1 partition of trigger_parted_p1 for values in (1);
2505+
create table trigger_parted_p2 partition of trigger_parted for values in (2)
2506+
partition by list (a);
2507+
create table trigger_parted_p2_2 partition of trigger_parted_p2 for values in (2);
2508+
alter table only trigger_parted_p2 disable trigger aft_row;
2509+
alter table trigger_parted_p2_2 enable always trigger aft_row;
25052510

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

0 commit comments

Comments
 (0)