On Fri, May 24, 2024 at 6:09 PM Jelte Fennema-Nio <m...@jeltef.nl> wrote: > 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.
I think that separating (1) and (3) is going to make us happy rather than sad. For example, if a client speaks to an intermediate pooler which speaks to a server, the client-pooler connection could have different values from the pooler-server connection, and then if everything is done via ParameterStatus messages it's actually more complicated for the pooler, which will have to edit ParameterStatus messages as they pass through, and possibly also create new ones out of nothing. If we separate things, the pooler can always pass ParameterStatus messages unchanged, and only has to worry about the separate infrastructure for handling these new things. Said differently, I'd agree with you if (a) ParameterStatus weren't so dubiously designed and (b) all poolers were going to want every protocol parameter to match on the input side and the output side. And maybe in practice (b) will happen, but I want to leave the door open to cases where it doesn't, and as soon as that happens, I think this becomes a hassle, whereas separate mechanisms don't really hurt much. As you and I discussed a bit in person last week, two for loops rather than one in the pooler isn't really that big of a deal. IMHO, that's a small price to pay for an increased chance of not boxing ourselves into a corner depending on how these parameters end up getting used in practice. As for (2) and (4), I agree there's some duplication, but I think it's quite minor. We have existing facilities for parsing integers and booleans that are reused by a lot of code already, and this is just one more place that can use them. That infrastructure is not GUC-specific. The permissions-checking stuff isn't either. The vast bulk of the GUC infrastructure is concerned with (1) allowing for GUCs to be set from many different sources and (2) handling their transactional nature. We don't need or want either of those characteristics here, so a lot of that complexity just isn't needed. -- Robert Haas EDB: http://www.enterprisedb.com
