Horton

Thread

Commits

  1. Rename libpq trace internal functions

  2. Introduce macros for protocol characters.

  1. Rename libpq trace internal functions

    Peter Eisentraut <peter@eisentraut.org> — 2024-04-24T07:39:02Z

    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;

(This also improves the readability of the file in general, since some
function names like "pqTraceOutputt" were a little hard to read
accurately.)

Some protocol characters have different meanings to and from the
server.  The old code structure had a common function for both, for
example, pqTraceOutputD().  The new structure splits this up into
separate ones to match the protocol message name, like
pqTraceOutput_Describe() and pqTraceOutput_DataRow().

  2. Re: Rename libpq trace internal functions

    Yugo Nagata <nagata@sraoss.co.jp> — 2024-04-24T10:34:22Z

    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()?

Regards,
Yugo Nagata

> (This also improves the readability of the file in general, since some
> function names like "pqTraceOutputt" were a little hard to read
> accurately.)
>
> Some protocol characters have different meanings to and from the
> server.  The old code structure had a common function for both, for
> example, pqTraceOutputD().  The new structure splits this up into
> separate ones to match the protocol message name, like
> pqTraceOutput_Describe() and pqTraceOutput_DataRow().


-- 
Yugo NAGATA <nagata@sraoss.co.jp>

  3. Re: Rename libpq trace internal functions

    Peter Eisentraut <peter@eisentraut.org> — 2024-05-02T14:22:49Z

    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.