Re: Introduce "log_connection_stages" setting.

Hi,

+1 for the idea!

> +        <entry><literal>authenticated</literal></entry>
> +        <entry>Logs the original identity that an authentication method employs to identify a user. In most cases,
theidentity string equals the PostgreSQL username,
 
> +        but some third-party authentication methods may alter the original user identifier before the server stores
it.Failed authentication is always logged regardless of the value of this setting.</entry>
 

I think the documentation needs to be rewrapped; those are very long lines.

On 11/17/22 07:36, Justin Pryzby wrote:
> This function hardcodes each of the 4 connections:
> 
>> +        if (pg_strcasecmp(stage, "received") == 0)
>> +            myextra[0] = true;
> 
> It'd be better to use #defines or enums for these.

Hardcoding seems reasonable to me, if this is the only place we're doing
string comparison.

>> --- a/src/backend/tcop/postgres.c
>> +++ b/src/backend/tcop/postgres.c
>> @@ -84,8 +84,11 @@ const char *debug_query_string; /* client-supplied query string */
>>  /* Note: whereToSendOutput is initialized for the bootstrap/standalone case */
>>  CommandDest whereToSendOutput = DestDebug;
>>  
>> -/* flag for logging end of session */
>> -bool        Log_disconnections = false;
>> +/* flags for logging information about session state */
>> +bool        Log_disconnected = false;
>> +bool        Log_authenticated = false;
>> +bool        Log_authorized = false;
>> +bool        Log_received = false;
> 
> I think this ought to be an integer with flag bits, rather than 4
> booleans (I don't know, but there might be more later?).  Then, the
> implementation follows the user-facing GUC and also follows
> log_destination.

Agreed. Or at the very least, follow what's done with
wal_consistency_checking? But I think flag bits would be better.

The tests should be expanded for cases other than 'all'.

As to the failing test cases: it looks like there's a keyword issue with
ALTER SYSTEM and 'all', but trying to fix it by quoting also fails. I
think it's because of GUC_LIST_QUOTE -- is there a reason that's used
here? I don't think we'd need any special characters in future option
names. wal_consistency_checking is very similar, and it just uses
GUC_LIST_INPUT.

Thanks,
--Jacob