pgsql: Remove function names from error messages

On Wed, Dec 19, 2018 at 05:53:48PM +0000, Alvaro Herrera wrote:

Remove function names from error messages

They are not necessary, and having them there gives useless work for
translators.

I am spotting a couple of extra ones for functions:
src/backend/access/transam/xlog.c:
(errmsg("pg_stop_backup cleanup done, waiting for required WAL segments
to be archived")));

src/backend/access/transam/xlog.c:
(errmsg("pg_stop_backup still waiting for all required WAL segments to
be archived (%d seconds elapsed)",

Perhaps these could be improved as well?
--
Michael

Michael Paquier <michael@paquier.xyz> writes:

On Wed, Dec 19, 2018 at 05:53:48PM +0000, Alvaro Herrera wrote:

Remove function names from error messages

I am spotting a couple of extra ones for functions:
src/backend/access/transam/xlog.c:
(errmsg("pg_stop_backup cleanup done, waiting for required WAL segments
to be archived")));
src/backend/access/transam/xlog.c:
(errmsg("pg_stop_backup still waiting for all required WAL segments to
be archived (%d seconds elapsed)",

Perhaps these could be improved as well?

Those are at least reporting SQL function names that the user will
recognize ... still, we don't normally localize error messages
by the reporting function name, so I tend to agree that these are
not following the style guide.

regards, tom lane

On Wed, Dec 19, 2018 at 07:42:31PM -0500, Tom Lane wrote:

Those are at least reporting SQL function names that the user will
recognize ... still, we don't normally localize error messages
by the reporting function name, so I tend to agree that these are
not following the style guide.

What do you think about something like the attached?
--
Michael

Michael Paquier <michael@paquier.xyz> writes:

On Wed, Dec 19, 2018 at 07:42:31PM -0500, Tom Lane wrote:

Those are at least reporting SQL function names that the user will
recognize ... still, we don't normally localize error messages
by the reporting function name, so I tend to agree that these are
not following the style guide.

What do you think about something like the attached?

Hm, I guess I shouldn't have used the word "localize". I didn't
mean whether the function name should be translated; what I meant
was that we normally don't mention individual functions at all in
error messages. The messages are supposed to be written as though
a monolithic entity "the system" is speaking to you. So what
I'd propose here is just

ereport(NOTICE,
(errmsg("waiting for required WAL segments to be archived")));

or something along that line. I'm not sure if the "cleanup done" part
is important, but I'd tend to the idea that it isn't.

regards, tom lane

On Wed, Dec 19, 2018 at 11:39:23PM -0500, Tom Lane wrote:

Hm, I guess I shouldn't have used the word "localize". I didn't
mean whether the function name should be translated; what I meant
was that we normally don't mention individual functions at all in
error messages. The messages are supposed to be written as though
a monolithic entity "the system" is speaking to you. So what
I'd propose here is just

ereport(NOTICE,
(errmsg("waiting for required WAL segments to be archived")));

or something along that line. I'm not sure if the "cleanup done" part
is important, but I'd tend to the idea that it isn't.

Okay. The cleanup part is roughly about the work of putting all the
shared variables back to an initial state, so indeed that does not
matter much to remove this part.

While at it, I am tempted to rework a bit the comments at the top of
do_pg_start_backup and do_pg_stop_backup as those are not only used for
the SQL-level interface but can also be used for base backups taken with
the replication protocol.

The WARNING about the segments not archived yet should not mention
directly pg_stop_backup as well as BASE_BACKUP never calls it.
--
Michael

On 2018-Dec-20, Michael Paquier wrote:

On Wed, Dec 19, 2018 at 05:53:48PM +0000, Alvaro Herrera wrote:

Remove function names from error messages

They are not necessary, and having them there gives useless work for
translators.

I am spotting a couple of extra ones for functions:
src/backend/access/transam/xlog.c:
(errmsg("pg_stop_backup cleanup done, waiting for required WAL segments
to be archived")));

Yeah, I grepped for other uses and found format() being mentioned too,
but there didn't seem to be any repetition in the messages other than
the ones I patched, so I didn't bother. Thanks for following through
with these ones in the other thread.

I grepped using '()' as part of the pattern to find function names;
maybe there's a more comprehensive way.

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Fri, Dec 21, 2018 at 11:05:37AM -0300, Alvaro Herrera wrote:

Yeah, I grepped for other uses and found format() being mentioned too,
but there didn't seem to be any repetition in the messages other than
the ones I patched, so I didn't bother. Thanks for following through
with these ones in the other thread.

I grepped using '()' as part of the pattern to find function names;
maybe there's a more comprehensive way.

For the note: I have been grepping for "pg_".
--
Michael