Thread: pgsql: Remove function names from error messages
Remove function names from error messages They are not necessary, and having them there gives useless work for translators. Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/68f6f2b7395fe3e403034bcd97a1fcfbcc68ae10 Modified Files -------------- src/backend/access/transam/xlogfuncs.c | 6 ++++-- src/backend/commands/extension.c | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-)
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
Attachment
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
Attachment
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
Attachment
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