Re: proposal: psql: show current user in prompt

From: Jelte Fennema-Nio <postgres(at)jeltef(dot)nl>
To: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
Cc: Peter Eisentraut <peter(at)eisentraut(dot)org>, Robert Haas <robertmhaas(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Kirk Wolak <wolakk(at)gmail(dot)com>, Corey Huinker <corey(dot)huinker(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: proposal: psql: show current user in prompt
Date: 2024-01-26 10:40:00
Message-ID: CAGECzQSrNAJ5HZVu_WwzbGHDAEKVymaCXjHnC9axLdSKSOz5yQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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"

> 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):

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 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.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2024-01-26 10:41:04 Re: Printing backtrace of postgres processes
Previous Message Kurlaev Jaroslav 2024-01-26 10:38:50 Finding every use of a built-in function