Hi,
On 4/4/23 1:21 PM, Alvaro Herrera wrote:
Hi,
On 2023-Apr-03, Andres Freund wrote:
Hm? That's what the _'s do. We build strings in parts in other places too.
No, what _() does is mark each piece for translation separately. But a
translation cannot be done on string pieces, and later have all the
pieces appended together to form a full sentence. Let me show the
"!terminating" case as example and grab some translations for it from
src/backend/po/de.po:
"invalidating" -> "... wird ungültig gemacht" (?)
(if logical) " obsolete replication" -> " obsolete Replikation"
" slot \"%s\" because it conflicts with recovery" -> " Slot \"%s\", weil sie in
Konflikt mit Wiederherstellung steht"
If you just concatenate all the translated phrases together, the
resulting string will make no sense; keep in mind the "obsolete
replication" part may or not may not be there. And there's no way to
make that work: even if you found an ordering of the English parts that
allows you to translate each piece separately and have it make sense for
German, the same won't work for Spanish or Japanese.
You have to give the translator a complete phrase and let them turn into
a complete translated phrases. Building from parts doesn't work. We're
very good at avoiding string building; we have a couple of cases, but
they are *very* minor.
string 1 "invalidating slot \"%s\" because it conflicts with recovery"
string 2 "invalidating obsolete replication slot \"%s\" because it conflicts with
recovery"
Thanks for looking at it and the explanations!
(I'm not clear on why did Bertrand omitted the word "replication" in the
case where the slot is not logical)
It makes more sense to add it, will do thanks!
I think the errdetail() are okay, it's the errmsg() bits that are bogus.
And yes, well caught on having to use errmsg_internal and
errdetail_internal() to avoid double translation.
So, IIUC having something like this would be fine?
"
if (check_on_xid)
{
if (terminating)
appendStringInfo(&err_msg, _("terminating process %d to release replication slot
\"%s\" because it conflicts with recovery"),
pid,
NameStr(slotname));
else
appendStringInfo(&err_msg, _("invalidating replication slot \"%s\"
because it conflicts with recovery"),
NameStr(slotname));
if (TransactionIdIsValid(*xid))
appendStringInfo(&err_detail, _("The slot conflicted with xid horizon
%u."), *xid);
else
appendStringInfo(&err_detail, _("Logical decoding on standby requires
wal_level to be at least logical on the primary server"));
}
else
{
if (terminating)
appendStringInfo(&err_msg, _("terminating process %d to release replication slot
\"%s\""),
pid,
NameStr(slotname));
else
appendStringInfo(&err_msg, _("invalidating obsolete replication slot
\"%s\""),
NameStr(slotname));
appendStringInfo(&err_detail, _("The slot's restart_lsn %X/%X exceeds the
limit by %llu bytes."),
LSN_FORMAT_ARGS(restart_lsn),
(unsigned long long) (oldestLSN - restart_lsn));
hint = true;
}
ereport(LOG,
errmsg_internal("%s", err_msg.data),
errdetail_internal("%s", err_detail.data),
hint ? errhint("You might need to increase
max_slot_wal_keep_size.") : 0);
"
as err_msg is not concatenated anymore (I mean it's just one sentence build one
time)
and this make use of errmsg_internal() and errdetail_internal().
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
