proposal: psql: show current user in prompt

On Fri, Feb 3, 2023 at 9:56 AM Pavel Stehule <pavel.stehule@gmail.com>
wrote:

Hi

one visitor of p2d2 (Prague PostgreSQL Developer Day) asked if it is
possible to show the current role in psql's prompt. I think it is not
possible, but fortunately (with some limits) almost all necessary work is
done, and the patch is short.

In the assigned patch I implemented a new prompt placeholder %N, that
shows the current role name.

(2023-02-03 15:52:28) postgres=# \set PROMPT1 '%n as %N at '%/%=%#
pavel as pavel at postgres=#set role to admin;
SET
pavel as admin at postgres=>set role to default;
SET
pavel as pavel at postgres=#

Comments, notes are welcome.

Regards

Pavel

This patch is cluttered with the BACKEND_PID patch and some guc_tables.c
stuff that I don't think is related.

We'd have to document the %N.

I think there is some value here for people who have to connect as several
different users (tech support), and need a reminder-at-a-glance whether
they are operating in the desired role. It may be helpful in audit
documentation as well.

pá 3. 2. 2023 v 20:42 odesílatel Corey Huinker <corey.huinker@gmail.com>
napsal:

On Fri, Feb 3, 2023 at 9:56 AM Pavel Stehule <pavel.stehule@gmail.com>
wrote:

Hi

one visitor of p2d2 (Prague PostgreSQL Developer Day) asked if it is
possible to show the current role in psql's prompt. I think it is not
possible, but fortunately (with some limits) almost all necessary work is
done, and the patch is short.

In the assigned patch I implemented a new prompt placeholder %N, that
shows the current role name.

(2023-02-03 15:52:28) postgres=# \set PROMPT1 '%n as %N at '%/%=%#
pavel as pavel at postgres=#set role to admin;
SET
pavel as admin at postgres=>set role to default;
SET
pavel as pavel at postgres=#

Comments, notes are welcome.

Regards

Pavel

This patch is cluttered with the BACKEND_PID patch and some guc_tables.c
stuff that I don't think is related.

I was little bit lazy, I am sorry. I did it in one my experimental branch.
Both patches are PoC, and there are not documentation yet. I will separate
it.

We'd have to document the %N.

I think there is some value here for people who have to connect as several
different users (tech support), and need a reminder-at-a-glance whether
they are operating in the desired role. It may be helpful in audit
documentation as well.

yes, I agree so it can be useful - it is not my idea - and it is maybe
partially deduced from some other databases.

Both patches are very simple - and they use almost already prepared
infrastructure.

Regards

Pavel

Pavel Stehule <pavel.stehule@gmail.com> writes:

Both patches are very simple - and they use almost already prepared
infrastructure.

It's not simple at all to make the psql feature depend on marking
"role" as GUC_REPORT when it never has been before. That will
cause the feature to misbehave when using older servers. I'm
even less impressed by having it fall back on PQuser(), which
would be misleading at exactly the times when it matters.

regards, tom lane

pá 3. 2. 2023 v 21:21 odesílatel Tom Lane <tgl@sss.pgh.pa.us> napsal:

Pavel Stehule <pavel.stehule@gmail.com> writes:

Both patches are very simple - and they use almost already prepared
infrastructure.

It's not simple at all to make the psql feature depend on marking
"role" as GUC_REPORT when it never has been before. That will
cause the feature to misbehave when using older servers. I'm
even less impressed by having it fall back on PQuser(), which
would be misleading at exactly the times when it matters.

It is a good note. This can be disabled for older servers, and maybe it
can introduce its own GUC (and again - it can be disallowed for older
servers).

My goal at this moment is to get some prototype. We can talk if this
feature request is valid or not, and we can talk about implementation.

There is another possibility to directly execute "select current_user()"
instead of looking at status parameters inside prompt processing. It can
work too.

Regards

Pavel

Hi

pá 3. 2. 2023 v 21:43 odesílatel Pavel Stehule <pavel.stehule@gmail.com>
napsal:

pá 3. 2. 2023 v 21:21 odesílatel Tom Lane <tgl@sss.pgh.pa.us> napsal:

Pavel Stehule <pavel.stehule@gmail.com> writes:

Both patches are very simple - and they use almost already prepared
infrastructure.

It's not simple at all to make the psql feature depend on marking
"role" as GUC_REPORT when it never has been before. That will
cause the feature to misbehave when using older servers. I'm
even less impressed by having it fall back on PQuser(), which
would be misleading at exactly the times when it matters.

It is a good note. This can be disabled for older servers, and maybe it
can introduce its own GUC (and again - it can be disallowed for older
servers).

Here is another version. For older servers it shows the string ERR0A000.
That is ERR code of "feature is not supported"

My goal at this moment is to get some prototype. We can talk if this
feature request is valid or not, and we can talk about implementation.

There is another possibility to directly execute "select current_user()"
instead of looking at status parameters inside prompt processing. It can
work too.

I tested using the query SELECT CURRENT_USER, but I don't think it is
usable now, because it doesn't work in the broken transaction.

Regards

Pavel

On Sat, Feb 4, 2023 at 3:33 PM Pavel Stehule <pavel.stehule@gmail.com>
wrote:

Hi

pá 3. 2. 2023 v 21:43 odesílatel Pavel Stehule <pavel.stehule@gmail.com>
napsal:

pá 3. 2. 2023 v 21:21 odesílatel Tom Lane <tgl@sss.pgh.pa.us> napsal:

Pavel Stehule <pavel.stehule@gmail.com> writes:

Both patches are very simple - and they use almost already prepared
infrastructure.

It's not simple at all to make the psql feature depend on marking
"role" as GUC_REPORT when it never has been before. That will
cause the feature to misbehave when using older servers. I'm
even less impressed by having it fall back on PQuser(), which
would be misleading at exactly the times when it matters.

It is a good note. This can be disabled for older servers, and maybe it
can introduce its own GUC (and again - it can be disallowed for older
servers).

Here is another version. For older servers it shows the string ERR0A000.
That is ERR code of "feature is not supported"

My goal at this moment is to get some prototype. We can talk if this
feature request is valid or not, and we can talk about implementation.

There is another possibility to directly execute "select current_user()"
instead of looking at status parameters inside prompt processing. It can
work too.

I tested using the query SELECT CURRENT_USER, but I don't think it is
usable now, because it doesn't work in the broken transaction.

Regards

Pavel

Regards

Pavel

regards, tom lane

I've tested this w/regards to psql. Latest commit.
It works as described. 'none' is displayed for the default role. (SET ROLE
DEFAULT), otherwise the specific ROLE is displayed.

I tried this patch on 15.2, but guc_files.c does not exist in that version,
so it did not install.
I also tried applying the %T patch, but since they touch the same file, it
would not install with it, without rebasing, repatching.

The Docs are updated, and it's a relatively contained patch.

Changed status to Ready for Committer. (100% Guessing here...)

Kirk

Kirk Wolak <wolakk@gmail.com> writes:

Changed status to Ready for Committer. (100% Guessing here...)

Basically, I want to reject this on the grounds that it's not
useful enough to justify the overhead of marking the "role" GUC
as GUC_REPORT. The problems with it not going to work properly
with old servers are an additional reason not to like it.

But, if I lose the argument and we do commit this, I think it
should just print an empty string when dealing with an old server.
"ERR02000" is an awful idea, not least because it could be a
real role name.

BTW, we should probably get rid of the PQuser() fallback in
%n (session_username()) as well. It's unlikely that there are
still servers in the wild that don't report "session_authorization",
but if we did find one then the output is potentially misleading.
I'd rather print nothing than something that might not be your
actual session authorization setting.

regards, tom lane

út 4. 4. 2023 v 18:42 odesílatel Tom Lane <tgl@sss.pgh.pa.us> napsal:

Kirk Wolak <wolakk@gmail.com> writes:

Changed status to Ready for Committer. (100% Guessing here...)

Basically, I want to reject this on the grounds that it's not
useful enough to justify the overhead of marking the "role" GUC
as GUC_REPORT. The problems with it not going to work properly
with old servers are an additional reason not to like it.

If I understand to next comment correctly, the overhead should not be too
big

/*
* ReportChangedGUCOptions: report recently-changed GUC_REPORT variables
*
* This is called just before we wait for a new client query.
*
* By handling things this way, we ensure that a ParameterStatus message
* is sent at most once per variable per query, even if the variable
* changed multiple times within the query. That's quite possible when
* using features such as function SET clauses. Function SET clauses
* also tend to cause values to change intraquery but eventually revert
* to their prevailing values; ReportGUCOption is responsible for avoiding
* redundant reports in such cases.
*/

But, if I lose the argument and we do commit this, I think it
should just print an empty string when dealing with an old server.
"ERR02000" is an awful idea, not least because it could be a
real role name.

ok

BTW, we should probably get rid of the PQuser() fallback in
%n (session_username()) as well. It's unlikely that there are
still servers in the wild that don't report "session_authorization",
but if we did find one then the output is potentially misleading.
I'd rather print nothing than something that might not be your
actual session authorization setting.

It should be a separate patch?

Updated patch attached

Regards

Pavel

Pavel Stehule <pavel.stehule@gmail.com> writes:

út 4. 4. 2023 v 18:42 odesílatel Tom Lane <tgl@sss.pgh.pa.us> napsal:

Basically, I want to reject this on the grounds that it's not
useful enough to justify the overhead of marking the "role" GUC
as GUC_REPORT. The problems with it not going to work properly
with old servers are an additional reason not to like it.

If I understand to next comment correctly, the overhead should not be too
big

Yeah, but how big is the use-case? The reason I'm skeptical is that
half the time what you're going to get is "none":

$ psql
psql (16devel)
Type "help" for help.

regression=# show role;
role
------
none
(1 row)

That's required by SQL spec I believe, but that doesn't make it useful
data to keep in one's prompt.

regards, tom lane

út 4. 4. 2023 v 19:55 odesílatel Tom Lane <tgl@sss.pgh.pa.us> napsal:

Pavel Stehule <pavel.stehule@gmail.com> writes:

út 4. 4. 2023 v 18:42 odesílatel Tom Lane <tgl@sss.pgh.pa.us> napsal:

Basically, I want to reject this on the grounds that it's not
useful enough to justify the overhead of marking the "role" GUC
as GUC_REPORT. The problems with it not going to work properly
with old servers are an additional reason not to like it.

If I understand to next comment correctly, the overhead should not be too
big

Yeah, but how big is the use-case? The reason I'm skeptical is that
half the time what you're going to get is "none":

$ psql
psql (16devel)
Type "help" for help.

regression=# show role;
role
------
none
(1 row)

That's required by SQL spec I believe, but that doesn't make it useful
data to keep in one's prompt.

Who needs it, and who uses different roles, then very quickly uses SET ROLE
TO command.

But I fully agree so current behavior can be a little bit messy. I like
this feature, and I think it can have some benefits. Proposed
implementation is minimalistic.

One hard problem is translation of the oid of current_user to name. It
requires an opened transaction, and then it cannot be postponed to the end
of the statement. On the other hand, when the change of role is done inside
a nested command, then it should not be visible from the client side.

Can you accept the introduction of a new invisible GUC, that can be
modified only by SET ROLE TO command when it is executed as top command?

Regards

Pavel

út 4. 4. 2023 v 20:50 odesílatel Pavel Stehule <pavel.stehule@gmail.com>
napsal:

út 4. 4. 2023 v 19:55 odesílatel Tom Lane <tgl@sss.pgh.pa.us> napsal:

Pavel Stehule <pavel.stehule@gmail.com> writes:

út 4. 4. 2023 v 18:42 odesílatel Tom Lane <tgl@sss.pgh.pa.us> napsal:

Basically, I want to reject this on the grounds that it's not
useful enough to justify the overhead of marking the "role" GUC
as GUC_REPORT. The problems with it not going to work properly
with old servers are an additional reason not to like it.

If I understand to next comment correctly, the overhead should not be

too

big

Yeah, but how big is the use-case? The reason I'm skeptical is that
half the time what you're going to get is "none":

$ psql
psql (16devel)
Type "help" for help.

regression=# show role;
role
------
none
(1 row)

That's required by SQL spec I believe, but that doesn't make it useful
data to keep in one's prompt.

Who needs it, and who uses different roles, then very quickly uses SET
ROLE TO command.

But I fully agree so current behavior can be a little bit messy. I like
this feature, and I think it can have some benefits. Proposed
implementation is minimalistic.

One hard problem is translation of the oid of current_user to name. It
requires an opened transaction, and then it cannot be postponed to the end
of the statement. On the other hand, when the change of role is done inside
a nested command, then it should not be visible from the client side.

Can you accept the introduction of a new invisible GUC, that can be
modified only by SET ROLE TO command when it is executed as top command?

It was stupid idea.

There can be implemented fallbacks. When the role is "none", then the :USER
can be displayed instead.

It can work, because the custom role "none" is not allowed

(2023-04-04 21:10:25) postgres=# create role none;
ERROR: role name "none" is reserved
LINE 1: create role none;

?

út 4. 4. 2023 v 21:11 odesílatel Pavel Stehule <pavel.stehule@gmail.com>
napsal:

út 4. 4. 2023 v 20:50 odesílatel Pavel Stehule <pavel.stehule@gmail.com>
napsal:

út 4. 4. 2023 v 19:55 odesílatel Tom Lane <tgl@sss.pgh.pa.us> napsal:

Pavel Stehule <pavel.stehule@gmail.com> writes:

út 4. 4. 2023 v 18:42 odesílatel Tom Lane <tgl@sss.pgh.pa.us> napsal:

Basically, I want to reject this on the grounds that it's not
useful enough to justify the overhead of marking the "role" GUC
as GUC_REPORT. The problems with it not going to work properly
with old servers are an additional reason not to like it.

If I understand to next comment correctly, the overhead should not be

too

big

Yeah, but how big is the use-case? The reason I'm skeptical is that
half the time what you're going to get is "none":

$ psql
psql (16devel)
Type "help" for help.

regression=# show role;
role
------
none
(1 row)

That's required by SQL spec I believe, but that doesn't make it useful
data to keep in one's prompt.

Who needs it, and who uses different roles, then very quickly uses SET
ROLE TO command.

But I fully agree so current behavior can be a little bit messy. I like
this feature, and I think it can have some benefits. Proposed
implementation is minimalistic.

One hard problem is translation of the oid of current_user to name. It
requires an opened transaction, and then it cannot be postponed to the end
of the statement. On the other hand, when the change of role is done inside
a nested command, then it should not be visible from the client side.

Can you accept the introduction of a new invisible GUC, that can be
modified only by SET ROLE TO command when it is executed as top command?

It was stupid idea.

There can be implemented fallbacks. When the role is "none", then the
:USER can be displayed instead.

It can work, because the custom role "none" is not allowed

(2023-04-04 21:10:25) postgres=# create role none;
ERROR: role name "none" is reserved
LINE 1: create role none;

?

attached updated patch

Regards

Pavel

On Tue, Apr 4, 2023 at 12:42 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Basically, I want to reject this on the grounds that it's not
useful enough to justify the overhead of marking the "role" GUC
as GUC_REPORT.

I agree with that. I think we need some method for optionally
reporting values, so that stuff like this can be handled without
adding it to the wire protocol for everyone. I don't think we can just
keep adding stuff to the set of things that gets reported for
everyone. It doesn't scale. We need a really good reason to enlarge
the set of values reported for all users, and I don't think this meets
that bar.

--
Robert Haas
EDB: http://www.enterprisedb.com

Robert Haas <robertmhaas@gmail.com> writes:

On Tue, Apr 4, 2023 at 12:42 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Basically, I want to reject this on the grounds that it's not
useful enough to justify the overhead of marking the "role" GUC
as GUC_REPORT.

I agree with that. I think we need some method for optionally
reporting values, so that stuff like this can be handled without
adding it to the wire protocol for everyone.

It could probably be possible to provide some mechanism for setting
GUC_REPORT on specific variables locally within sessions. I don't
think this'd be much of a protocol-break problem, because clients
should already be coded to deal gracefully with ParameterStatus messages
for variables they don't know. However, connecting that up to something
like a psql prompt feature would still be annoying. I doubt I'd want
to go as far as having psql try to turn on GUC_REPORT automatically
if it sees %N in the prompt ...

regards, tom lane

On Wed, Apr 5, 2023 at 9:56 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Tue, Apr 4, 2023 at 12:42 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Basically, I want to reject this on the grounds that it's not
useful enough to justify the overhead of marking the "role" GUC
as GUC_REPORT.

I agree with that. I think we need some method for optionally
reporting values, so that stuff like this can be handled without
adding it to the wire protocol for everyone.

It could probably be possible to provide some mechanism for setting
GUC_REPORT on specific variables locally within sessions. I don't
think this'd be much of a protocol-break problem, because clients
should already be coded to deal gracefully with ParameterStatus messages
for variables they don't know. However, connecting that up to something
like a psql prompt feature would still be annoying. I doubt I'd want
to go as far as having psql try to turn on GUC_REPORT automatically
if it sees %N in the prompt ...

Oh, I had it in mind that it would do exactly that. And I think that
should be mediated by a wire protocol message, not a GUC, so that
users don't mess things up for psql or other clients -- in either
direction -- via SET commands.

Maybe there's a better way, that just seemed like the obvious design.

--
Robert Haas
EDB: http://www.enterprisedb.com

st 5. 4. 2023 v 15:56 odesílatel Tom Lane <tgl@sss.pgh.pa.us> napsal:

Robert Haas <robertmhaas@gmail.com> writes:

On Tue, Apr 4, 2023 at 12:42 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Basically, I want to reject this on the grounds that it's not
useful enough to justify the overhead of marking the "role" GUC
as GUC_REPORT.

I agree with that. I think we need some method for optionally
reporting values, so that stuff like this can be handled without
adding it to the wire protocol for everyone.

It could probably be possible to provide some mechanism for setting
GUC_REPORT on specific variables locally within sessions. I don't
think this'd be much of a protocol-break problem, because clients
should already be coded to deal gracefully with ParameterStatus messages
for variables they don't know. However, connecting that up to something
like a psql prompt feature would still be annoying. I doubt I'd want
to go as far as having psql try to turn on GUC_REPORT automatically
if it sees %N in the prompt ...

I agree with this analyze

Regards

Pavel

st 5. 4. 2023 v 16:02 odesílatel Robert Haas <robertmhaas@gmail.com> napsal:

On Wed, Apr 5, 2023 at 9:56 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Tue, Apr 4, 2023 at 12:42 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Basically, I want to reject this on the grounds that it's not
useful enough to justify the overhead of marking the "role" GUC
as GUC_REPORT.

I agree with that. I think we need some method for optionally
reporting values, so that stuff like this can be handled without
adding it to the wire protocol for everyone.

It could probably be possible to provide some mechanism for setting
GUC_REPORT on specific variables locally within sessions. I don't
think this'd be much of a protocol-break problem, because clients
should already be coded to deal gracefully with ParameterStatus messages
for variables they don't know. However, connecting that up to something
like a psql prompt feature would still be annoying. I doubt I'd want
to go as far as having psql try to turn on GUC_REPORT automatically
if it sees %N in the prompt ...

Oh, I had it in mind that it would do exactly that. And I think that
should be mediated by a wire protocol message, not a GUC, so that
users don't mess things up for psql or other clients -- in either
direction -- via SET commands.

If the GUC_REPORT should not be used, then only one possibility is
enhancing the protocol, about the possibility to read some predefined
server's features from the client.
It can be much cheaper than SQL query, and it can be used when the current
transaction is aborted. I can imagine a possibility to read server time or
a server session role from a prompt processing routine.

But for this specific case, you need to cache the role name somewhere. You
can simply get oid everytime, but for role name you need to access to
system catalogue, and it is not possible in aborted transactions. So at the
end, you probably should read "role" GUC.

Can this design be acceptable?

Regards

Pavel

On Wed, Apr 5, 2023 at 11:34 AM Pavel Stehule <pavel.stehule@gmail.com> wrote:

If the GUC_REPORT should not be used, then only one possibility is enhancing the protocol, about the possibility to read some predefined server's features from the client.
It can be much cheaper than SQL query, and it can be used when the current transaction is aborted. I can imagine a possibility to read server time or a server session role from a prompt processing routine.

But for this specific case, you need to cache the role name somewhere. You can simply get oid everytime, but for role name you need to access to system catalogue, and it is not possible in aborted transactions. So at the end, you probably should read "role" GUC.

Can this design be acceptable?

I don't think we want to add a dedicated protocol message that says
"send me the role GUC right now". I mean, we could, but being able to
tell the GUC mechanism "please send me the role GUC after every
command" sounds a lot easier to use.

--
Robert Haas
EDB: http://www.enterprisedb.com

st 5. 4. 2023 v 17:47 odesílatel Robert Haas <robertmhaas@gmail.com> napsal:

On Wed, Apr 5, 2023 at 11:34 AM Pavel Stehule <pavel.stehule@gmail.com>
wrote:

If the GUC_REPORT should not be used, then only one possibility is

enhancing the protocol, about the possibility to read some predefined
server's features from the client.

It can be much cheaper than SQL query, and it can be used when the

current transaction is aborted. I can imagine a possibility to read server
time or a server session role from a prompt processing routine.

But for this specific case, you need to cache the role name somewhere.

You can simply get oid everytime, but for role name you need to access to
system catalogue, and it is not possible in aborted transactions. So at the
end, you probably should read "role" GUC.

Can this design be acceptable?

I don't think we want to add a dedicated protocol message that says
"send me the role GUC right now". I mean, we could, but being able to
tell the GUC mechanism "please send me the role GUC after every
command" sounds a lot easier to use.

I'll try it

Regards

Pavel