Re: Using defines for protocol characters

2023-08-22 Thread Nathan Bossart
On Thu, Aug 17, 2023 at 09:52:03AM -0700, Nathan Bossart wrote: > On Thu, Aug 17, 2023 at 12:22:24PM -0400, Robert Haas wrote: >> I think this is going to be a major improvement in terms of readability! >> >> I'm pretty happy with this version, too. We may be running out of >> things to argue abou

Re: Using defines for protocol characters

2023-08-17 Thread Nathan Bossart
On Thu, Aug 17, 2023 at 12:22:24PM -0400, Robert Haas wrote: > I think this is going to be a major improvement in terms of readability! > > I'm pretty happy with this version, too. We may be running out of > things to argue about -- and no, I don't want to argue about > underscores more! Awesome.

Re: Using defines for protocol characters

2023-08-17 Thread Dave Cramer
On Thu, Aug 17, 2023 at 9:22 AM Robert Haas wrote: > On Thu, Aug 17, 2023 at 12:13 PM Nathan Bossart > wrote: > > On Thu, Aug 17, 2023 at 09:31:55AM +0900, Michael Paquier wrote: > > > Looks sensible seen from here. > > > > Thanks for taking a look. > > I think this is going to be a major improv

Re: Using defines for protocol characters

2023-08-17 Thread Robert Haas
On Thu, Aug 17, 2023 at 12:13 PM Nathan Bossart wrote: > On Thu, Aug 17, 2023 at 09:31:55AM +0900, Michael Paquier wrote: > > Looks sensible seen from here. > > Thanks for taking a look. I think this is going to be a major improvement in terms of readability! I'm pretty happy with this version,

Re: Using defines for protocol characters

2023-08-17 Thread Nathan Bossart
On Thu, Aug 17, 2023 at 09:31:55AM +0900, Michael Paquier wrote: > Looks sensible seen from here. Thanks for taking a look. > This patch is missing the installation of protocol.h in > src/tools/msvc/Install.pm for MSVC. For pqcomm.h, we are doing that: > lcopy('src/include/libpq/pqcomm.h', $targ

Re: Using defines for protocol characters

2023-08-16 Thread Michael Paquier
On Wed, Aug 16, 2023 at 12:29:56PM -0700, Nathan Bossart wrote: > I moved the definitions out to a separate file in v6. Looks sensible seen from here. This patch is missing the installation of protocol.h in src/tools/msvc/Install.pm for MSVC. For pqcomm.h, we are doing that: lcopy('src/include/l

Re: Using defines for protocol characters

2023-08-16 Thread Nathan Bossart
On Tue, Aug 15, 2023 at 11:40:07PM +0200, Alvaro Herrera wrote: > On 2023-Aug-16, Michael Paquier wrote: > >> On Wed, Aug 16, 2023 at 06:25:09AM +0900, Tatsuo Ishii wrote: >> > Currently pqcomm.h needs c.h which is not problem for Pgpool-II. But >> > what about other middleware? >> >> Why do you

Re: Using defines for protocol characters

2023-08-15 Thread Tatsuo Ishii
> On Wed, Aug 16, 2023 at 06:25:09AM +0900, Tatsuo Ishii wrote: >> Currently pqcomm.h needs c.h which is not problem for Pgpool-II. But >> what about other middleware? > > Why do you need to include directly c.h? There are definitions in > there that are not intended to be exposed. pqcomm.h has

Re: Using defines for protocol characters

2023-08-15 Thread Alvaro Herrera
On 2023-Aug-16, Michael Paquier wrote: > On Wed, Aug 16, 2023 at 06:25:09AM +0900, Tatsuo Ishii wrote: > > Currently pqcomm.h needs c.h which is not problem for Pgpool-II. But > > what about other middleware? > > Why do you need to include directly c.h? There are definitions in > there that are

Re: Using defines for protocol characters

2023-08-15 Thread Michael Paquier
On Wed, Aug 16, 2023 at 06:25:09AM +0900, Tatsuo Ishii wrote: > Currently pqcomm.h needs c.h which is not problem for Pgpool-II. But > what about other middleware? Why do you need to include directly c.h? There are definitions in there that are not intended to be exposed. -- Michael signature.a

Re: Using defines for protocol characters

2023-08-15 Thread Tatsuo Ishii
> On Tue, Aug 15, 2023 at 02:46:24PM +0900, Tatsuo Ishii wrote: >> Is it possible to put the new define staff into protocol.h then let >> pqcomm.h include protocol.h? This makes Pgpool-II and other middle >> ware/drivers written in C easier to use the defines so that they only >> include protocol.h

Re: Using defines for protocol characters

2023-08-15 Thread Nathan Bossart
On Tue, Aug 15, 2023 at 02:46:24PM +0900, Tatsuo Ishii wrote: > Is it possible to put the new define staff into protocol.h then let > pqcomm.h include protocol.h? This makes Pgpool-II and other middle > ware/drivers written in C easier to use the defines so that they only > include protocol.h to us

Re: Using defines for protocol characters

2023-08-14 Thread Tatsuo Ishii
> I tried to address all the feedback in v5 of the patch, which is attached. > I limited the patch to only the characters that have names in the "Message > Formats" section of protocol.sgml instead of trying to invent names for > unnamed characters. > > I'm aware of one inconsistency. While I gro

Re: Using defines for protocol characters

2023-08-14 Thread Nathan Bossart
I tried to address all the feedback in v5 of the patch, which is attached. I limited the patch to only the characters that have names in the "Message Formats" section of protocol.sgml instead of trying to invent names for unnamed characters. I'm aware of one inconsistency. While I grouped all the

Re: Using defines for protocol characters

2023-08-09 Thread Alvaro Herrera
On 2023-Aug-09, Nathan Bossart wrote: > On Wed, Aug 09, 2023 at 10:44:42AM -0600, Dave Cramer wrote: > > On Wed, 9 Aug 2023 at 10:34, Tom Lane wrote: > >> I agree with Peter: let's use the names in the protocol document > >> with a single prefix. I've got mixed feelings about whether that prefix

Re: Using defines for protocol characters

2023-08-09 Thread Nathan Bossart
On Wed, Aug 09, 2023 at 10:44:42AM -0600, Dave Cramer wrote: > On Wed, 9 Aug 2023 at 10:34, Tom Lane wrote: >> I agree with Peter: let's use the names in the protocol document >> with a single prefix. I've got mixed feelings about whether that prefix >> should have an underscore, though. > > Wel

Re: Using defines for protocol characters

2023-08-09 Thread Dave Cramer
On Wed, 9 Aug 2023 at 10:34, Tom Lane wrote: > Dave Cramer writes: > > On Wed, 9 Aug 2023 at 09:19, Peter Eisentraut > wrote: > >> 3. IMO, the names of the protocol messages in protocol.sgml are > >> canonical. Your patch appends "Request" and "Response" in cases where > >> that is not part of

Re: Using defines for protocol characters

2023-08-09 Thread Tom Lane
Dave Cramer writes: > On Wed, 9 Aug 2023 at 09:19, Peter Eisentraut wrote: >> 3. IMO, the names of the protocol messages in protocol.sgml are >> canonical. Your patch appends "Request" and "Response" in cases where >> that is not part of the actual name. Also, some messages are documented >> to

Re: Using defines for protocol characters

2023-08-09 Thread Dave Cramer
On Wed, 9 Aug 2023 at 09:19, Peter Eisentraut wrote: > 1. As was discussed, these definitions should go into > src/include/libpq/pqcomm.h, not a new file. > I'm ambivalent, this is very easy to do. > > 2. I would prefer an underscore after PgMsg, like PqMsg_DescribeRequest, > so it's easier to

Re: Using defines for protocol characters

2023-08-09 Thread Peter Eisentraut
1. As was discussed, these definitions should go into src/include/libpq/pqcomm.h, not a new file. 2. I would prefer an underscore after PgMsg, like PqMsg_DescribeRequest, so it's easier to visually locate the start of the actual message name. 3. IMO, the names of the protocol messages in prot

Re: Using defines for protocol characters

2023-08-07 Thread Alvaro Herrera
On 2023-Aug-07, Nathan Bossart wrote: > On Mon, Aug 07, 2023 at 04:02:08PM -0400, Tom Lane wrote: > > Dave Cramer writes: > >> Can we vote or whatever it takes to decide on a naming pattern that is > >> acceptable ? > > > > I'm good with Robert's proposal above. > > +1 WFM as well. -- Álvar

Re: Using defines for protocol characters

2023-08-07 Thread Tatsuo Ishii
> On Mon, 7 Aug 2023 at 16:50, Tatsuo Ishii wrote: > >> > On Mon, Aug 07, 2023 at 04:02:08PM -0400, Tom Lane wrote: >> >> Dave Cramer writes: >> >>> On Mon, 7 Aug 2023 at 12:59, Robert Haas >> wrote: >> PqMsgEmptyQueryResponse or something like that seems better, if we >> want to keep

Re: Using defines for protocol characters

2023-08-07 Thread Dave Cramer
On Mon, 7 Aug 2023 at 16:50, Tatsuo Ishii wrote: > > On Mon, Aug 07, 2023 at 04:02:08PM -0400, Tom Lane wrote: > >> Dave Cramer writes: > >>> On Mon, 7 Aug 2023 at 12:59, Robert Haas > wrote: > PqMsgEmptyQueryResponse or something like that seems better, if we > want to keep the curre

Re: Using defines for protocol characters

2023-08-07 Thread Tatsuo Ishii
> On Mon, Aug 07, 2023 at 04:02:08PM -0400, Tom Lane wrote: >> Dave Cramer writes: >>> On Mon, 7 Aug 2023 at 12:59, Robert Haas wrote: PqMsgEmptyQueryResponse or something like that seems better, if we want to keep the current capitalization. I'm not a huge fan of the way we vary o

Re: Using defines for protocol characters

2023-08-07 Thread Peter Smith
+#ifndef _PROTOCOL_H +#define _PROTOCOL_H + +#define PQMSG_REQ_BIND 'B' +#define PQMSG_REQ_CLOSE 'C' +#define PQMSG_REQ_DESCRIBE 'D' +#define PQMSG_REQ_EXECUTE 'E' +#define PQMSG_REQ_FUNCTION_CALL 'F' +#define PQMSG_REQ_FLUSH_DATA'H' +#define

Re: Using defines for protocol characters

2023-08-07 Thread Nathan Bossart
On Mon, Aug 07, 2023 at 04:02:08PM -0400, Tom Lane wrote: > Dave Cramer writes: >> On Mon, 7 Aug 2023 at 12:59, Robert Haas wrote: >>> PqMsgEmptyQueryResponse or something like that seems better, if we >>> want to keep the current capitalization. I'm not a huge fan of the way >>> we vary our capi

Re: Using defines for protocol characters

2023-08-07 Thread Tom Lane
Dave Cramer writes: > On Mon, 7 Aug 2023 at 12:59, Robert Haas wrote: >> PqMsgEmptyQueryResponse or something like that seems better, if we >> want to keep the current capitalization. I'm not a huge fan of the way >> we vary our capitalization conventions so much all over the code base, >> but I

Re: Using defines for protocol characters

2023-08-07 Thread Dave Cramer
On Mon, 7 Aug 2023 at 12:59, Robert Haas wrote: > On Mon, Aug 7, 2023 at 2:25 PM Tom Lane wrote: > > +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

Re: Using defines for protocol characters

2023-08-07 Thread Robert Haas
On Mon, Aug 7, 2023 at 2:25 PM Tom Lane wrote: > +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. PqMsgEmptyQueryR

Re: Using defines for protocol characters

2023-08-07 Thread Nathan Bossart
On Mon, Aug 07, 2023 at 02:24:58PM -0400, Tom Lane wrote: > Robert Haas 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 >> NegotiateProtocolVers

Re: Using defines for protocol characters

2023-08-07 Thread Tom Lane
Robert Haas 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_PROTO

Re: Using defines for protocol characters

2023-08-07 Thread Robert Haas
On Mon, Aug 7, 2023 at 1:19 PM Dave Cramer wrote: > Any other changes required ? 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 i

Re: Using defines for protocol characters

2023-08-07 Thread Dave Cramer
On Mon, 7 Aug 2023 at 03:10, Alvaro Herrera wrote: > On 2023-Aug-07, Peter Smith wrote: > > > I guess, your patch would not be much different; you can still have > > all the nice names and assign the appropriate values to the enum > > values same as now, but using an enum you might also gain > >

Re: Using defines for protocol characters

2023-08-07 Thread Alvaro Herrera
On 2023-Aug-07, Peter Smith wrote: > I guess, your patch would not be much different; you can still have > all the nice names and assign the appropriate values to the enum > values same as now, but using an enum you might also gain > type-checking in the code and also get warnings for the "switch"

Re: Using defines for protocol characters

2023-08-07 Thread Peter Smith
Hi, I wondered if any consideration was given to using an enum instead of all the #defines. I guess, your patch would not be much different; you can still have all the nice names and assign the appropriate values to the enum values same as now, but using an enum you might also gain type-checking i

Re: Using defines for protocol characters

2023-08-04 Thread Dave Cramer
On Fri, 4 Aug 2023 at 03:44, Alvaro Herrera wrote: > On 2023-Aug-03, Dave Cramer wrote: > > > New patch attached which uses PREPARED_SUB_COMMAND and > > PORTAL_SUB_COMMAND instead > > Hmm, I would keep the prefix in this case and make the message type a > second prefix, with the subtype last -

Re: Using defines for protocol characters

2023-08-04 Thread Alvaro Herrera
On 2023-Aug-03, Dave Cramer wrote: > New patch attached which uses PREPARED_SUB_COMMAND and > PORTAL_SUB_COMMAND instead Hmm, I would keep the prefix in this case and make the message type a second prefix, with the subtype last -- PQMSG_CLOSE_PREPARED, PQMSG_DESCRIBE_PORTAL and so on. You def

Re: Using defines for protocol characters

2023-08-03 Thread Tatsuo Ishii
> On Thu, 3 Aug 2023 at 16:59, Tatsuo Ishii wrote: > >> > On Thu, 3 Aug 2023 at 13:25, Tatsuo Ishii wrote: >> > >> >> > Greetings, >> >> > >> >> > Attached is a patch which introduces a file protocol.h. Instead of >> using >> >> > the actual characters everywhere in the code this patch names the

Re: Using defines for protocol characters

2023-08-03 Thread Dave Cramer
On Thu, 3 Aug 2023 at 16:59, Tatsuo Ishii wrote: > > On Thu, 3 Aug 2023 at 13:25, Tatsuo Ishii wrote: > > > >> > Greetings, > >> > > >> > Attached is a patch which introduces a file protocol.h. Instead of > using > >> > the actual characters everywhere in the code this patch names the > >> > cha

Re: Using defines for protocol characters

2023-08-03 Thread Tatsuo Ishii
> On Thu, 3 Aug 2023 at 13:25, Tatsuo Ishii wrote: > >> > Greetings, >> > >> > Attached is a patch which introduces a file protocol.h. Instead of using >> > the actual characters everywhere in the code this patch names the >> > characters and removes the comments beside each usage. >> >> > +#defi

Re: Using defines for protocol characters

2023-08-03 Thread Dave Cramer
On Thu, 3 Aug 2023 at 15:22, Dave Cramer wrote: > > > On Thu, 3 Aug 2023 at 13:25, Tatsuo Ishii wrote: > >> > Greetings, >> > >> > Attached is a patch which introduces a file protocol.h. Instead of using >> > the actual characters everywhere in the code this patch names the >> > characters and r

Re: Using defines for protocol characters

2023-08-03 Thread Dave Cramer
On Thu, 3 Aug 2023 at 13:25, Tatsuo Ishii wrote: > > Greetings, > > > > Attached is a patch which introduces a file protocol.h. Instead of using > > the actual characters everywhere in the code this patch names the > > characters and removes the comments beside each usage. > > > +#define DESCRIBE

Re: Using defines for protocol characters

2023-08-03 Thread Tatsuo Ishii
> Greetings, > > Attached is a patch which introduces a file protocol.h. Instead of using > the actual characters everywhere in the code this patch names the > characters and removes the comments beside each usage. > +#define DESCRIBE_PREPARED 'S' > +#define DESCRIBE_PORTAL

Re: Using defines for protocol characters

2023-08-03 Thread Nathan Bossart
On Thu, Aug 03, 2023 at 12:07:21PM -0600, Dave Cramer wrote: > On Thu, 3 Aug 2023 at 11:59, Alvaro Herrera wrote: >> I don't really like the name pattern you've chosen though; I think we >> need to have a common prefix in the defines. Maybe prepending PQMSG_ to >> each name would be enough. And

Re: Using defines for protocol characters

2023-08-03 Thread Dave Cramer
On Thu, 3 Aug 2023 at 11:59, Alvaro Herrera wrote: > On 2023-Aug-03, Dave Cramer wrote: > > > Greetings, > > > > Attached is a patch which introduces a file protocol.h. Instead of using > > the actual characters everywhere in the code this patch names the > > characters and removes the comments b

Re: Using defines for protocol characters

2023-08-03 Thread Alvaro Herrera
On 2023-Aug-03, Dave Cramer wrote: > Greetings, > > Attached is a patch which introduces a file protocol.h. Instead of using > the actual characters everywhere in the code this patch names the > characters and removes the comments beside each usage. I saw this one last week. I think it's a very

Using defines for protocol characters

2023-08-03 Thread Dave Cramer
Greetings, Attached is a patch which introduces a file protocol.h. Instead of using the actual characters everywhere in the code this patch names the characters and removes the comments beside each usage. Dave Cramer 0001-Created-protocol.h.patch Description: Binary data