Horton

Thread

Commits

  1. Reorganize pqcomm.h a bit

  1. [PATCH] Reorganize pqcomm.h a bit

    Jacob Champion <jacob.champion@enterprisedb.com> — 2025-11-21T19:10:12Z

    Hi,

I was reviewing a patchset by Jelte [1] and decided that pqcomm.h has
gotten hard to read/organize.

Attached is a patch that groups the magic PG_PROTOCOL() codes, adds a
comment to a typedef that was orphaned when protocol.h arrived, and
tweaks some whitespace to make the "paragraphs" easier to scan (IMHO).
I did have some misgivings about separating CANCEL_REQUEST_CODE from
its packet struct, but I think it's a net improvement.

WDYT?

--Jacob

[1] https://postgr.es/m/DDPR5BPWH1RJ.1LWAK6QAURVAY@jeltef.nl

  2. Re: [PATCH] Reorganize pqcomm.h a bit

    Jelte Fennema-Nio <postgres@jeltef.nl> — 2025-11-21T21:39:12Z

    On Fri, Nov 21, 2025, 12:10 Jacob Champion <jacob.champion@enterprisedb.com>
wrote:

>
> WDYT?
>

Overall seems like reasonable restructuring. I think this note feels out of
place now though:

* The cancel request code must not match any protocol version number
* we're ever likely to use. This random choice should do.

I think it'd be better to remove that paragraph and maybe extend the
section intro to be something like this (feel free to change/ignore as you
see fit):

Reserved protocol version numbers. These don't denote an actual protocol
version but instead have special semantics. These version numbers will
never be used as an actual protocol version.

Finally, the newline addition at line 71 I don't understand the purpose of.

>

  3. Re: [PATCH] Reorganize pqcomm.h a bit

    Jacob Champion <jacob.champion@enterprisedb.com> — 2025-11-21T22:01:19Z

    On Fri, Nov 21, 2025 at 1:39 PM Jelte Fennema-Nio <postgres@jeltef.nl> wrote:
> Overall seems like reasonable restructuring. I think this note feels out of place now though:
>
> * The cancel request code must not match any protocol version number
> * we're ever likely to use. This random choice should do.
>
> I think it'd be better to remove that paragraph and maybe extend the section intro to be something like this (feel free to change/ignore as you see fit):

Good point. I think "actual protocol version" might get a little
confusing for a casual reader if/when your _GREASE macro arrives,
though. I'll do some wordsmithing.

> Finally, the newline addition at line 71 I don't understand the purpose of.

This header file separates some sections with two empty lines (though
there appears to be no consistency), and if we're going to do that
anyway, then it scans easier IMO to have the PG_PROTOCOL section set
off from the socket section preceding it. I did the same thing for
some of the typedefs and ALPN code later on, though I'm not wedded to
that (or any of it) if it makes things worse for others.

Thanks,
--Jacob

  4. Re: [PATCH] Reorganize pqcomm.h a bit

    Jacob Champion <jacob.champion@enterprisedb.com> — 2025-11-22T00:25:11Z

    On Fri, Nov 21, 2025 at 2:01 PM Jacob Champion
<jacob.champion@enterprisedb.com> wrote:
> Good point. I think "actual protocol version" might get a little
> confusing for a casual reader if/when your _GREASE macro arrives,
> though. I'll do some wordsmithing.

Actually, maybe brevity is best, as in v2? They're reserved, they're
special; readers can look at the comment for the reservation in
question to find out why, and then we don't have to generalize in the
section header.

--Jacob

  5. Re: [PATCH] Reorganize pqcomm.h a bit

    Jelte Fennema-Nio <postgres@jeltef.nl> — 2025-11-22T01:53:16Z

    On Fri, Nov 21, 2025, 17:25 Jacob Champion <jacob.champion@enterprisedb.com>
wrote:

> On Fri, Nov 21, 2025 at 2:01 PM
>
> Actually, maybe brevity is best, as in v2?
>

Looks fine to me

>

  6. Re: [PATCH] Reorganize pqcomm.h a bit

    Heikki Linnakangas <hlinnaka@iki.fi> — 2025-11-24T09:33:32Z

    On 22/11/2025 03:53, Jelte Fennema-Nio wrote:
> On Fri, Nov 21, 2025, 17:25 Jacob Champion 
> <jacob.champion@enterprisedb.com 
> <mailto:jacob.champion@enterprisedb.com>> wrote:
> 
>     On Fri, Nov 21, 2025 at 2:01 PM
> 
>     Actually, maybe brevity is best, as in v2?
> 
> 
> Looks fine to me

+1

- Heikki

  7. Re: [PATCH] Reorganize pqcomm.h a bit

    Jacob Champion <jacob.champion@enterprisedb.com> — 2025-11-24T20:31:04Z

    On Mon, Nov 24, 2025 at 1:33 AM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
> On 22/11/2025 03:53, Jelte Fennema-Nio wrote:
> > Looks fine to me
>
> +1

Committed; thank you both!

--Jacob