Re: Minimal logical decoding on standbys

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