From 0730f32f7608310c1cc689fa587dd439e66fc6b7 Mon Sep 17 00:00:00 2001 From: Alvaro Herrera Date: Wed, 14 Jul 2021 13:40:02 -0400 Subject: [PATCH v3 2/2] Fix pg_dump for triggers with changed enabled flag --- src/bin/pg_dump/pg_dump.c | 95 +++++++++++++++++++++++++++++--- src/bin/pg_dump/pg_dump.h | 1 + src/bin/pg_dump/t/002_pg_dump.pl | 26 +++++++-- 3 files changed, 111 insertions(+), 11 deletions(-) diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index 912144c43e..c1f8f3ec9a 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -7998,6 +7998,7 @@ getTriggers(Archive *fout, TableInfo tblinfo[], int numTables) i_tgconstrrelid, i_tgconstrrelname, i_tgenabled, + i_tgisinternal, i_tgdeferrable, i_tginitdeferred, i_tgdef; @@ -8016,18 +8017,62 @@ getTriggers(Archive *fout, TableInfo tblinfo[], int numTables) tbinfo->dobj.name); resetPQExpBuffer(query); - if (fout->remoteVersion >= 90000) + if (fout->remoteVersion >= 130000) { /* * NB: think not to use pretty=true in pg_get_triggerdef. It * could result in non-forward-compatible dumps of WHEN clauses * due to under-parenthesization. + * + * NB: We need to see tginternal triggers in partitions, in case + * the tgenabled flag has been changed from the parent. */ appendPQExpBuffer(query, - "SELECT tgname, " - "tgfoid::pg_catalog.regproc AS tgfname, " - "pg_catalog.pg_get_triggerdef(oid, false) AS tgdef, " - "tgenabled, tableoid, oid " + "SELECT t.tgname, " + "t.tgfoid::pg_catalog.regproc AS tgfname, " + "pg_catalog.pg_get_triggerdef(t.oid, false) AS tgdef, " + "t.tgenabled, t.tableoid, t.oid, t.tgisinternal " + "FROM pg_catalog.pg_trigger t " + "LEFT JOIN pg_catalog.pg_trigger u ON u.oid = t.tgparentid " + "WHERE t.tgrelid = '%u'::pg_catalog.oid " + "AND (NOT t.tgisinternal OR t.tgenabled != u.tgenabled)", + tbinfo->dobj.catId.oid); + } + else if (fout->remoteVersion >= 110000) + { + /* + * NB: think not to use pretty=true in pg_get_triggerdef. It + * could result in non-forward-compatible dumps of WHEN clauses + * due to under-parenthesization. + * + * NB: We need to see tginternal triggers in partitions, in case + * the tgenabled flag has been changed from the parent. No + * tgparentid in version 11-12, so we have to match them via + * pg_depend. + */ + appendPQExpBuffer(query, + "SELECT t.tgname, " + "t.tgfoid::pg_catalog.regproc AS tgfname, " + "pg_catalog.pg_get_triggerdef(t.oid, false) AS tgdef, " + "t.tgenabled, t.tableoid, t.oid, t.tgisinternal " + "FROM pg_catalog.pg_trigger t " + "LEFT JOIN pg_catalog.pg_depend AS d ON " + " d.classid = 'pg_catalog.pg_trigger'::pg_catalog.regclass AND " + " d.refclassid = 'pg_catalog.pg_trigger'::pg_catalog.regclass AND d.objid = t.oid " + "LEFT JOIN pg_catalog.pg_trigger AS pt ON pt.oid = refobjid " + "WHERE t.tgrelid = '%u'::pg_catalog.oid " + "AND (NOT t.tgisinternal%s)", + tbinfo->dobj.catId.oid, + tbinfo->ispartition ? + " OR t.tgenabled != pt.tgenabled" : ""); + } + else if (fout->remoteVersion >= 90000) + { + appendPQExpBuffer(query, + "SELECT t.tgname, " + "t.tgfoid::pg_catalog.regproc AS tgfname, " + "pg_catalog.pg_get_triggerdef(t.oid, false) AS tgdef, " + "t.tgenabled, t.tableoid, t.oid, t.tgisinternal " "FROM pg_catalog.pg_trigger t " "WHERE tgrelid = '%u'::pg_catalog.oid " "AND NOT tgisinternal", @@ -8090,6 +8135,7 @@ getTriggers(Archive *fout, TableInfo tblinfo[], int numTables) i_tgconstrrelid = PQfnumber(res, "tgconstrrelid"); i_tgconstrrelname = PQfnumber(res, "tgconstrrelname"); i_tgenabled = PQfnumber(res, "tgenabled"); + i_tgisinternal = PQfnumber(res, "tgisinternal"); i_tgdeferrable = PQfnumber(res, "tgdeferrable"); i_tginitdeferred = PQfnumber(res, "tginitdeferred"); i_tgdef = PQfnumber(res, "tgdef"); @@ -8101,14 +8147,18 @@ getTriggers(Archive *fout, TableInfo tblinfo[], int numTables) for (j = 0; j < ntups; j++) { + Oid trigoid = atooid(PQgetvalue(res, j, i_oid)); + bool tgisinternal = *(PQgetvalue(res, j, i_tgisinternal)) == 't'; + tginfo[j].dobj.objType = DO_TRIGGER; tginfo[j].dobj.catId.tableoid = atooid(PQgetvalue(res, j, i_tableoid)); - tginfo[j].dobj.catId.oid = atooid(PQgetvalue(res, j, i_oid)); + tginfo[j].dobj.catId.oid = trigoid; AssignDumpId(&tginfo[j].dobj); tginfo[j].dobj.name = pg_strdup(PQgetvalue(res, j, i_tgname)); tginfo[j].dobj.namespace = tbinfo->dobj.namespace; tginfo[j].tgtable = tbinfo; tginfo[j].tgenabled = *(PQgetvalue(res, j, i_tgenabled)); + tginfo[j].tgisinternal = tgisinternal; if (i_tgdef >= 0) { tginfo[j].tgdef = pg_strdup(PQgetvalue(res, j, i_tgdef)); @@ -17799,7 +17849,38 @@ dumpTrigger(Archive *fout, const TriggerInfo *tginfo) "pg_catalog.pg_trigger", "TRIGGER", trigidentity->data); - if (tginfo->tgenabled != 't' && tginfo->tgenabled != 'O') + if (tginfo->tgisinternal) + { + /* + * Triggers marked internal only appear here because their 'tgenabled' + * flag differs from its parent's. The trigger is created already, + * so remove the CREATE and replace it with an ALTER. + */ + resetPQExpBuffer(query); + appendPQExpBuffer(query, "\nALTER %sTABLE %s ", + tbinfo->relkind == RELKIND_FOREIGN_TABLE ? "FOREIGN " : "", + fmtQualifiedDumpable(tbinfo)); + switch (tginfo->tgenabled) + { + case 'f': + case 'D': + appendPQExpBufferStr(query, "DISABLE"); + break; + case 't': + case 'O': + appendPQExpBufferStr(query, "ENABLE"); + break; + case 'R': + appendPQExpBufferStr(query, "ENABLE REPLICA"); + break; + case 'A': + appendPQExpBufferStr(query, "ENABLE ALWAYS"); + break; + } + appendPQExpBuffer(query, " TRIGGER %s;\n", + fmtId(tginfo->dobj.name)); + } + else if (tginfo->tgenabled != 't' && tginfo->tgenabled != 'O') { appendPQExpBuffer(query, "\nALTER %sTABLE %s ", tbinfo->relkind == RELKIND_FOREIGN_TABLE ? "FOREIGN " : "", diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h index efb8c30e71..f5e170e0db 100644 --- a/src/bin/pg_dump/pg_dump.h +++ b/src/bin/pg_dump/pg_dump.h @@ -425,6 +425,7 @@ typedef struct _triggerInfo Oid tgconstrrelid; char *tgconstrrelname; char tgenabled; + bool tgisinternal; bool tgdeferrable; bool tginitdeferred; char *tgdef; diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl index 448b1be26c..57f909f2c4 100644 --- a/src/bin/pg_dump/t/002_pg_dump.pl +++ b/src/bin/pg_dump/t/002_pg_dump.pl @@ -2519,10 +2519,28 @@ my %tests = ( }, }, - # this shouldn't ever get emitted + 'Partition measurement_y2006m3 creation - disabled child trigger' => { + create_order => 93, + create_sql => + 'CREATE TABLE dump_test_second_schema.measurement_y2006m3 + PARTITION OF dump_test.measurement + FOR VALUES FROM (\'2006-03-01\') TO (\'2006-04-01\'); + ALTER TABLE dump_test_second_schema.measurement_y2006m3 DISABLE TRIGGER test_trigger', + regexp => qr/^ + \QALTER TABLE dump_test_second_schema.measurement_y2006m3 DISABLE TRIGGER test_trigger;\E + /xm, + like => { + %full_runs, + section_post_data => 1, + role => 1, + binary_upgrade => 1, + }, + }, + + # this shouldn't get emitted except for triggers with altered disabled/enabled 'Creation of row-level trigger in partition' => { regexp => qr/^ - \QCREATE TRIGGER test_trigger AFTER INSERT ON dump_test_second_schema.measurement\E + \QCREATE TRIGGER test_trigger AFTER INSERT ON dump_test_second_schema.measurement_y2006m2\E /xm, like => {}, }, @@ -3177,9 +3195,9 @@ my %tests = ( }, 'GRANT SELECT ON TABLE measurement_y2006m2' => { - create_order => 92, + create_order => 94, create_sql => 'GRANT SELECT ON - TABLE dump_test_second_schema.measurement_y2006m2 + TABLE dump_test_second_schema.measurement_y2006m2, dump_test_second_schema.measurement_y2006m3 TO regress_dump_test_role;', regexp => qr/^\QGRANT SELECT ON TABLE dump_test_second_schema.measurement_y2006m2 TO regress_dump_test_role;\E/m, -- 2.30.2