Thread: Rename libpq trace internal functions
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().
Attachment
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>
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.