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

On Tue, 2 Jan 2024 at 18:51, Robert Haas <robertmhaas(at)gmail(dot)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.