On Tue, 2 Jan 2024 at 18:51, Robert Haas <robertmh...@gmail.com> wrote: > > On the whole, this feels like you are trying to force some things > > into the GUC model that should not be there. I do perceive that > > there are things that could be protocol-level variables, but > > trying to say they are a kind of GUC seems like it will create > > more problems than it solves. > > This is not a bad thought. If we made the things that were settable > with this mechanism distinct from the set of things that are settable > as GUCs, that might work out better. For example, it completely the > objection to (3). But I'm not 100% sure, either.
It seems like the general sentiment in the thread is that protocol parameters are different enough from GUCs that they should be handled separately. And I think I agree. Partially because of the transactional reasons mentioned upthread, but also because allowing to change defaults of protocol parameters in postgresql.conf seems like a really bad idea. If the client does not specify the protocol parameter, then they almost certainly want whatever value the default has been for ages. Giving a DBA the ability to change that seems like a recipe to accidentally break many clients. It does cause some new questions for this patchset though: - Since we currently don't have any protocol parameters. How do I test it? Should I add a debug protocol parameter specifically for this purpose? Or should my tests just always error at the moment? - If I create a debug protocol parameter, what designs should it inherit from GUCs? e.g. parsing and input validation sounds useful. And where should it be stored e.g. protocol_params.c? - How do you get the value of a protocol parameter status? Do we expect the client to keep track of what it has sent? Do we always send a ParameterStatus message whenever it is changed? Or should we add a ParamaterGet protocol message too? > > > 4. Have an easy way to use the value contained in a ParameterStatus > > > message as the value for a GUC > > > > I agree that GUC_LIST_QUOTE is a mess, but "I'm too lazy to deal > > with that" seems like a rather poor argument for instead expending > > the amount of labor involved in a protocol change. > > Not sure about this one. It seems unwarranted to introduce an > accusation of laziness when someone is finally making the effort to > address what is IMV a pretty serious deficiency in our current > implementation, but I have no educated opinion about what if anything > ought to be done about GUC_LIST_QUOTE or how that relates to the > present effort. To clarify, the main thing that I want to do is take the value from ParameterStatus and somehow, without having to escape it, set that value for that GUC for a different session. As explained above, I now think that this newly proposed protocol message is a bad fit for this. But I still think that that is not a weird thing to want. The current situation is that you get a value in ParameterStatus, but before it's useful you first need to do some escaping. And to know how to escape it requires you to maintain a hardcoded list of GUCs that are GUC_LIST_QUOTE (which might change in the next Postgres release). I see two options to address this: 1. Add another protocol message that sets GUCs instead of protocol parameters (which would behave just like SET, i.e. it would be transactional) 2. Support preparing "SET search_path = $1" and then bind a single string to it. i.e. have this PSQL command not fail with a syntax error: > SET search_path = $1; \bind '"user", public'; ERROR: 42601: syntax error at or near "$1" LINE 1: SET search_path = $1; I'll take a look at implementing option 2, since I have a feeling that's less likely to be controversial.
