8000 Fix pg_dump for disabled triggers on partitioned tables · postgrespro/postgres@f0e21f2 · GitHub
[go: up one dir, main page]
Skip to content

Commit f0e21f2

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 df80fa2 commit f0e21f2

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
@@ -7998,6 +7998,7 @@ getTriggers(Archive *fout, TableInfo tblinfo[], int numTables)
79987998
i_tgconstrrelid,
79997999
i_tgconstrrelname,
80008000
i_tgenabled,
8001+
i_tgisinternal,
80018002
i_tgdeferrable,
80028003
i_tginitdeferred,
80038004
i_tgdef;
@@ -8016,18 +8017,63 @@ getTriggers(Archive *fout, TableInfo tblinfo[], int numTables)
80168017
tbinfo->dobj.name);
80178018

80188019
resetPQExpBuffer(query);
8019-
if (fout->remoteVersion >= 90000)
8020+
if (fout->remoteVersion >= 130000)
80208021
{
80218022
/*
80228023
* NB: think not to use pretty=true in pg_get_triggerdef. It
80238024
* could result in non-forward-compatible dumps of WHEN clauses
80248025
* due to under-parenthesization.
8026+
*
8027+
* NB: We need to see tgisinternal triggers in partitions, in case
8028+
* the tgenabled flag has been changed from the parent.
80258029
*/
80268030
appendPQExpBuffer(query,
8027-
"SELECT tgname, "
8028-
"tgfoid::pg_catalog.regproc AS tgfname, "
8029-
"pg_catalog.pg_get_triggerdef(oid, false) AS tgdef, "
8030-
"tgenabled, tableoid, oid "
8031+
"SELECT t.tgname, "
8032+
"t.tgfoid::pg_catalog.regproc AS tgfname, "
8033+
"pg_catalog.pg_get_triggerdef(t.oid, false) AS tgdef, "
8034+
"t.tgenabled, t.tableoid, t.oid, t.tgisinternal "
8035+
"FROM pg_catalog.pg_trigger t "
8036+
"LEFT JOIN pg_catalog.pg_trigger u ON u.oid = t.tgparentid "
8037+
"WHERE t.tgrelid = '%u'::pg_catalog.oid "
8038+
"AND (NOT t.tgisinternal OR t.tgenabled != u.tgenabled)",
8039+
tbinfo->dobj.catId.oid);
8040+
}
8041+
else if (fout->remoteVersion >= 110000)
8042+
{
8043+
/*
8044+
* NB: We need to see tgisinternal triggers in partitions, in case
8045+
* the tgenabled flag has been changed from the parent. No
8046+
* tgparentid in version 11-12, so we have to match them via
8047+
* pg_depend.
8048+
*
8049+
* See above about pretty=true in pg_get_triggerdef.
8050+
*/
8051+
appendPQExpBuffer(query,
8052+
"SELECT t.tgname, "
8053+
"t.tgfoid::pg_catalog.regproc AS tgfname, "
8054+
"pg_catalog.pg_get_triggerdef(t.oid, false) AS tgdef, "
8055+
"t.tgenabled, t.tableoid, t.oid, t.tgisinternal "
8056+
"FROM pg_catalog.pg_trigger t "
8057+
"LEFT JOIN pg_catalog.pg_depend AS d ON "
8058+
" d.classid = 'pg_catalog.pg_trigger'::pg_catalog.regclass AND "
8059+
" d.refclassid = 'pg_catalog.pg_trigger'::pg_catalog.regclass AND "
8060+
" d.objid = t.oid "
8061+
"LEFT JOIN pg_catalog.pg_trigger AS pt ON pt.oid = refobjid "
8062+
"WHERE t.tgrelid = '%u'::pg_catalog.oid "
8063+
"AND (NOT t.tgisinternal%s)",
8064+
tbinfo->dobj.catId.oid,
8065+
tbinfo->ispartition ?
8066+
" OR t.tgenabled != pt.tgenabled" : "");
8067+
}
8068+
else if (fout->remoteVersion >= 90000)
8069+
{
8070+
/* See above about pretty=true in pg_get_triggerdef */
8071+
appendPQExpBuffer(query,
8072+
"SELECT t.tgname, "
8073+
"t.tgfoid::pg_catalog.regproc AS tgfname, "
8074+
"pg_catalog.pg_get_triggerdef(t.oid, false) AS tgdef, "
8075+
"t.tgenabled, false as tgisinternal, "
8076+
"t.tableoid, t.oid "
80318077
"FROM pg_catalog.pg_trigger t "
80328078
"WHERE tgrelid = '%u'::pg_catalog.oid "
80338079
"AND NOT tgisinternal",
@@ -8042,6 +8088,7 @@ getTriggers(Archive *fout, TableInfo tblinfo[], int numTables)
80428088
"SELECT tgname, "
80438089
"tgfoid::pg_catalog.regproc AS tgfname, "
80448090
"tgtype, tgnargs, tgargs, tgenabled, "
8091+
"false as tgisinternal, "
80458092
"tgisconstraint, tgconstrname, tgdeferrable, "
80468093
"tgconstrrelid, tginitdeferred, tableoid, oid, "
80478094
"tgconstrrelid::pg_catalog.regclass AS tgconstrrelname "
@@ -8090,6 +8137,7 @@ getTriggers(Archive *fout, TableInfo tblinfo[], int numTables)
80908137
i_tgconstrrelid = PQfnumber(res, "tgconstrrelid");
80918138
i_tgconstrrelname = PQfnumber(res, "tgconstrrelname");
80928139
i_tgenabled = PQfnumber(res, "tgenabled");
8140+
i_tgisinternal = PQfnumber(res, "tgisinternal");
80938141
i_tgdeferrable = PQfnumber(res, "tgdeferrable");
80948142
i_tginitdeferred = PQfnumber(res, "tginitdeferred");
80958143
i_tgdef = PQfnumber(res, "tgdef");
@@ -8109,6 +8157,7 @@ getTriggers(Archive *fout, TableInfo tblinfo[], int numTables)
81098157
tginfo[j].dobj.namespace = tbinfo->dobj.namespace;
81108158
tginfo[j].tgtable = tbinfo;
81118159
tginfo[j].tgenabled = *(PQgetvalue(res, j, i_tgenabled));
8160+
tginfo[j].tgisinternal = *(PQgetvalue(res, j, i_tgisinternal)) == 't';
81128161
if (i_tgdef >= 0)
81138162
{
81148163
tginfo[j].tgdef = pg_strdup(PQgetvalue(res, j, i_tgdef));
@@ -17799,7 +17848,40 @@ dumpTrigger(Archive *fout, const TriggerInfo *tginfo)
1779917848
"pg_catalog.pg_trigger", "TRIGGER",
1780017849
trigidentity->data);
1780117850

17802-
if (tginfo->tgenabled != 't' && tginfo->tgenabled != 'O')
17851+
if (tginfo->tgisinternal)
17852+
{
17853+
/*
17854+
* Triggers marked internal only appear here because their 'tgenabled'
17855+
* flag differs from its parent's. The trigger is created already, so
17856+
* remove the CREATE and replace it with an ALTER. (Clear out the
17857+
* DROP query too, so that pg_dump --create does not cause errors.)
17858+
*/
17859+
resetPQExpBuffer(query);
17860+
resetPQExpBuffer(delqry);
17861+
appendPQExpBuffer(query, "\nALTER %sTABLE %s ",
17862+
tbinfo->relkind == RELKIND_FOREIGN_TABLE ? "FOREIGN " : "",
17863+
fmtQualifiedDumpable(tbinfo));
17864+
switch (tginfo->tgenabled)
17865+
{
17866+
case 'f':
17867+
case 'D':
17868+
appendPQExpBufferStr(query, "DISABLE");
17869+
break;
17870+
case 't':
17871+
case 'O':
17872+
appendPQExpBufferStr(query, "ENABLE");
17873+
break;
17874+
case 'R':
17875+
appendPQExpBufferStr(query, "ENABLE REPLICA");
17876+
break;
17877+
case 'A':
17878+
appendPQExpBufferStr(query, "ENABLE ALWAYS");
17879+
break;
17880+
}
17881+
appendPQExpBuffer(query, " TRIGGER %s;\n",
17882+
fmtId(tginfo->dobj.name));
17883+
}
17884+
else if (tginfo->tgenabled != 't' && tginfo->tgenabled != 'O')
1780317885
{
1780417886
appendPQExpBuffer(query, "\nALTER %sTABLE %s ",
1780517887
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
@@ -425,6 +425,7 @@ typedef struct _triggerInfo
425425
Oid tgconstrrelid;
426426
char *tgconstrrelname;
427427
char tgenabled;
428+
bool tgisinternal;
428429
bool tgdeferrable;
429430
bool tginitdeferred;
430431
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
@@ -2519,12 +2519,68 @@
25192519
},
25202520
},
25212521
2522-
# this shouldn't ever get emitted
2523-
'Creation of row-level trigger in partition' => {
2522+
'Disabled trigger on partition is altered' => {
2523+
create_order => 93,
2524+
create_sql =>
2525+
'CREATE TABLE dump_test_second_schema.measurement_y2006m3
2526+
PARTITION OF dump_test.measurement
2527+
FOR VALUES FROM (\'2006-03-01\') TO (\'2006-04-01\');
2528+
ALTER TABLE dump_test_second_schema.measurement_y2006m3 DISABLE TRIGGER test_trigger;
2529+
CREATE TABLE dump_test_second_schema.measurement_y2006m4
2530+
PARTITION OF dump_test.measurement
2531+
FOR VALUES FROM (\'2006-04-01\') TO (\'2006-05-01\');
2532+
ALTER TABLE dump_test_second_schema.measurement_y2006m4 ENABLE REPLICA TRIGGER test_trigger;
2533+
CREATE TABLE dump_test_second_schema.measurement_y2006m5
2534+
PARTITION OF dump_test.measurement
2535+
FOR VALUES FROM (\'2006-05-01\') TO (\'2006-06-01\');
2536+
ALTER TABLE dump_test_second_schema.measurement_y2006m5 ENABLE ALWAYS TRIGGER test_trigger;
2537+
',
2538+
regexp => qr/^
2539+
\QALTER TABLE dump_test_second_schema.measurement_y2006m3 DISABLE TRIGGER test_trigger;\E
2540+
/xm,
2541+
like => {
2542+
%full_runs,
2543+
section_post_data => 1,
2544+
role => 1,
2545+
binary_upgrade => 1,
2546+
},
2547+
},
2548+
2549+
'Replica trigger on partition is altered' => {
25242550
regexp => qr/^
2525-
\QCREATE TRIGGER test_trigger AFTER INSERT ON dump_test_second_schema.measurement\E
2551+
\QALTER TABLE dump_test_second_schema.measurement_y2006m4 ENABLE REPLICA TRIGGER test_trigger;\E
25262552
/xm,
2527-
like => {},
2553+
like => {
2554+
%full_runs,
2555+
section_post_data => 1,
2556+
role => 1,
2557+
binary_upgrade => 1,
2558+
},
2559+
},
2560+
2561+
'Always trigger on partition is altered' => {
2562+
regexp => qr/^
2563+
\QALTER TABLE dump_test_second_schema.measurement_y2006m5 ENABLE ALWAYS TRIGGER test_trigger;\E
2564+
/xm,
2565+
like => {
2566+
%full_runs,
2567+
section_post_data => 1,
2568+
role => 1,
2569+
binary_upgrade => 1,
2570+
},
2571+
},
2572+
2573+
# We should never see the creation of a trigger on a partition
2574+
'Disabled trigger on partition is not created' => {
2575+
regexp => qr/CREATE TRIGGER test_trigger.*ON dump_test_second_schema/,
2576+
like => {},
2577+
unlike => { %full_runs, %dump_test_schema_runs },
2578+
},
2579+
2580+
# Triggers on partitions should not be dropped individually
2581+
'Triggers on partitions are not dropped' => {
2582+
regexp => qr/DROP TRIGGER test_trigger.*ON dump_test_second_schema/,
2583+
like => {}
25282584
},
25292585
25302586
'CREATE TABLE test_fourth_table_zero_col' => {
@@ -3177,9 +3233,12 @@
31773233
},
31783234
31793235
'GRANT SELECT ON TABLE measurement_y2006m2' => {
3180-
create_order => 92,
3181-
create_sql => 'GRANT SELECT ON
3182-
TABLE dump_test_second_schema.measurement_y2006m2
3236+
create_order => 94,
3237+
create_sql => 'GRANT SELECT ON TABLE
3238+
dump_test_second_schema.measurement_y2006m2,
3239+
dump_test_second_schema.measurement_y2006m3,
3240+
dump_test_second_schema.measurement_y2006m4,
3241+
dump_test_second_schema.measurement_y2006m5
31833242
TO regress_dump_test_role;',
31843243
regexp =>
31853244
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)
0