Re: proposal: psql: show current user in prompt

pá 26. 1. 2024 v 11:40 odesílatel Jelte Fennema-Nio <postgres(at)jeltef(dot)nl>
napsal:

> On Thu, 25 Jan 2024 at 21:43, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
> wrote:
> > 2. using GUC for all reported GUC looks not too readable. Maybe it
> should be used just for customized reporting, not for default
>
> I thought about this too, because indeed the default list is quite
> long. But I decided against it because it seemed strange that you
> cannot disable reporting for the options that are currently reporting
> by default. Especially since the current default essentially boils
> down to: "whatever the psql client needed"
>

I see a possibility of disabling reporting as possibly dangerous. Without
reporting encoding you can break psql. So it should be limited just to few
values where is known behave.

> > 3. Another issue of your proposal is less friendly enabling disabling
> reporting of specific GUC. Maintaining a list needs more work than just
> enabling and disabling one specific GUC.
> > I think this is the main disadvantage of your proposal. In my proposal I
> don't need to know the state of any GUC. Just I send PQlinkParameterStatus
> or PQunlinkParameterStatus. With your proposal, I need to read
> _pq_.report_parameters, parse it, and modify, and send it back. This
> doesn't look too practical.
>
> While I agree it's a little bit less friendly, I think you're
> overestimating the difficulty of using my proposed approach. Most
> importantly there's no need to parse the current GUC value. A client
> always knows what variables it wants to have reported. So anytime that
> changes the client can simply regenerate the full list of gucs that it
> wants to report and send that. So something similar to the following
> pseudo code (using += for string concatenation):
>

I disagree with this - I can imagine some proxies add some own reported GUC
and the client can know nothing about it.

>
> char *report_parameters = "server_version,client_encoding"
> if (show_user_in_prompt)
> report_parameters += ",user"
> if (show_search_path_in_prompt)
> report_parameters += ",search_path"
> PQsetParameter("_pq_.report_parameters", report_parameters)
>

It is simple only when default value of report_parameters is constant, but
when not it can be fragile

>
> > Personally I prefer usage of a more generic API than my
> PQlinkParameterStatus and PQunlinkParameterStatus. You talked about it with
> Robert If I remember.
> >
> > Can be nice if I can write just
> >
> > /* similar princip is used inside ncurses */
> > set_report_guc_message_no = PQgetMessageNo("set_report_guc");
> > /* the result can be processed by server and by all proxies on the line
> */
> >
> > if (set_report_guc_message_no == -1)
> > fprintf(stderr, "feature is not supported");
> > result = PQsendMessage(set_report_guc_message, "user");
> > if (result == -1)
> > fprintf(stderr, "some error ...");
> >
> > With some API like this it can be easier to do some small protocol
> enhancement. Maybe this is overengineering. Enhancing protocol is not too
> common, and with usage PQsendTypedCommand enhancing protocol is less work
> too.
>
> Honestly, I don't see a benefit to this over protocol extension
> parameters using the ParameterSet message. Afaict this is also
> essentially a key + value message type. And protocol extension
> parameters have the benefit that they are already an established thing
> in the protocol, and that they can easily be integrated with the
> current GUC system.
>

It was an idea.

Personally I like an idea that I described in mail to Peter. Using
dedicated connection related GUC for immutably reported GUC, and using
possibility to set dedicated function in protocol for enabling, disabling
other GUC. It looks (to me) like the most robust solution.

Regards

Pavel