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.