On Wed, 14 Oct 2020 at 07:44, Alvaro Herrera <alvherre@alvh.no-ip.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.
Regards,
--
Masahiko Sawada http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services