Re: [BUG] pg_dump does not properly deal with BEGIN ATOMIC function - Mailing list pgsql-hackers
From | Tom Lane |
---|---|
Subject | Re: [BUG] pg_dump does not properly deal with BEGIN ATOMIC function |
Date | |
Msg-id | 49272.1685708176@sss.pgh.pa.us Whole thread Raw |
In response to | [BUG] pg_dump does not properly deal with BEGIN ATOMIC function ("Imseih (AWS), Sami" <simseih@amazon.com>) |
Responses |
Re: [BUG] pg_dump does not properly deal with BEGIN ATOMIC function
Re: [BUG] pg_dump does not properly deal with BEGIN ATOMIC function |
List | pgsql-hackers |
"Imseih (AWS), Sami" <simseih@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 &&
pgsql-hackers by date: