Date: Tue, 13 Oct 2020 19:44:08 -0300
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Masahiko Sawada <masahiko.sawada@2ndquadrant.com>
Cc: "Drouvot, Bertrand" <bdrouvot@amazon.com>,
	PostgreSQL-development <pgsql-hackers@postgresql.org>
Subject: Re: Add Information during standby recovery conflicts
Message-ID: <20201013224408.GA5517@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: 
 <CA+fd4k7PhTtN9D5gfi8y2DEHVBawue7vvuEK9d5XUHrp5tD7gg@mail.gmail.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk

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.