"Imseih (AWS), Sami" <sims...@amazon.com> writes:
> With the attached repro, pg_restore fails with
> pg_restore: error: could not execute query: ERROR:  constraint "a_pkey" for 
> table "a" does not exist
> Command was: CREATE FUNCTION public.a_f(c1_in text, c2 integer DEFAULT 60) 
> RETURNS void

Hmph.  The other thing worth noticing is that pg_dump prints
a warning:

pg_dump: warning: could not resolve dependency loop among these items:

or with -v:

pg_dump: warning: could not resolve dependency loop among these items:
pg_dump:   FUNCTION a_f  (ID 218 OID 40664)
pg_dump:   CONSTRAINT a_pkey  (ID 4131 OID 40663)
pg_dump:   POST-DATA BOUNDARY  (ID 4281)
pg_dump:   TABLE DATA a  (ID 4278 OID 40657)
pg_dump:   PRE-DATA BOUNDARY  (ID 4280)

So it's lacking a rule to tell it what to do in this case, and the
default is the wrong way around.  I think we need to fix it in
about the same way as the equivalent case for matviews, which
leads to the attached barely-tested patch.

BTW, now that I see a case the default printout here seems
completely ridiculous.  I think we need to do

    pg_log_warning("could not resolve dependency loop among these items:");
    for (i = 0; i < nLoop; i++)
    {
        char        buf[1024];

        describeDumpableObject(loop[i], buf, sizeof(buf));
-       pg_log_info("  %s", buf);
+       pg_log_warning("  %s", buf);
    }

but I didn't actually change that in the attached.

                        regards, tom lane

diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 3af97a6039..689a3acff5 100644
--- 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 ed6ce41ad7..bc8f2ec36d 100644
--- 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 745578d855..759be027c8 100644
--- 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 &&

Reply via email to