Re: Add new protocol message to change GUCs for usage with future protocol-only GUCs - Mailing list pgsql-hackers

On Tue, 16 Jan 2024 at 21:36, Robert Haas <robertmhaas@gmail.com> wrote:
> Sorry for not getting back to this right away; there are quite a few
> threads competing for my attention.

No worries, I know it's a busy time.

> But I think we would want to have those changes in all supported
> branches before we could think of requesting 3.1 or higher by default.
> Imagine that in v17 we both added server support for protocol version
> 3.1 and also adopted your 0001. Then, a v17 libpq would fail to
> connect to a v16 or earlier PostgreSQL instance. In effect, it would
> be a complete wire compatibility break. There's no way that such a
> patch is going to be acceptable. If I were to commit a patch from you
> or anyone else that does that, many angry people would show up to yell
> at both of us. So unless I'm misunderstanding the situation, 0001 is
> pretty much dead on arrival for now and for quite a while to come.

It's understandable you're worried about breaking clients, but afaict
**you are actually misunderstanding the situation**. I totally agree
that we cannot bump the protocol version without also merging 0002
(that's why the version bump is in patch 0005 not patch 0001). But
0002 does **not** need to be backported to all supported branches. The
only libpq versions that can ever receive a NegotiateVersionProtocol
are ones that request 3.1, and since none of the pre-17 libpq versions
ever request 3.1 there's no need for them to be able to handle
NegotiateVersionProtocol.

If you try out the patchset, you will see that you can connect with
psql16 to postgres17 and psql17 to postgres16. Both without any
problems. The only time when you will get problems is if you connect
to a server from before these versions that you mentioned (2017-11-21,
11.0, 10.2, 9.6.7, 9.5.11, 9.4.16, 9.3.21)

> Also, I'm pretty doubtful that we want
> PQunsupportedProtocolExtensions().

I definitely think we should include this API. As a client author (and
even user), you might want to know what features are supported by the
server you are connected to. That way you can avoid calling functions
that would otherwise fail. This is also the reason why
PQprotocolVersion() and PQserverVersion() exist. IMHO
PQunsupportedProtocolExtensions() is simply a natural addition to
those already existing feature-discovery APIs.

I'll move the addition of PQunsupportedProtocolExtensions() to a
separate patch though, since I do agree that it's a separate item from
the rest of 0002.

> That seems like allowing the client
> to have too much direct visibility into what's happening at the
> protocol level. That kind of interface might make sense if we were
> trying to support unknown protocol extensions from third parties, but
> for stuff in core, I think there should be specific APIs that relate
> to specific features e.g. you call PQsetWireProtocolRole(char
> *whatever) and it returns success or failure according to whether that
> capability is available without telling you how that's negotiated on
> the wire.

I think we have a very different idea of what is a desirable API for
client authors that use libpq to build their clients. libpq its API is
pretty low level, so I think it makes total sense for client authors
to know what protocol extension parameters exist. It seems totally
acceptable for me to have them call PQsetParameter directly:

PQsetParameter("_pq_.protocol_roles", "true")
PQsetParameter("_pq_.report_parameters", "role,search_path")

Otherwise we need to introduce **two** new functions for every
protocol extension that is going to be introduced, a blocking and a
non-blocking one (e.g. PQsetWireProtocolRole() and
PQsendSetWireProtocolRole()). And that seems like unnecessary API
bloat to me.

To be clear, I think it would probably make sense for client authors
to expose functions like that for the users of the client. But I think
libpq should not add an API surface that can easily be avoided (e.g.
there's also no function to begin a transaction, even though pretty
much every client exposes one).