Kirk Wolak <wol...@gmail.com> writes: > On Fri, Jun 2, 2023 at 8:16 AM Tom Lane <t...@sss.pgh.pa.us> wrote: >> BTW, now that I see a case the default printout here seems >> completely ridiculous. I think we need to do >> - pg_log_info(" %s", buf); >> + pg_log_warning(" %s", buf);
> If I comprehend the suggestion, it will label each line with a warning. > Which implies I have 6 Warnings. Right, I'd forgotten that pg_log_warning() will interpose "warning:". Attached are two more-carefully-thought-out suggestions. The easy way is to use pg_log_warning_detail(), which produces output like pg_dump: warning: could not resolve dependency loop among these items: pg_dump: detail: FUNCTION a_f (ID 216 OID 40532) pg_dump: detail: CONSTRAINT a_pkey (ID 3466 OID 40531) pg_dump: detail: POST-DATA BOUNDARY (ID 3612) pg_dump: detail: TABLE DATA a (ID 3610 OID 40525) pg_dump: detail: PRE-DATA BOUNDARY (ID 3611) Alternatively, we could assemble the details by hand, as in the second patch, producing pg_dump: warning: could not resolve dependency loop among these items: FUNCTION a_f (ID 216 OID 40532) CONSTRAINT a_pkey (ID 3466 OID 40531) POST-DATA BOUNDARY (ID 3612) TABLE DATA a (ID 3610 OID 40525) PRE-DATA BOUNDARY (ID 3611) I'm not really sure which of these I like better. The first one is a much simpler code change, and there is some value in labeling the output like that. The second patch's output seems less cluttered, but it's committing a modularity sin by embedding formatting knowledge at the caller level. Thoughts? BTW, there is a similar abuse of pg_log_info just a few lines above this, and probably others elsewhere. I won't bother writing patches for other places till we have agreement on what the output ought to look like. regards, tom lane
diff --git a/src/bin/pg_dump/pg_dump_sort.c b/src/bin/pg_dump/pg_dump_sort.c index 745578d855..3c66b8bf1a 100644 --- a/src/bin/pg_dump/pg_dump_sort.c +++ b/src/bin/pg_dump/pg_dump_sort.c @@ -1253,7 +1253,7 @@ repairDependencyLoop(DumpableObject **loop, char buf[1024]; describeDumpableObject(loop[i], buf, sizeof(buf)); - pg_log_info(" %s", buf); + pg_log_warning_detail(" %s", buf); } if (nLoop > 1)
diff --git a/src/bin/pg_dump/pg_dump_sort.c b/src/bin/pg_dump/pg_dump_sort.c index 745578d855..f0eef849d3 100644 --- a/src/bin/pg_dump/pg_dump_sort.c +++ b/src/bin/pg_dump/pg_dump_sort.c @@ -971,6 +971,7 @@ static void repairDependencyLoop(DumpableObject **loop, int nLoop) { + PQExpBuffer msgbuf; int i, j; @@ -1247,14 +1248,17 @@ repairDependencyLoop(DumpableObject **loop, * If we can't find a principled way to break the loop, complain and break * it in an arbitrary fashion. */ - pg_log_warning("could not resolve dependency loop among these items:"); + msgbuf = createPQExpBuffer(); for (i = 0; i < nLoop; i++) { char buf[1024]; describeDumpableObject(loop[i], buf, sizeof(buf)); - pg_log_info(" %s", buf); + appendPQExpBuffer(msgbuf, "\n %s", buf); } + pg_log_warning("could not resolve dependency loop among these items:%s", + msgbuf->data); + destroyPQExpBuffer(msgbuf); if (nLoop > 1) removeObjectDependency(loop[0], loop[1]->dumpId);