Fix pg_dump's failure to honor dependencies of SQL functions.

A new-style SQL function can contain a parse-time dependency
on a unique index, much as views and matviews can (such cases
arise from GROUP BY and ON CONFLICT clauses, for example).
To dump and restore such a function successfully, pg_dump must
postpone the function until after the unique index is created,
which will happen in the post-data part of the dump.  Therefore
we have to remove the normal constraint that functions are
dumped in pre-data.  Add code similar to the existing logic
that handles this for matviews.  I added test cases for both
as well, since code coverage tests showed that we weren't
testing the matview logic.

Per report from Sami Imseih.  Back- to v14 where
new-style SQL functions came in.

Discussion: https://postgr.es/m/2C1933AB-C2F8-499B-9D18-4AC1882256A0@amazon.com

diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 3af97a6039eeaaa6332b29736daf8769abe061a8..5dab1ba9eae6e055ddcdcd26504920d8ded06c0f 100644 (file)
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -6085,6 +6085,7 @@ getAggregates(Archive *fout, int *numAggs)
                          agginfo[i].aggfn.argtypes,
                          agginfo[i].aggfn.nargs);
        }
+       agginfo[i].aggfn.postponed_def = false; /* might get set during sort */
 
        /* Decide whether we want to dump it */
        selectDumpableObject(&(agginfo[i].aggfn.dobj), fout);
@@ -6283,6 +6284,7 @@ getFuncs(Archive *fout, int *numFuncs)
            parseOidArray(PQgetvalue(res, i, i_proargtypes),
                          finfo[i].argtypes, finfo[i].nargs);
        }
+       finfo[i].postponed_def = false; /* might get set during sort */
 
        /* Decide whether we want to dump it */
        selectDumpableObject(&(finfo[i].dobj), fout);
@@ -12168,7 +12170,8 @@ dumpFunc(Archive *fout, const FuncInfo *finfo)
                                  .namespace = finfo->dobj.namespace->dobj.name,
                                  .owner = finfo->rolname,
                                  .description = keyword,
-                                 .section = SECTION_PRE_DATA,
+                                 .section = finfo->postponed_def ?
+                                 SECTION_POST_DATA : SECTION_PRE_DATA,
                                  .createStmt = q->data,
                                  .dropStmt = delqry->data));
 

diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h
index ed6ce41ad7ecc0ef2b3e5ca7d0b51b729cb6080e..bc8f2ec36db30e0d3502d748d134903f43c60b44 100644 (file)
--- a/src/bin/pg_dump/pg_dump.h
+++ b/src/bin/pg_dump/pg_dump.h
@@ -227,6 +227,7 @@ typedef struct _funcInfo
    int         nargs;
    Oid        *argtypes;
    Oid         prorettype;
+   bool        postponed_def;  /* function must be postponed into post-data */
 } FuncInfo;
 
 /* AggInfo is a superset of FuncInfo */

diff --git a/src/bin/pg_dump/pg_dump_sort.c b/src/bin/pg_dump/pg_dump_sort.c
index e503eaaa26b0c64f9c272cda3426638f3b2bf3e6..523a19c1557d04889a0fc76abf77594bdc365ac4 100644 (file)
--- a/src/bin/pg_dump/pg_dump_sort.c
+++ b/src/bin/pg_dump/pg_dump_sort.c
@@ -868,6 +868,28 @@ repairMatViewBoundaryMultiLoop(DumpableObject *boundaryobj,
    }
 }
 
+/*
+ * If a function is involved in a multi-object loop, we can't currently fix
+ * that by splitting it into two DumpableObjects.  As a stopgap, we try to fix
+ * it by dropping the constraint that the function be dumped in the pre-data
+ * section.  This is sufficient to handle cases where a function depends on
+ * some unique index, as can happen if it has a GROUP BY for example.
+ */
+static void
+repairFunctionBoundaryMultiLoop(DumpableObject *boundaryobj,
+                               DumpableObject *nextobj)
+{
+   /* remove boundary's dependency on object after it in loop */
+   removeObjectDependency(boundaryobj, nextobj->dumpId);
+   /* if that object is a function, mark it as postponed into post-data */
+   if (nextobj->objType == DO_FUNC)
+   {
+       FuncInfo   *nextinfo = (FuncInfo *) nextobj;
+
+       nextinfo->postponed_def = true;
+   }
+}
+
 /*
  * Because we make tables depend on their CHECK constraints, while there
  * will be an automatic dependency in the other direction, we need to break
@@ -1062,6 +1084,28 @@ repairDependencyLoop(DumpableObject **loop,
        }
    }
 
+   /* Indirect loop involving function and data boundary */
+   if (nLoop > 2)
+   {
+       for (i = 0; i < nLoop; i++)
+       {
+           if (loop[i]->objType == DO_FUNC)
+           {
+               for (j = 0; j < nLoop; j++)
+               {
+                   if (loop[j]->objType == DO_PRE_DATA_BOUNDARY)
+                   {
+                       DumpableObject *nextobj;
+
+                       nextobj = (j < nLoop - 1) ? loop[j + 1] : loop[0];
+                       repairFunctionBoundaryMultiLoop(loop[j], nextobj);
+                       return;
+                   }
+               }
+           }
+       }
+   }
+
    /* Table and CHECK constraint */
    if (nLoop == 2 &&
        loop[0]->objType == DO_TABLE &&

diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index 387c5d3afbf4af30ab198acf2f84a4c3a06719c8..15852188c423894f3147d07e1be1a52b4f8faaa7 100644 (file)
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -2490,6 +2490,27 @@ my %tests = (
        },
    },
 
+   'Check ordering of a function that depends on a primary key' => {
+       create_order => 41,
+       create_sql => '
+           CREATE TABLE dump_test.ordering_table (id int primary key, data int);
+           CREATE FUNCTION dump_test.ordering_func ()
+           RETURNS SETOF dump_test.ordering_table
+           LANGUAGE sql BEGIN ATOMIC
+           SELECT * FROM dump_test.ordering_table GROUP BY id; END;',
+       regexp => qr/^
+           \QALTER TABLE ONLY dump_test.ordering_table\E
+           \n\s+\QADD CONSTRAINT ordering_table_pkey PRIMARY KEY (id);\E
+           .*^
+           \QCREATE FUNCTION dump_test.ordering_func\E/xms,
+       like =>
+         { %full_runs, %dump_test_schema_runs, section_post_data => 1, },
+       unlike => {
+           exclude_dump_test_schema => 1,
+           only_dump_measurement => 1,
+       },
+   },
+
    'CREATE PROCEDURE dump_test.ptest1' => {
        create_order => 41,
        create_sql => 'CREATE PROCEDURE dump_test.ptest1(a int)
@@ -2732,6 +2753,25 @@ my %tests = (
        },
    },
 
+   'Check ordering of a matview that depends on a primary key' => {
+       create_order => 42,
+       create_sql => '
+           CREATE MATERIALIZED VIEW dump_test.ordering_view AS
+               SELECT * FROM dump_test.ordering_table GROUP BY id;',
+       regexp => qr/^
+           \QALTER TABLE ONLY dump_test.ordering_table\E
+           \n\s+\QADD CONSTRAINT ordering_table_pkey PRIMARY KEY (id);\E
+           .*^
+           \QCREATE MATERIALIZED VIEW dump_test.ordering_view AS\E
+           \n\s+\QSELECT id,\E/xms,
+       like =>
+         { %full_runs, %dump_test_schema_runs, section_post_data => 1, },
+       unlike => {
+           exclude_dump_test_schema => 1,
+           only_dump_measurement => 1,
+       },
+   },
+
    'CREATE POLICY p1 ON test_table' => {
        create_order => 22,
        create_sql => 'CREATE POLICY p1 ON dump_test.test_table