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 b9b408c48724 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

diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index f8d99b772219555759af05746d432852a82a3ca9..fc2fc5891e20366dc9b06b5580ec65545696afdc 100644 (file)
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -7816,6 +7816,7 @@ getTriggers(Archive *fout, TableInfo tblinfo[], int numTables)
                                i_tgconstrrelid,
                                i_tgconstrrelname,
                                i_tgenabled,
+                               i_tgisinternal,
                                i_tgdeferrable,
                                i_tginitdeferred,
                                i_tgdef;
@@ -7834,18 +7835,63 @@ 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 tgisinternal 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: We need to see tgisinternal 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.
+                        *
+                        * See above about pretty=true in pg_get_triggerdef.
+                        */
+                       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)
+               {
+                       /* See above about pretty=true in pg_get_triggerdef */
+                       appendPQExpBuffer(query,
+                                                         "SELECT t.tgname, "
+                                                         "t.tgfoid::pg_catalog.regproc AS tgfname, "
+                                                         "pg_catalog.pg_get_triggerdef(t.oid, false) AS tgdef, "
+                                                         "t.tgenabled, false as tgisinternal, "
+                                                         "t.tableoid, t.oid "
                                                          "FROM pg_catalog.pg_trigger t "
                                                          "WHERE tgrelid = '%u'::pg_catalog.oid "
                                                          "AND NOT tgisinternal",
@@ -7860,6 +7906,7 @@ getTriggers(Archive *fout, TableInfo tblinfo[], int numTables)
                                                          "SELECT tgname, "
                                                          "tgfoid::pg_catalog.regproc AS tgfname, "
                                                          "tgtype, tgnargs, tgargs, tgenabled, "
+                                                         "false as tgisinternal, "
                                                          "tgisconstraint, tgconstrname, tgdeferrable, "
                                                          "tgconstrrelid, tginitdeferred, tableoid, oid, "
                                                          "tgconstrrelid::pg_catalog.regclass AS tgconstrrelname "
@@ -7908,6 +7955,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");
@@ -7927,6 +7975,7 @@ getTriggers(Archive *fout, TableInfo tblinfo[], int numTables)
                        tginfo[j].dobj.namespace = tbinfo->dobj.namespace;
                        tginfo[j].tgtable = tbinfo;
                        tginfo[j].tgenabled = *(PQgetvalue(res, j, i_tgenabled));
+                       tginfo[j].tgisinternal = *(PQgetvalue(res, j, i_tgisinternal)) == 't';
                        if (i_tgdef >= 0)
                        {
                                tginfo[j].tgdef = pg_strdup(PQgetvalue(res, j, i_tgdef));
@@ -17681,7 +17730,40 @@ dumpTrigger(Archive *fout, 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.  (Clear out the
+                * DROP query too, so that pg_dump --create does not cause errors.)
+                */
+               resetPQExpBuffer(query);
+               resetPQExpBuffer(delqry);
+               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 b995f5596241706662fcfff6a8cbff5aec38b03a..0224911b502fe1ff184a1bfce6a5c1110e80926c 100644 (file)
--- a/src/bin/pg_dump/pg_dump.h
+++ b/src/bin/pg_dump/pg_dump.h
@@ -413,6 +413,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 7014bbd836d3643196d32eb2cd03f2a06b55c553..5d41ee8ed3a26fcbf3ba0979cd8e6b7b358b8677 100644 (file)
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -2400,12 +2400,68 @@ my %tests = (
                },
        },
 
-       # this shouldn't ever get emitted
-       'Creation of row-level trigger in partition' => {
+       'Disabled trigger on partition is altered' => {
+               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;
+                                               CREATE TABLE dump_test_second_schema.measurement_y2006m4
+                                               PARTITION OF dump_test.measurement
+                                               FOR VALUES FROM (\'2006-04-01\') TO (\'2006-05-01\');
+                                               ALTER TABLE dump_test_second_schema.measurement_y2006m4 ENABLE REPLICA TRIGGER test_trigger;
+                                               CREATE TABLE dump_test_second_schema.measurement_y2006m5
+                                               PARTITION OF dump_test.measurement
+                                               FOR VALUES FROM (\'2006-05-01\') TO (\'2006-06-01\');
+                                               ALTER TABLE dump_test_second_schema.measurement_y2006m5 ENABLE ALWAYS 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,
+               },
+       },
+
+       'Replica trigger on partition is altered' => {
                regexp => qr/^
-                       \QCREATE TRIGGER test_trigger AFTER INSERT ON dump_test_second_schema.measurement\E
+                       \QALTER TABLE dump_test_second_schema.measurement_y2006m4 ENABLE REPLICA TRIGGER test_trigger;\E
                        /xm,
-               like => {},
+               like => {
+                       %full_runs,
+                       section_post_data => 1,
+                       role              => 1,
+                       binary_upgrade    => 1,
+               },
+       },
+
+       'Always trigger on partition is altered' => {
+               regexp => qr/^
+                       \QALTER TABLE dump_test_second_schema.measurement_y2006m5 ENABLE ALWAYS TRIGGER test_trigger;\E
+                       /xm,
+               like => {
+                       %full_runs,
+                       section_post_data => 1,
+                       role              => 1,
+                       binary_upgrade    => 1,
+               },
+       },
+
+       # We should never see the creation of a trigger on a partition
+       'Disabled trigger on partition is not created' => {
+               regexp => qr/CREATE TRIGGER test_trigger.*ON dump_test_second_schema/,
+               like   => {},
+               unlike => { %full_runs, %dump_test_schema_runs },
+       },
+
+       # Triggers on partitions should not be dropped individually
+       'Triggers on partitions are not dropped' => {
+               regexp => qr/DROP TRIGGER test_trigger.*ON dump_test_second_schema/,
+               like   => {}
        },
 
        'CREATE TABLE test_fourth_table_zero_col' => {
@@ -3044,9 +3100,12 @@ my %tests = (
        },
 
        'GRANT SELECT ON TABLE measurement_y2006m2' => {
-               create_order => 92,
-               create_sql   => 'GRANT SELECT ON
-                                                  TABLE dump_test_second_schema.measurement_y2006m2
+               create_order => 94,
+               create_sql   => 'GRANT SELECT ON TABLE
+                                                  dump_test_second_schema.measurement_y2006m2,
+                                                  dump_test_second_schema.measurement_y2006m3,
+                                                  dump_test_second_schema.measurement_y2006m4,
+                                                  dump_test_second_schema.measurement_y2006m5
                                                   TO regress_dump_test_role;',
                regexp =>
                  qr/^\QGRANT SELECT ON TABLE dump_test_second_schema.measurement_y2006m2 TO regress_dump_test_role;\E/m,

diff --git a/src/test/regress/expected/sanity_check.out b/src/test/regress/expected/sanity_check.out
index 192445878d21d823a02e8ef7e7a4d5f702c1cb99..1030a03a578795b44dbddb3b6e718b0e0d0288ad 100644 (file)
--- a/src/test/regress/expected/sanity_check.out
+++ b/src/test/regress/expected/sanity_check.out
@@ -211,6 +211,8 @@ tmp|f
 trigger_parted|t
 trigger_parted_p1|t
 trigger_parted_p1_1|t
+trigger_parted_p2|t
+trigger_parted_p2_2|t
 varchar_tbl|f
 view_base_table|t
 -- restore normal output mode

diff --git a/src/test/regress/expected/triggers.out b/src/test/regress/expected/triggers.out
index 5dbb0a0aa29e77c427fad3b91d773ee666556489..207e0d377957bce236ab6f712285100748db9ea4 100644 (file)
--- a/src/test/regress/expected/triggers.out
+++ b/src/test/regress/expected/triggers.out
@@ -3257,6 +3257,11 @@ create trigger aft_row after insert or update on trigger_parted
 create table trigger_parted_p1 partition of trigger_parted for values in (1)
   partition by list (a);
 create table trigger_parted_p1_1 partition of trigger_parted_p1 for values in (1);
+create table trigger_parted_p2 partition of trigger_parted for values in (2)
+  partition by list (a);
+create table trigger_parted_p2_2 partition of trigger_parted_p2 for values in (2);
+alter table only trigger_parted_p2 disable trigger aft_row;
+alter table trigger_parted_p2_2 enable always trigger aft_row;
 -- verify transition table conversion slot's lifetime
 -- https://postgr.es/m/39a71864-b120-5a5c-8cc5-c632b6f16761@amazon.com
 create table convslot_test_parent (col1 text primary key);

diff --git a/src/test/regress/sql/triggers.sql b/src/test/regress/sql/triggers.sql
index f8e23f034b49e053d581f5241671b6b62f1d2e1e..c1917ca3c5843587f8a56429ed53287786582439 100644 (file)
--- a/src/test/regress/sql/triggers.sql
+++ b/src/test/regress/sql/triggers.sql
@@ -2415,6 +2415,11 @@ create trigger aft_row after insert or update on trigger_parted
 create table trigger_parted_p1 partition of trigger_parted for values in (1)
   partition by list (a);
 create table trigger_parted_p1_1 partition of trigger_parted_p1 for values in (1);
+create table trigger_parted_p2 partition of trigger_parted for values in (2)
+  partition by list (a);
+create table trigger_parted_p2_2 partition of trigger_parted_p2 for values in (2);
+alter table only trigger_parted_p2 disable trigger aft_row;
+alter table trigger_parted_p2_2 enable always trigger aft_row;
 
 -- verify transition table conversion slot's lifetime
 -- https://postgr.es/m/39a71864-b120-5a5c-8cc5-c632b6f16761@amazon.com