Horton

Re: Add new protocol message to change GUCs for usage with future protocol-only GUCs

Heikki Linnakangas <hlinnaka@iki.fi>

View thread
From: Heikki Linnakangas <hlinnaka@iki.fi>
To: Jelte Fennema-Nio <postgres@jeltef.nl>, Robert Haas <robertmhaas@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Jacob Burroughs <jburroughs@instructure.com>, Dave Cramer <davecramer@gmail.com>, Peter Smith <smithpb2250@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, "Andrey M. Borodin" <x4mmm@yandex-team.ru>, Pavel Stehule <pavel.stehule@gmail.com>, Jeff Davis <pgsql@j-davis.com>, Peter Eisentraut <peter@eisentraut.org>
Date: 2024-08-15T22:03:57Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. libpq: Add min/max_protocol_version connection options

  2. libpq: Handle NegotiateProtocolVersion message differently

  3. Add PQfullProtocolVersion() to surface the precise protocol version.

  4. Do not hardcode PG_PROTOCOL_LATEST in NegotiateProtocolVersion

  5. libpq: Handle NegotiateProtocolVersion message

  6. Provide for forward compatibility with future minor protocol versions.

On 14/08/2024 21:04, Jelte Fennema-Nio wrote:
> On Wed, 7 Aug 2024 at 22:10, Robert Haas <robertmhaas@gmail.com> wrote:
>> I respect that, but I don't want to get flamed for doing something
>> that might be controversial without anybody else endorsing it. I'll
>> commit this if it gets some support, but not otherwise. I'm willing to
>> commit a patch adding a new function even if nobody else votes, but
>> not this.
> 
> Makes sense. I'm not in too much of a hurry with this specific one. So
> I'll leave it like this for now and hopefully someone else responds.
> If this becomes close to being the final unmerged patch of this
> patchset, I'll probably cut my losses and create a patch that adds a
> function instead.

I think Jelte's proposal on PQprotocolVersion() is OK. As he pointed 
out, the function is pretty useless as it is, so I doubt anyone is doing 
anything interesting with it. Perhaps we should even change it to return 
300000 for protocol version 3.0, and just leave a note in the docs like 
"in older versions of libpq, this returned 3 for protocol version 3.0".

On Wed, 7 Aug 2024 at 22:10, Robert Haas <robertmhaas@gmail.com> wrote:
>> > > Patch 7: Bump the protocol version to 3.2 (see commit message for why
>> > > not bumping to 3.1)
>> >
>> > Good commit message. The point is arguable, so putting forth your best
>> > argument is important.
>>
>> Just to clarify: do you agree with the point now, after that argument?
> 
> Well, here again, I would like to know what other people think. It
> doesn't intrinsically matter to me that much what we do here, but it
> matters to me a lot that extensive recriminations don't ensue
> afterwards.

Makes sense to me. It's sad that pgbouncer had such a bug, but it makes 
sense to accommodate it. We're not going to run out of integers. This 
deserves some more commentary in the docs I think. If I understand the 
plan correctly, if the client requests version 3.1, the server accepts 
it, but behaves exactly the same as 3.0. Or should 3.1 be forbidden 
altogether?

On the default for "max_protocol_version": I'm pretty disappointed if we 
cannot change the default to "latest". I realize that that won't work 
with poolers that don't implement NegotiateProtocolVersion. But I'm 
afraid if we make the new protocol version opt-in, no one will use it, 
and the poolers etc still won't bother to implement 
NegotiateProtocolVersion for years to come, if ever. We can revisit this 
decision later in the release cycle though. But I'd suggest changing the 
default to "latest" for now, so that more hackers working with 
connection poolers will notice, and we get more testing of the new 
protocol and the negotiation.

-- 
Heikki Linnakangas
Neon (https://neon.tech)