Date: Wed, 24 Apr 2024 19:34:22 +0900
From: Yugo NAGATA <nagata@sraoss.co.jp>
To: Peter Eisentraut <peter@eisentraut.org>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: Rename libpq trace internal functions
Message-Id: <20240424193422.ef3ea25136e665448852a8fa@sraoss.co.jp>
In-Reply-To: <575e4f9d-acfe-45e3-b7f1-7e32c579090e@eisentraut.org>
References: <575e4f9d-acfe-45e3-b7f1-7e32c579090e@eisentraut.org>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Archived-At: <https://www.postgresql.org/message-id/20240424193422.ef3ea25136e665448852a8fa%40sraoss.co.jp>
Precedence: bulk

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>