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