pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Nathan Bossart <nathandbossart@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: Dave Cramer <davecramer@gmail.com>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Peter Smith <smithpb2250@gmail.com>
Cc: Tatsuo Ishii <ishii@sraoss.co.jp>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: Using defines for protocol characters
Date: Mon, 7 Aug 2023 11:38:49 -0700
Message-ID: <20230807183849.GA1244255@nathanxps13> (raw)
In-Reply-To: <838284.1691432698@sss.pgh.pa.us>
References: <CAHut+PuE-3tc_6m06bwnjftk2KBM728ZSDhuL1_=vRCfEGMZ1A@mail.gmail.com>
	<20230807091044.jjgsl2rgheazaung@alvherre.pgsql>
	<CADK3HH+aXM82oY-tUzidBS1bfqoUFubVxygmFaBFZFV=3iHYdw@mail.gmail.com>
	<CA+TgmoZ=XfKEqiq80SbN0R_XpLnnkgjGuO5WBTL0nYTseKD4TQ@mail.gmail.com>
	<838284.1691432698@sss.pgh.pa.us>

On Mon, Aug 07, 2023 at 02:24:58PM -0400, Tom Lane wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> IMHO, the correspondence between the names in the patch and the
>> traditional names in the documentation could be stronger. For example,
>> the documentation mentions EmptyQueryResponse and
>> NegotiateProtocolVersion, but in this patch those become
>> PQMSG_RESP_NEGOTIATE_PROTOCOL and PQMSG_RESP_EMPTY_QUERY. I think we
>> could consider directly using the names from the documentation, right
>> down to capitalization, perhaps with a prefix, but I'm also totally
>> fine with this use of uppercase letters and underscores. But why not
>> do a strict translation, like EmptyQueryResponse ->
>> PQMSG_EMPTY_QUERY_RESPONSE, NegotiateProtocolVersion ->
>> PQMSG_NEGOTIATE_PROTOCOL_VERSION? To me at least, the current patch is
>> inventing new and slightly different names for things that already
>> have names...
> 
> +1.  For ease of greppability, maybe even PQMSG_EmptyQueryResponse
> and so on?  Then one grep would find both uses of the constants and
> code/docs references.  Not sure if the prefix should be all-caps or
> not if we go this way.

+1 for Tom's suggestion.  I have no strong opinion about the prefix, but I
do think easing greppability is worthwhile.

As mentioned earlier [0], I think we should also consider putting the
definitions in pqcomm.h.

[0] https://postgr.es/m/20230803185356.GA1144430%40nathanxps13

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com





view thread (47+ messages)  latest in thread

Message-ID: <20230807183849.GA1244255@nathanxps13>
Permalink:  ../20230807183849.GA1244255@nathanxps13/
Also on:    postgresql.org/message-id/20230807183849.GA1244255@nathanxps13

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: nathandbossart@gmail.com, tgl@sss.pgh.pa.us, robertmhaas@gmail.com, davecramer@gmail.com, alvherre@alvh.no-ip.org, smithpb2250@gmail.com, ishii@sraoss.co.jp, pgsql-hackers@lists.postgresql.org
  Subject: Re: Using defines for protocol characters
  In-Reply-To: <20230807183849.GA1244255@nathanxps13>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox