Re: Introduce "log_connection_stages" setting.

On 2/1/23 11:59, Sergey Dudoladov wrote:
> Justin, thank you for the fast review. The new version is attached.

This is looking very good. One bigger comment:

> + myextra = (int *) guc_malloc(ERROR, sizeof(int));
> + *myextra = newlogconnect;

If I've understood Tom correctly in [1], both of these guc_mallocs
should be using a loglevel less than ERROR, to avoid forcing a
postmaster exit when out of memory. (I used WARNING in that thread
instead, which seemed to be acceptable.)

And a couple of nitpicks:

> + Causes the specified stages of each connection attempt to the server to be logged. Example: <literal>authorized,disconnected</literal>.

Long line; should be rewrapped.

> + else {
> + GUC_check_errcode(ERRCODE_INVALID_PARAMETER_VALUE);
> + GUC_check_errmsg("invalid value '%s'", stage);
> + GUC_check_errdetail("Valid values to use in the list are 'all', 'received', 'authenticate
> + " If 'all' is present, it must be the only value in the list.");

I think the errmsg here should reuse the standard message format
invalid value for parameter "%s": "%s"
both for consistency and ease of translation.

Thanks!
--Jacob

[1] https://www.postgresql.org/message-id/2012342.1658356951%40sss.pgh.pa.us