Re: Log message for GSS connection is missing once connection authorization is successful.

From: vignesh C <vignesh21(at)gmail(dot)com>
To: Euler Taveira <euler(dot)taveira(at)2ndquadrant(dot)com>
Cc: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, Stephen Frost <sfrost(at)snowman(dot)net>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Log message for GSS connection is missing once connection authorization is successful.
Date: 2020-11-03 07:18:50
Message-ID: CALDaNm36xidovwu89ELA4fKaHVAocC0-HUtODrohjhfqRk0iJA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sun, Nov 1, 2020 at 3:34 AM Euler Taveira
<euler(dot)taveira(at)2ndquadrant(dot)com> wrote:
>
> On Sat, 31 Oct 2020 at 00:34, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
>>
>> On Fri, Oct 30, 2020 at 6:35 PM Euler Taveira
>> <euler(dot)taveira(at)2ndquadrant(dot)com> wrote:
>> >
>> > + appendStringInfo(&logmsg, "replication ");
>> > +
>> > + appendStringInfo(&logmsg, "connection authorized: user=%s",
>> > + port->user_name);
>> > + if (!am_walsender)
>> > + appendStringInfo(&logmsg, " database=%s", port->database_name);
>> > +
>> > + if (port->application_name != NULL)
>> > + appendStringInfo(&logmsg, " application_name=%s",
>> > + port->application_name);
>> > +
>> >
>> > Your approach breaks localization. You should use multiple errmsg.
>> >
>>
>> IIUC, isn't it enough calling a single errmsg() inside ereport() with
>> the prepared logmsg.data (which is a string)? The errmsg() function
>> will do the required translation of the logmsg.data. Why do we need
>> multiple errmsg() calls? Could you please elaborate a bit on how the
>> way currently it is done in the patch breaks localization?
>>
>
> No. The strings are specified in the appendStringInfo, hence you should add _()
> around the string to be translated. There is nothing to be translated if you
> specify only the format identifier. You can always test if gettext extracts the
> string to be translated by executing 'make update-po' (after specifying
> --enable-nls in the configure). Search for your string in one of the generated
> files (po/LL.po.new).
>
> You shouldn't split messages like that because not all languages have the same
> order as English. Having said that you risk providing a nonsense translation
> because someone decided to translate pieces of a sentence separately.
>
> + appendStringInfo(&logmsg, "replication ");
> +
> + appendStringInfo(&logmsg, "connection authorized: user=%s",
> + port->user_name);
>
> This hunk will break translation. In Portuguese, the adjective "replication" is
> translated after the noun "connection". If you decided to keep this code as is,
> the printed message won't follow the grammar rules. You will have "replicação
> conexão autorizada" instead of "conexão de replicação autorizada". The former
> isn't grammatically correct. Avoid splitting sentences that are translated.
>

Thanks for the explanation, I have attached a v5 patch with the
changes where the translation should not have any problem.

Regards,
Vignesh
EnterpriseDB: http://www.enterprisedb.com

Attachment Content-Type Size
v5-0001-Improving-the-connection-authorization-message-fo.patch text/x-patch 12.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jinbao Chen 2020-11-03 07:29:38 Re: Add table AM 'tid_visible'
Previous Message Peter Eisentraut 2020-11-03 07:08:14 Re: Keep elog(ERROR) and ereport(ERROR) calls in the cold path