On 24.04.24 12:34, Yugo NAGATA wrote:
> On Wed, 24 Apr 2024 09:39:02 +0200
> Peter Eisentraut <peter@eisentraut.org> wrote:
>
>> libpq's pqTraceOutputMessage() used to look like this:
>>
>> case 'Z': /* Ready For Query */
>> pqTraceOutputZ(conn->Pfdebug, message, &logCursor);
>> break;
>>
>> Commit f4b54e1ed98 introduced macros for protocol characters, so now
>> it looks like this:
>>
>> case PqMsg_ReadyForQuery:
>> pqTraceOutputZ(conn->Pfdebug, message, &logCursor);
>> break;
>>
>> But this introduced a disconnect between the symbol in the switch case
>> and the function name to be called, so this made the manageability of
>> this file a bit worse.
>>
>> This patch changes the function names to match, so now it looks like
>> this:
>>
>> case PqMsg_ReadyForQuery:
>> pqTraceOutput_ReadyForQuery(conn->Pfdebug, message, &logCursor);
>> break;
>
> +1
>
> I prefer the new function names since it seems more natural and easier to read.
>
> I noticed pqTraceOutputNR() is left as is, is this intentional? Or, shoud this
> be changed to pqTranceOutput_NoticeResponse()?
pqTraceOutputNR() is shared code used internally by _ErrorResponse() and
_NoticeResponse(). I have updated the comments a bit to make this clearer.
With that, I have committed it. Thanks.