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-28 09:42:13
Message-ID: CAGECzQSg0ND=gRmpZHBsiaZT5-a17y=VFf3AQfi5N7oECtM1Ug@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sat, 27 Jan 2024 at 20:44, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
> client_encoding, standard_conforming_strings, server_version, default_transaction_read_only, in_hot_standby and scram_iterations
> are used by libpq directly, so it can be wrong to introduce the possibility to break it.

libpq could add these ones automatically to the list, just like a
proxy. But I think you are probably right and always reporting our
current default set is probably easier.

>> Maybe I'm misunderstanding what you're saying, but it's not clear to
>> me why you are seeing two different use cases here. To me if we have
>> the ParameterSet message then they are both the same. When you enable
>> %N you would send a ParamaterSet message for _pq_.report_parameters
>> and set it to a list of gucs including "role". And when you disable %N
>> you would set _pq_.report_parameters to a list of GUCs without "role".
>> Then if any proxy still needed "role" even if the list it receives in
>> _pq_.report_parameters doesn't contain it, then this proxy would
>> simply prepend "role" to the list of GUCs before forwarding the
>> ParameterSet message.
>
>
> Your scenario can work but looks too fragile. I checked - GUC now cannot contain some special chars, so writing parser should not be hard work. But your proposal means the proxy should be smart about it, and have to check any change of _pq_.report_parameters, and this point can be fragile and a source of hardly searched bugs.

Yes, proxies should be smart about it. But if there's new message
types introduced specifically for this, then proxies need to be smart
about it too. Because they would need to remember which reporting was
requested by the client, to be able to correctly ask for reporting
GUCs it after server connection . Using GUCs actually makes this
easier to implement (and thus less error prone), because proxies
already have logic to re-sync GUCs after connection assignment.

I think this is probably one of the core reasons why I would very much
prefer GUCs over new message types to configure protocol extensions
like this: It means proxies would not to keep track of and re-sync a
new kind of connection state every time a protocol extension is added.
They can make their GUC tracking and re-syncing robust, and that's all
they would need.

> This is true, but how common is this situation? Probably every client that uses one proxy will use the same defaultly reported GUC.

If you have different clients connecting to the same proxy, it seems
quite likely that this will happen. This does not seem uncommon to me,
e.g. actual application would need different things always reported
than some dev client. Or clients for different languages might ask to
report slightly different settings.

> Reporting has no extra overhead. The notification is reduced. When there is a different set of reported GUC, then the proxy can try to find another connection with the same set or can reconnect.

Honestly, this logic seems much more fragile to implement. And
requiring reconnection seems problematic from a performance point of
view.

> I think so there is still opened question what should be correct behaviour when client execute RESET ALL or DISCARD ALL. Without special protection the communication with proxy can be broken - and we use GUC for reported variables, then my case, prompt in psql will be broken too. Inside psql I have not callback on change of reported GUC. So this is reason why reporting based on mutable GUC is fragile :-/

Specifically for this reason, the current patchset in the other thread
already ignores RESET ALL and DISCARD ALL for protocol extension
parameters (including _pq_.report_parameters). So this would be a
non-issue.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jelte Fennema-Nio 2024-01-28 09:51:48 Re: [EXTERNAL] Re: Add non-blocking version of PQcancel
Previous Message Alena Rybakina 2024-01-28 09:07:46 Re: Evaluate arguments of correlated SubPlans in the referencing ExprState