On Wed, 10 Jan 2024 at 22:12, Robert Haas <robertmh...@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.