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
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.
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
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,
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
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
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
> 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
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
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
> 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
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
> 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
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
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
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
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
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
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
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
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
> 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
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
> 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
+#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
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
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
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
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
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
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
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
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
> >
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"
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
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 -
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
> 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
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
> 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
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
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
> 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
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
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
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
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
47 matches
Mail list logo