On Fri, Oct 30, 2020 at 6:35 PM Euler Taveira
<euler.tave...@2ndquadrant.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?
+ ereport(LOG, errmsg("%s", logmsg.data));
>
> +$node->append_conf('postgresql.conf', "logging_collector= 'on'");
> +$node->append_conf('postgresql.conf', "log_connections= 'on'");
>
> booleans don't need quotes.
>
I think that's not correct. If I'm right, the snippet pointed above is
from a perl script. In C, the strings are null terminated and they are
represented within double quotes. So we need to use double quotes for
_("on") : _("off"). And also the definition of _( ) macro points to a
function err_gettext() that expects C-style string i.e null
terminated.
With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com
