Re: Add Information during standby recovery conflicts

At Wed, 14 Oct 2020 17:39:20 +0900, Masahiko Sawada <masahiko(dot)sawada(at)2ndquadrant(dot)com> wrote in
> On Wed, 14 Oct 2020 at 07:44, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> wrote:
> >
> > On 2020-Oct-14, Masahiko Sawada wrote:
> >
> > > I've attached the patch as an idea of fixing the above comments as
> > > well as the comment from Alvaro. I can be applied on top of v4 patch.
> >
> > One note about the translation stuff. Currently you have _("...") where
> > the string is produced, and then ereport(.., errmsg("%s", str) where it
> > is used. Both those things will attempt to translate the string, which
> > isn't great. It is better if we only translate once. You have two
> > options to fix this: one is to change _() to gettext_noop() (which marks
> > the string for translation so that it appears in the message catalog,
> > but it does not return the translation -- it returns the original, and
> > then errmsg() translates at run time). The other is to change errmsg()
> > to errmsg_internal() .. so the function returns the translated message
> > and errmsg_internal() doesn't apply a translation.
> >
> > I prefer the first option, because if we ever include a server feature
> > to log both the non-translated message alongside the translated one, we
> > will already have both in hand.
>
> Thanks, I didn't know that. So perhaps ATWrongRelkindError() has the
> same translation problem? It uses _() when producing the message but
> also uses errmsg().
>
> I've attached the patch changed accordingly. I also fixed some bugs
> around recovery conflicts on locks and changed the code so that the
> log shows pids instead of virtual transaction ids since pids are much
> easy to use for the users.

You're misunderstanding.

ereport(..(errmsg("%s", _("hogehoge")))) results in
fprintf((translated("%s")), translate("hogehoge")).

So your change (errmsg("%s", gettext_noop("hogehoge")) results in

fprintf((translated("%s")), DONT_translate("hogehoge")).

which leads to a translation problem.

(errmsg(gettext_noop("hogehoge"))

works fine. You can see the instance in aclchk.c.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center