Remove trailing newlines from pg_upgrade's messages

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

Responses

Browse pgsql-hackers by date

  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