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

On Wed, 7 Aug 2024 at 22:10, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> I took another look at 0002 today:

Applied all 0002 feedback. Although I used Min(proto,
PG_PROTOCOL_LATEST) because Max results in the wrong value.

> I respect that, but I don't want to get flamed for doing something
> that might be controversial without anybody else endorsing it. I'll
> commit this if it gets some support, but not otherwise. I'm willing to
> commit a patch adding a new function even if nobody else votes, but
> not this.

Makes sense. I'm not in too much of a hurry with this specific one. So
I'll leave it like this for now and hopefully someone else responds.
If this becomes close to being the final unmerged patch of this
patchset, I'll probably cut my losses and create a patch that adds a
function instead.

> Not entirely. The documentation of the environment variable gets the
> name of the environment variable wrong.

Oops... fixed

> > > I wonder whether we could define 3.2 to report on all supported
> > > protocol parameters even if they weren't in the startup message, to
> > > avoid having to jam a lot of stuff we don't really care about into the
> > > startup message.
> >
> > I think that's a good idea. I'll try to look into doing that soonish.
>
> Is this something that you still intend to do? It looks to me like
> this would change some things as early as 0006.

Yeah I still intend to do this, but haven't found the time yet. For
now I've moved all the protocol parameter stuff together in 0008
(which still needs extra thinking. The only thing that 0006 now does
is handle different protocol versions better as well as applying your
other 0006 feedback, but it still errors for all protocol parameters.

> I guess I'm also a little unsure whether we need to deal with both
> NULL and "" here. Are they semantically different? Is it reasonable to
> try to prevent NULL from occurring in the first place? Or maybe it's
> fine the way it is. Not sure.

NULL happens when report_parameters is not set at all by the user, and
"" happens when the connection string contains report_parameters=''.

It's possible to prevent NULL from occurring, by setting the
"compiled" field to the empty string in the PQconninfoOptions array.
But this means that an extra strdup will be done for every PGconn that
is created to copy this empty string. None of the other entries in
PQconninfoOptions use an empty string for the "compiled" field, all
use NULL. So I think it's best to use NULL for these protocol
parameters too. Which then in turn means that we have to check for
both values.

> I also happened to notice that 0009 has a typo: report_paramters.

Fixed