Horton

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

Jelte Fennema-Nio <postgres@jeltef.nl>

View thread
From: Jelte Fennema-Nio <postgres@jeltef.nl>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Jacob Champion <jacob.champion@enterprisedb.com>, Heikki Linnakangas <hlinnaka@iki.fi>, 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>, Daniele Varrazzo <daniele.varrazzo@gmail.com>
Date: 2024-08-19T20:53:46Z
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 Mon, 19 Aug 2024 at 16:16, Robert Haas <robertmhaas@gmail.com> wrote:
> If somebody is using PQprotocolVersion() to detect the arrival of a
> new protocol version, it stands to reason that they only care about
> new major protocol versions, because that's what the function is
> defined to tell you about. Anyone who has done a moderate amount of
> looking into this area will understand that the protocol has a major
> version number and a minor version number and that this function only
> returns the former. Therefore, they should expect that the arrival of
> a new minor protocol version won't change the return value of this
> function.

What I'm trying to say is: I don't think there's any usecase where
people would care about a major bump, but not a minor bump. Especially
keeping in mind that a minor bump had never occurred when originally
creating this function. And because we never did it, there has so far
been no definition of what is the actual difference between a major
and a minor bump.

> I really don't understand why we're still arguing about this. It seems
> to me that we've established that there is some usage of the existing
> function, and that changing the return value will break something.
> Sure, so far as we know that something is "only" regression tests, but
> there's no guarantee that there couldn't be other code that we don't
> know about that breaks worse

My point is that the code that breaks, actually wants to be broken in this case.

> and even there isn't, who wants to break
> regression tests when there's nothing actually wrong?

Updating the regression test would be less work than adding support
for a new API. So if the main problem is

> Now we could
> decide we're going to do it anyway because of whatever reason we might
> have, but it doesn't seem like that's what most people want to do.
>
> I feel like we're finally in a position to get some things done here
> and this doesn't seem like the point to get stuck on. YMMV, of course.

I'd love to hear a response from Jacob and Heikki on my arguments
after their last response. But if after reading those arguments they
still think we should add a new function, I'll update the patchset to
include a new function.