On Wed, 10 Jan 2024 at 22:12, Robert Haas <robertmhaas@gmail.com> wrote:
> Could you explain why you think that the protocol version bump is necessary?
Patch 0006 adds a new protocol message to the protocol. Somehow the
client needs to be told that the server understands that message.
Using the protocol version seems like the best/simplest/cleanest way
to do that to me.
In theory we could add a dedicated protocol extension parameter (e.g.
_pq_.enable_protocol_level_set) that the client would need to set to
true before it would be able to use ParameterSet. But that just sounds
like introducing unnecessary complexity to me.
Bumping the protocol version carries exactly the same level of risk as
adding new protocol extension parameters. Both will always allow old
clients to connect to the newer server. And both also allow a new
client to connect to an old server just fine as well, as long as that
server has ae65f6066dc3d19a55f4fdcd3b30003c5ad8dbed (which was
introduced in PG11.0 and was backpatched to all then supported PG
versions).
> > It now limits the possible GUCs that can be changed to PGC_USERSET and
> > PGC_SUSET. If desired, we could limit it even further by using an
> > allowlist of reasonable GUCs or set a flag on GUCs that can be
> > "upgraded" . Things that seem at least reasonable to me are "role",
> > "session_authorization", "client_encoding".
>
> I don't know whether that limit helps anything or not, and you haven't
> really said why you imposed it.
The main reason I did this is to make sure that the required context
can only be hardened, not softened. e.g. it would be very bad if
PGC_POSTMASTER GUCs could suddenly be changed with a protocol message.
So it was more meant as fixing a bug, than really reducing the number
of GUCs this has an impact on significantly.
> I'm still afraid that
> trying to allow this kind of nail-down for a broad range of GUCs (even
> if not all) is going to be messy. But I'm also plenty willing to
> listen to contrary opinions. I hear yours, but I wonder what others
> think? Tom particularly.
I wouldn't mind heavily reducing the GUCs that can be nailed-down like
this. For my usecase (connection pooling) I only really care about it
being possible to nail-down session_authorization.
Honestly, I care more about patch 0010 than patch 0008. Patch 0008
simply seemed like the easiest way to demonstrate the ParameterSet
message.