From: Tom Lane <tgl@sss.pgh.pa.us>
To: Kirk Wolak <wolakk@gmail.com>
Cc: Imseih (AWS), Sami <simseih@amazon.com>
Cc: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: [BUG] pg_dump does not properly deal with BEGIN ATOMIC function
Date: Sat, 03 Jun 2023 14:28:11 -0400
Message-ID: <1868.1685816891@sss.pgh.pa.us> (raw)
In-Reply-To: <CACLU5mTMoj8yx-tUFk84S1tfY-byvNVAWvVJ0MqG8Bp5B-wm_A@mail.gmail.com>
References: <2C1933AB-C2F8-499B-9D18-4AC1882256A0@amazon.com>
<49272.1685708176@sss.pgh.pa.us>
<CACLU5mTMoj8yx-tUFk84S1tfY-byvNVAWvVJ0MqG8Bp5B-wm_A@mail.gmail.com>
Kirk Wolak <wolakk@gmail.com> writes:
> On Fri, Jun 2, 2023 at 8:16 AM Tom Lane <tgl@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
Attachments:download all (2 files, .tar.gz)easy-warning-fix.patch (text/x-patch, 419B) downloaddiff --git a/src/bin/pg_dump/pg_dump_sort.c b/src/bin/pg_dump/pg_dump_sort.cindex 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)
harder-warning-fix.patch (text/x-patch, 1005B) downloaddiff --git a/src/bin/pg_dump/pg_dump_sort.c b/src/bin/pg_dump/pg_dump_sort.cindex 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);
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, wolakk@gmail.com, simseih@amazon.com, peter.eisentraut@enterprisedb.com, pgsql-hackers@lists.postgresql.org
Subject: Re: [BUG] pg_dump does not properly deal with BEGIN ATOMIC function
In-Reply-To: <1868.1685816891@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox