Hi,
On 11/27/20 2:40 PM, Alvaro Herrera wrote:
> On 2020-Nov-27, Drouvot, Bertrand wrote:
>
>> + if (nprocs > 0)
>> + {
>> + ereport(LOG,
>> + (errmsg("%s after %ld.%03d ms",
>> + get_recovery_conflict_desc(reason), msecs, usecs),
>> + (errdetail_log_plural("Conflicting process: %s.",
>> + "Conflicting processes: %s.",
>> + nprocs, buf.data))));
>> + }
>> +/* Return the description of recovery conflict */
>> +static const char *
>> +get_recovery_conflict_desc(ProcSignalReason reason)
>> +{
>> + const char *reasonDesc = gettext_noop("unknown reason");
>> +
>> + switch (reason)
>> + {
>> + case PROCSIG_RECOVERY_CONFLICT_BUFFERPIN:
>> + reasonDesc = gettext_noop("recovery is still waiting for recovery conflict on buffer pin");
>> + break;
> This doesn't work from a translation point of view. First, you're
> building a sentence from parts, which is against policy. Second, you're
> not actually invoking gettext to translate the string returned by
> get_recovery_conflict_desc.
>
> I think this needs to be rethought. To handle the first problem I
> suggest to split the error message in two. One phrase is the complain
> that recovery is waiting, and the other string is the reason for the
> wait. Separate both either by splitting with a :, or alternatively put
> the other sentence in DETAIL. (The latter would require to mix with the
> list of conflicting processes, which might be hard.)
>
> The first idea would look like this:
>
> LOG: recovery still waiting after %ld.03d ms: for recovery conflict on buffer pin
> DETAIL: Conflicting processes: 1, 2, 3.
>
> To achieve this, apart from editing the messages returned by
> get_recovery_conflict_desc, you'd need to ereport this way:
>
> ereport(LOG,
> errmsg("recovery still waiting after %ld.%03d ms: %s",
> msecs, usecs, _(get_recovery_conflict_desc(reason))),
> errdetail_log_plural("Conflicting process: %s.",
> "Conflicting processes: %s.",
>
Thanks for your comments, I did not know that.
I've attached a new version that takes your comments into account.
Thanks!
Bertrand
