Re: proposal: psql: show current user in prompt - Mailing list pgsql-hackers

From Jelte Fennema-Nio
Subject Re: proposal: psql: show current user in prompt
Date
Msg-id CAGECzQSrNAJ5HZVu_WwzbGHDAEKVymaCXjHnC9axLdSKSOz5yQ@mail.gmail.com
Whole thread Raw
In response to Re: proposal: psql: show current user in prompt  (Pavel Stehule <pavel.stehule@gmail.com>)
Responses Re: proposal: psql: show current user in prompt
List pgsql-hackers
On Thu, 25 Jan 2024 at 21:43, Pavel Stehule <pavel.stehule@gmail.com> wrote:
> 2. using GUC for all reported GUC looks not too readable. Maybe it should be used just for customized reporting, not
fordefault 

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"

> 3. Another issue of your proposal is less friendly enabling disabling reporting of specific GUC. Maintaining a list
needsmore 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.
JustI send PQlinkParameterStatus or PQunlinkParameterStatus. With your proposal, I need to read _pq_.report_parameters,
parseit, 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):

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)

> Personally I prefer usage of a more generic API than my PQlinkParameterStatus and PQunlinkParameterStatus. You talked
aboutit 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.
Enhancingprotocol 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.



pgsql-hackers by date:

Previous
From: Kurlaev Jaroslav
Date:
Subject: Finding every use of a built-in function
Next
From: Alvaro Herrera
Date:
Subject: Re: Printing backtrace of postgres processes