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

On Tue, Jan 16, 2024 at 9:21 PM Jelte Fennema-Nio <me@jeltef.nl> wrote:
> 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.

OK, yeah, fuzzy thinking on my part.

> 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.

I think it's hard to say for sure what API is going to work well here,
because we just don't have much experience with this. I do agree that
we want to avoid API bloat. However, I also think that the reason why
the API you're proposing here looks good in this case is because libpq
itself doesn't really need to do anything differently for these
parameters. It doesn't actually really change anything about the
protocol; it only nails down the server behavior in a way that can't
be changed. Another current request is to have a way to have certain
data types always be sent in binary format, specified by OID. Do we
want that to be written as PQsetParameter("always_binary_format",
"123,456,789,101112") or do we want it to maybe look more like
PQalwaysBinaryFormat(int count, Oid *stuff)? Or, another example, say
we want to set client_to_server_compression_method=lz4. It's very
difficult for me to believe that libpq should be doing strcmp()
against the proposed protocol parameter settings and thus realizing
that it needs to start compressing ... especially since there might be
compression parameters (like level or degree of parallelism) that the
client needs and the server doesn't.

Also, I never intended for _pq_ to become a user-visible namespace
that people would have to care about; that was just a convention that
I adopted to differentiate setting a protocol parameter from setting a
GUC. I think it's a mistake to make that string something users have
to worry about directly. It wouldn't begin and end with an underscore
if it were intended to be user-visible.

--
Robert Haas
EDB: http://www.enterprisedb.com