Horton

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

Jelte Fennema-Nio <me@jeltef.nl>

View thread
From: Jelte Fennema-Nio <me@jeltef.nl>
To: 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-05-24T22:08: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 Fri, 24 May 2024 at 15:28, Robert Haas <robertmhaas@gmail.com> wrote:
>
> On Thu, May 23, 2024 at 4:00 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > I don't recall exactly what I thought earlier, but now I think we'd
> > be better off with separate infrastructure.  guc.c is unduly complex
> > already.  Perhaps there are bits of it that could be factored out
> > and shared, but I bet not a lot.
>
> OK. That seems fine to me, but I bet Jelte is going to disagree.

I indeed disagree. I think the effort needed to make guc.c handle
protocol parameters is extremely little. The 0011 patch are all the
changes that are needed to achieve that, and that patch only adds 65
additional lines. And only 15 of those 65 lines actually have to do
anything somewhat weird, to be able to handle the transactionality
discrepancy between protocol parameters and other GUCs. The other 50
lines are (imho) really clean and fit perfectly with the way guc.c is
currently structured (i.e. they add PGC_PROTOCOL and PGC_SU_PROTOCOL
in a really obvious way)

Separating it from the GUC infrastructure will mean we need to
duplicate a lot of the infrastructure. Assuming we don't care about
SHOW or pg_settings (which I agree are not super important), the
things that we would want for protocol parameters to have that guc.c
gives us for free are:
1. Reporting the value of the parameter to the client (done using
ParameterStatus)
2. Parsing and validating of the input, bool, int, enum, etc, but also
check_hook and assign_hook.
3. Logic in all connection poolers to change GUC values to the
client's expected values whenever a server connection is handed off to
a client
4. Permission checking, if we want some protocol extensions to only be
configurable by a highly privileged user

All of those things would have to be duplicated/re-implemented if we
make protocol parameters their own dedicated thing. Doing that work
seems like a waste of time to me, and would imho add much more
complexity than the proposed 65 lines of code in 0011.