From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | pgsql-hackers(at)lists(dot)postgresql(dot)org |
Subject: | Remove trailing newlines from pg_upgrade's messages |
Date: | 2022-06-14 18:57:40 |
Message-ID: | 113191.1655233060@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Per the discussion at [1], pg_upgrade currently doesn't use
common/logging.c's functions. Making it do so looks like a
bigger lift than is justified, but there is one particular
inconsistency that I think we ought to remove: pg_upgrade
expects (most) message strings to end in newlines, while logging.c
expects them not to. This is bad for a couple of reasons:
* Translatable strings that otherwise could be shared with other
code are different.
* Developers might mistakenly add or leave off a newline because of
familiarity with how it's done elsewhere. This is especially bad for
pg_fatal() which is otherwise caller-compatible with the version
provided by logging.c. We fixed a couple of bugs of exactly that
description recently, and I found a few more as I went through
pg_upgrade for the attached patch. It doesn't help any that as it
stands, pg_upgrade requires some messages to end in newline and
others not: there are some places that are adding an extra newline,
apparently because whoever coded them was confused about which
convention applied.
Hence, the patch below removes trailing newlines from all of
pg_upgrade's message strings, and teaches its logging infrastructure
to print them where appropriate. As in logging.c, there's now an
Assert that no format string passed to pg_log() et al ends with
a newline.
This doesn't quite exactly match the code's prior behavior. Aside
from the buggy-looking newlines mentioned above, there are a few
messages that formerly ended with a double newline, thus intentionally
producing a blank line, and now they don't. I could have removed just
one of their newlines, but I'd have had to give up the Assert about
it, and I did not think that the extra blank lines were important
enough to justify that.
BTW, as I went through the code I realized just how badly pg_upgrade
needs a visit from the message style police. Its messages are not
even consistent with each other, let alone with our message style
guidelines. I have refrained (mostly) from doing any re-wording
here, but it could stand to be done.
I'll stick this in the CF queue, but I wonder if there is any case
for squeezing it into v15 instead of waiting for v16.
regards, tom lane
[1] https://www.postgresql.org/message-id/4036037.1655174501%40sss.pgh.pa.us
Attachment | Content-Type | Size |
---|---|---|
normalize-pg_upgrade-message-newlines-1.patch | text/x-diff | 84.2 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Peter Geoghegan | 2022-06-14 19:01:30 | Re: better page-level checksums |
Previous Message | Robert Haas | 2022-06-14 18:52:06 | Re: better page-level checksums |