Thread: Things I don't like about \du's "Attributes" column

Nearby I dissed psql's \du command for its incoherent "Attributes"
column [1].  It's too late to think about changing that for v16,
but here's some things I think we should consider for v17:

* It seems weird that some attributes are described in the negative
("Cannot login", "No inheritance").  I realize that this corresponds
to the defaults, so that a user created by CREATE USER with no options
shows nothing in the Attributes column; but I wonder how much that's
worth.  As far as "Cannot login" goes, you could argue that the silent
default ought to be for the properties assigned by CREATE ROLE, since
the table describes itself as "List of roles".  I'm not dead set on
changing this, but it seems like a topic that deserves a fresh look.

* I do not like the display of rolconnlimit, ie "No connections" or
"%d connection(s)".  A person not paying close attention might think
that that means the number of *current* connections the user has.
A minimal fix could be to word it like "No connections allowed" or
"%d connection(s) allowed".  But see below.

* I do not like the display of rolvaliduntil, either.  Consider

regression=# create user alice password 'secret';
CREATE ROLE
regression=# create user bob valid until 'infinity';
CREATE ROLE
regression=# \du
...
 alice     |
 bob       | Password valid until infinity
...

This output claims that bob has an indefinitely-valid password, when in
fact he has no password at all.  On the other hand, nothing is said about
alice, who actually *does* have a password valid until infinity.  It's
difficult to imagine a more misleading way to present this.

Now, it's hard to do better given that the \du command is examining the
universally-readable pg_roles view, because that doesn't betray any hint
of whether the user has a password or not.  I wonder though what is the
rationale for letting unprivileged users see the rolvaliduntil column
but not whether a password exists at all.  I suggest that maybe it'd
be okay to change the pg_roles view along the lines of

-       '********'::text as rolpassword,
+       case when rolpassword is not null then '********'::text end as rolpassword,

Then we could fix \du to say nothing if rolpassword is null,
and when it isn't, print "Password valid until infinity" whenever
that is the case (ie, rolvaliduntil is null or infinity).

* On a purely presentation level, how did we possibly arrive
at the idea that the connection-limit and valid-until properties
deserve their own lines in the Attributes column while the other
properties are comma-separated?  That makes no sense whatsoever,
nor does it look nice in \x display format.

I do grasp the distinction that the other properties are permission
bits while these two aren't, but that doesn't naturally lead to
this formatting.  I'd vote for either

(a) each property gets its own line, or

(b) move these two things into separate columns.  Some of the
verbiage could then be dropped in favor of the column title.

Right now (b) would lead to an undesirably wide table; but
if we push the "Member of" column out to a different \d command
as discussed in the other thread, maybe it'd be practical.

Anyway, for now I'm just throwing this topic out for discussion.

            regards, tom lane

[1] https://www.postgresql.org/message-id/4128935.1687478926%40sss.pgh.pa.us

On 23.06.2023 03:50, Tom Lane wrote:
> Nearby I dissed psql's \du command for its incoherent "Attributes"
> column [1].  It's too late to think about changing that for v16,
> but here's some things I think we should consider for v17:

If there are no others willing, I am ready to take up this topic. There 
is definitely room for improvement here.
But first I want to finish with the \du command.

-- 
Pavel Luzanov
Postgres Professional: https://postgrespro.com

On 23.06.2023 03:50, Tom Lane wrote:
> * On a purely presentation level, how did we possibly arrive
> at the idea that the connection-limit and valid-until properties
> deserve their own lines in the Attributes column while the other
> properties are comma-separated?  That makes no sense whatsoever,
> nor does it look nice in \x display format.

I think this a reason why footer property explicitly disabled in the output.
As part of reworking footer should be enabled, as it worked for other 
meta-commands.

Just to don't forget.

-- 
Pavel Luzanov
Postgres Professional: https://postgrespro.com

Nearby I dissed psql's \du command for its incoherent "Attributes"
column [1].  It's too late to think about changing that for v16,
but here's some things I think we should consider for v17:

* It seems weird that some attributes are described in the negative
("Cannot login", "No inheritance").  I realize that this corresponds
to the defaults, so that a user created by CREATE USER with no options
shows nothing in the Attributes column; but I wonder how much that's
worth.  As far as "Cannot login" goes, you could argue that the silent
default ought to be for the properties assigned by CREATE ROLE, since
the table describes itself as "List of roles".  I'm not dead set on
changing this, but it seems like a topic that deserves a fresh look.

* I do not like the display of rolconnlimit, ie "No connections" or
"%d connection(s)".  A person not paying close attention might think
that that means the number of *current* connections the user has.
A minimal fix could be to word it like "No connections allowed" or
"%d connection(s) allowed".  But see below.

* I do not like the display of rolvaliduntil, either.

I suggest that maybe it'd
be okay to change the pg_roles view along the lines of

-       '********'::text as rolpassword,
+       case when rolpassword is not null then '********'::text end as rolpassword,

Then we could fix \du to say nothing if rolpassword is null,
and when it isn't, print "Password valid until infinity" whenever
that is the case (ie, rolvaliduntil is null or infinity).

* On a purely presentation level, how did we possibly arrive
at the idea that the connection-limit and valid-until properties
deserve their own lines in the Attributes column while the other
properties are comma-separated?  That makes no sense whatsoever,
nor does it look nice in \x display format.

(b) move these two things into separate columns.

--DROP ROLE alice, bob, charlie, admin;

CREATE ROLE alice LOGIN SUPERUSER NOINHERIT PASSWORD 'alice' VALID UNTIL 'infinity' CONNECTION LIMIT 5;
CREATE ROLE bob LOGIN REPLICATION BYPASSRLS CREATEDB VALID UNTIL '2022-01-01';
CREATE ROLE charlie LOGIN CREATEROLE PASSWORD 'charlie' CONNECTION LIMIT 0;
CREATE ROLE admin;

COMMENT ON ROLE alice IS 'Superuser but with connection limit and with no inheritance';
COMMENT ON ROLE bob IS 'No password but with expire time';
COMMENT ON ROLE charlie IS 'No connections allowed';
COMMENT ON ROLE admin IS 'Group role without login';


Master.
=# \du                             List of roles Role name |                         Attributes                         
-----------+------------------------------------------------------------ admin     | Cannot login alice     | Superuser, No inheritance                                 +           | 5 connections                                             +           | Password valid until infinity bob       | Create DB, Replication, Bypass RLS                        +           | Password valid until 2022-01-01 00:00:00+03 charlie   | Create role                                               +           | No connections postgres  | Superuser, Create role, Create DB, Replication, Bypass RLS

=# \du+                                                            List of roles Role name |                         Attributes                         |                         Description                         
-----------+------------------------------------------------------------+------------------------------------------------------------- admin     | Cannot login                                               | Group role without login alice     | Superuser, No inheritance                                 +| Superuser but with connection limit and with no inheritance           | 5 connections                                             +|            | Password valid until infinity                              |  bob       | Create DB, Replication, Bypass RLS                        +| No password but with expire time           | Password valid until 2022-01-01 00:00:00+03                |  charlie   | Create role                                               +| No connections allowed           | No connections                                             |  postgres  | Superuser, Create role, Create DB, Replication, Bypass RLS | 


Patched.
=# \du                                    List of roles Role name | Can login? |                         Attributes                         
-----------+------------+------------------------------------------------------------ admin     | no         |  alice     | yes        | Superuser, No inheritance bob       | yes        | Create DB, Replication, Bypass RLS charlie   | yes        | Create role postgres  | yes        | Superuser, Create role, Create DB, Replication, Bypass RLS
(5 rows)

=# \du+                                                                                                List of roles Role name | Can login? |                         Attributes                         | Has password? |  Password expire time  | Max connections |                         Description                         
-----------+------------+------------------------------------------------------------+---------------+------------------------+-----------------+------------------------------------------------------------- admin     | no         |                                                            | no            |                        |              -1 | Group role without login alice     | yes        | Superuser, No inheritance                                  | yes           | infinity               |               5 | Superuser but with connection limit and with no inheritance bob       | yes        | Create DB, Replication, Bypass RLS                         | no            | 2022-01-01 00:00:00+03 |              -1 | No password but with expire time charlie   | yes        | Create role                                                | yes           |                        |               0 | No connections allowed postgres  | yes        | Superuser, Create role, Create DB, Replication, Bypass RLS | yes           |                        |              -1 | 
(5 rows)

-- 
Pavel Luzanov
Postgres Professional: https://postgrespro.com

Would it make sense to make the column non-nullable and always set it to infinity when there is no expiry?

-- 
Pavel Luzanov
Postgres Professional: https://postgrespro.com

On Thu, Jun 22, 2023 at 8:50 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> * It seems weird that some attributes are described in the negative
> ("Cannot login", "No inheritance").  I realize that this corresponds
> to the defaults, so that a user created by CREATE USER with no options
> shows nothing in the Attributes column; but I wonder how much that's
> worth.  As far as "Cannot login" goes, you could argue that the silent
> default ought to be for the properties assigned by CREATE ROLE, since
> the table describes itself as "List of roles".  I'm not dead set on
> changing this, but it seems like a topic that deserves a fresh look.

I wonder if we shouldn't try to display the roles's properties using
SQL keywords rather than narrating. Someone can be confused by "No
connections" but "CONNECTION LIMIT 0" is pretty hard to mistake;
likewise "LOGIN" or "NOLOGIN" seems clear enough. If we took this
approach, there would still be a question in my mind about whether to
show values where the configured value of the property matches the
default, and maybe we would want to do that in some cases and skip it
in others, or maybe we would end up with a uniform rule, but that
issue could be considered somewhat separately from how to print the
properties we choose to display.

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

Robert Haas <robertmhaas@gmail.com> writes:
> I wonder if we shouldn't try to display the roles's properties using
> SQL keywords rather than narrating. Someone can be confused by "No
> connections" but "CONNECTION LIMIT 0" is pretty hard to mistake;
> likewise "LOGIN" or "NOLOGIN" seems clear enough.

Mmm ... maybe.  I think those of us who are native English speakers
may overrate the intelligibility of SQL keywords to those who aren't.
So I'm inclined to feel that preserving translatability of the
role property descriptions is a good thing.  But it'd be good to
hear comments on that point from people who actually use it.

            regards, tom lane

On Tue, Jan 2, 2024 at 1:17 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Mmm ... maybe.  I think those of us who are native English speakers
> may overrate the intelligibility of SQL keywords to those who aren't.
> So I'm inclined to feel that preserving translatability of the
> role property descriptions is a good thing.  But it'd be good to
> hear comments on that point from people who actually use it.

+1 for comments from people who use it.

My thought was that such people probably need to interpret LOGIN and
NOLOGIN into their preferred language either way, but if \du displays
something else, then they also need to mentally construct a reverse
mapping, from whatever string is showing up there to the corresponding
SQL syntax. The current display has that problem even for English
speakers -- you have to know that "Cannot login" corresponds to
"NOLOGIN" and that "No connections" corresponds to "CONNECTION LIMIT
0" and so forth. No matter what we put there, if it's English or
Turkish or Hindi rather than SQL, you have to try to figure out what
the displayed text corresponds to at the SQL level in order to fix
anything that isn't the way you want it to be, or to recreate the
configuration on another instance.

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

Robert Haas <robertmhaas@gmail.com> writes:
> My thought was that such people probably need to interpret LOGIN and
> NOLOGIN into their preferred language either way, but if \du displays
> something else, then they also need to mentally construct a reverse
> mapping, from whatever string is showing up there to the corresponding
> SQL syntax. The current display has that problem even for English
> speakers -- you have to know that "Cannot login" corresponds to
> "NOLOGIN" and that "No connections" corresponds to "CONNECTION LIMIT
> 0" and so forth.

True, although if you aren't happy with the current state then what
you actually need to construct is a SQL command to set a *different*
state from what \du is saying.  Going from LOGIN to NOLOGIN or vice
versa can also be non-obvious.  So you're likely to end up consulting
"\h alter user" no matter what, if you don't have it memorized.

I think your argument does have relevance for the other issue about
whether it's good to be silent about the defaults.  If \du says
nothing at all about a particular property, that certainly isn't
helping you to decide whether you want to change it and if so to what.
I'm not convinced that point is dispositive, but it's something
to consider.

            regards, tom lane

On Tue, Jan 2, 2024 at 1:46 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> True, although if you aren't happy with the current state then what
> you actually need to construct is a SQL command to set a *different*
> state from what \du is saying.  Going from LOGIN to NOLOGIN or vice
> versa can also be non-obvious.  So you're likely to end up consulting
> "\h alter user" no matter what, if you don't have it memorized.

That could be true in some cases, but I don't think it's true in all
cases. If you're casually familiar with ALTER USER you probably
remember that many of the properties are negated by sticking NO on the
front; you're less likely to forget that than you are to forget the
name of some specific property. And certainly if you see CONNECTION
LIMIT 24 and you want to change it to CONNECTION LIMIT 42 it's going
to be pretty clear what to adjust.

> I think your argument does have relevance for the other issue about
> whether it's good to be silent about the defaults.  If \du says
> nothing at all about a particular property, that certainly isn't
> helping you to decide whether you want to change it and if so to what.
> I'm not convinced that point is dispositive, but it's something
> to consider.

I agree with 100% of what you say here.

To add to that a bit, I would probably never ask a user to give me the
output of \du to troubleshoot some issue. I would either ask them for
pg_dumpall -g output, or I'd ask them to give me the raw contents of
pg_authid. That's because I know that either of those things are going
to tell me about ALL the properties of the role, or at least all of
the properties of the role that are stored in pg_authid, without
omitting anything that some hacker thought was uninteresting. I don't
know that \du is going to do that, and I'm not going to want to read
the code to figure out which cases it thinks are uninteresting,
*especially* if it behaves differently by version.

The counterargument is that if you don't omit anything, the output
gets very long, which is a problem, because either you go wide, and
then you get wrapping, or you use multiple-lines, and then if there
are 500 users the output goes on forever.

I think a key consideration here is how easy it will be for somebody
to guess the value of a property that is not mentioned. Personally,
I'd assume that if CONNECTION LIMIT isn't mentioned, it's unlimited.
But a lot of the other options are less clear. Probably NOSUPERUSER is
the default and SUPERUSER is the exception, but it's very unclear
whether LOGIN or NOLOGIN is should be treated as the "normal" case,
given that the feature encompasses users and groups and non-login
roles that people access via SET ROLE and things that look like groups
but are also used as login roles.

And with some of the other options, it's just harder to remember
whether there's a default and what it is exactly than for other object
types. With something like a table column, it feels intuitive that if
you just ask for a table column, you get a "normal" table column ...
and then if you add a fillfactor or a CHECK constraint it will show up
in the \d output, and otherwise not. But to try to apply that concept
here means that we suppose the user knows whether the default is
INHERIT or NOINHERIT, whether the default is BYPASSRLS or NOBYPASSRLS,
etc. And I'm just a little bit skeptical of that assumption. Perhaps
it's just that I've spent less time doing user management than table
administration and so I'm the only one who finds this fuzzier than
some other kinds of SQL objects, but I'm not sure it's just that.
Roles are pretty weird.

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

On 02.01.2024 22:38, Robert Haas wrote:

To add to that a bit, I would probably never ask a user to give me the
output of \du to troubleshoot some issue. I would either ask them for
pg_dumpall -g output, or I'd ask them to give me the raw contents of
pg_authid. That's because I know that either of those things are going
to tell me about ALL the properties of the role, or at least all of
the properties of the role that are stored in pg_authid, without
omitting anything that some hacker thought was uninteresting. I don't
know that \du is going to do that, and I'm not going to want to read
the code to figure out which cases it thinks are uninteresting,
*especially* if it behaves differently by version.

\d commands are a convenient way to see the contents of the system
catalogs. Short commands, instead of long SQL queries. Imo, this is their
main purpose.

Interpreting values ('No connection' instead of 0 and so on)
can be useful if the actual values are easy to identify. If there is
doubt whether it will be clear, then it is better to show it as is.
The documentation contains a description of the system catalogs.
It tells you how to interpret the values correctly.

The counterargument is that if you don't omit anything, the output
gets very long, which is a problem, because either you go wide, and
then you get wrapping, or you use multiple-lines, and then if there
are 500 users the output goes on forever.

This can be mostly solved by using extended mode. Key properties for \du,
all others for \du+. Also \du+ can used with \x.
Of course, the question arises as to which properties are key and
which are not. Here we need to reach a compromise.

Personally,
I'd assume that if CONNECTION LIMIT isn't mentioned, it's unlimited.
But a lot of the other options are less clear. Probably NOSUPERUSER is
the default and SUPERUSER is the exception, but it's very unclear
whether LOGIN or NOLOGIN is should be treated as the "normal" case,
given that the feature encompasses users and groups and non-login
roles that people access via SET ROLE and things that look like groups
but are also used as login roles.

And with some of the other options, it's just harder to remember
whether there's a default and what it is exactly than for other object
types.

psql provides a handy tool for solving such questions - ECHO_HIDDEN variable.
But it is very important that the query text is easily transformed into
the command output.
Proposed patch tries to implement this approach.

-- 
Pavel Luzanov
Postgres Professional: https://postgrespro.com

Some attributes are arguably important enough to warrant their own column. The most obvious is NOLOGIN, since those roles are generally used for a very different purpose than LOGIN roles. SUPERUSER might be another candidate (though, I much prefer a dedicated "sudo role" than explicit SU on roles).

I like this idea.
But what if all the attributes are moved to separate columns?
This solves all issues except the wide output. Less significant attributes
can be moved to extended mode. Here's what it might look like:

postgres@postgres(17.0)=# \du                             List of roles Role name | Login | Superuser | Create role | Create DB | Replication 
-----------+-------+-----------+-------------+-----------+------------- admin     | no    | no        | no          | no        | no alice     | yes   | yes       | no          | no        | no bob       | yes   | no        | no          | yes       | yes charlie   | yes   | no        | yes         | no        | no postgres  | yes   | yes       | yes         | yes       | yes
(5 rows)

postgres@postgres(17.0)=# \du+                                                                                                     List of roles Role name | Login | Superuser | Create role | Create DB | Replication | Bypass RLS | Inheritance | Password |      Valid until       | Connection limit |                         Description                         
-----------+-------+-----------+-------------+-----------+-------------+------------+-------------+----------+------------------------+------------------+------------------------------------------------------------- admin     | no    | no        | no          | no        | no          | no         | yes         | no       |                        |               -1 | Group role without login alice     | yes   | yes       | no          | no        | no          | no         | no          | yes      | infinity               |                5 | Superuser but with connection limit and with no inheritance bob       | yes   | no        | no          | yes       | yes         | yes        | yes         | no       | 2022-01-01 00:00:00+03 |               -1 | No password but with expire time charlie   | yes   | no        | yes         | no        | no          | no         | yes         | yes      |                        |                0 | No connections allowed postgres  | yes   | yes       | yes         | yes       | yes         | yes        | yes         | yes      |                        |               -1 | 
(5 rows)

postgres@postgres(17.0)=# \x \du+ bob
Expanded display is on.
List of roles
-[ RECORD 1 ]----+---------------------------------
Role name        | bob
Login            | yes
Superuser        | no
Create role      | no
Create DB        | yes
Replication      | yes
Bypass RLS       | yes
Inheritance      | yes
Password         | no
Valid until      | 2022-01-01 00:00:00+03
Connection limit | -1
Description      | No password but with expire time

-- 
Pavel Luzanov
Postgres Professional: https://postgrespro.com

Another approach based on early suggestions.

The Attributes column includes only the enabled logical attributes.
Regardless of whether the attribute is enabled by default or not.
This changes the current behavior, but makes it clearer: everything
that is enabled is displayed. This principle is easy to maintain in
subsequent versions, even if there is a desire to change the default
value for any attribute. In addition, the issue with the 'LOGIN' attribute
is being resolved, the default value of which depends on the command
(CREATE ROLE or CREATE USER).

The attribute names correspond to the keywords of the CREATE ROLE command.
The attributes are listed in the same order as in the documentation.
(I think that the LOGIN attribute should be moved to the first place,
both in the documentation and in the command.)


The "Connection limit" and "Valid until" attributes are placed in separate columns.
The "Password?" column has been added.

Sample output.

Patch v3:
=# \du                                                             List of roles Role name |                            Attributes                             | Password? |      Valid until       | Connection limit 
-----------+-------------------------------------------------------------------+-----------+------------------------+------------------ admin     | INHERIT                                                           | no        |                        |               -1 alice     | SUPERUSER LOGIN                                                   | yes       | infinity               |                5 bob       | CREATEDB INHERIT LOGIN REPLICATION BYPASSRLS                      | no        | 2022-01-01 00:00:00+03 |               -1 charlie   | CREATEROLE INHERIT LOGIN                                          | yes       |                        |                0 postgres  | SUPERUSER CREATEDB CREATEROLE INHERIT LOGIN REPLICATION BYPASSRLS | no        |                        |               -1
(5 rows)


The output of the command is long. But there are other commands of
comparable length: \dApS, \dfS, \doS.

Small modification with newline separator for Attributes column:

Patch v3 with newlines:
=# \du                                  List of roles Role name | Attributes  | Password? |      Valid until       | Connection limit 
-----------+-------------+-----------+------------------------+------------------ admin     | INHERIT     | no        |                        |               -1 alice     | SUPERUSER  +| yes       | infinity               |                5           | LOGIN       |           |                        |  bob       | CREATEDB   +| no        | 2022-01-01 00:00:00+03 |               -1           | INHERIT    +|           |                        |            | LOGIN      +|           |                        |            | REPLICATION+|           |                        |            | BYPASSRLS   |           |                        |  charlie   | CREATEROLE +| yes       |                        |                0           | INHERIT    +|           |                        |            | LOGIN       |           |                        |  postgres  | SUPERUSER  +| no        |                        |               -1           | CREATEDB   +|           |                        |            | CREATEROLE +|           |                        |            | INHERIT    +|           |                        |            | LOGIN      +|           |                        |            | REPLICATION+|           |                        |            | BYPASSRLS   |           |                        | 
(5 rows)

For comparison, here's what it looks like now:

master:
=# \du                             List of roles Role name |                         Attributes                         
-----------+------------------------------------------------------------ admin     | Cannot login alice     | Superuser, No inheritance                                 +           | 5 connections                                             +           | Password valid until infinity bob       | Create DB, Replication, Bypass RLS                        +           | Password valid until 2022-01-01 00:00:00+03 charlie   | Create role                                               +           | No connections postgres  | Superuser, Create role, Create DB, Replication, Bypass RLS


From my point of view, any of the proposed alternatives is better than what we have now.
But for moving forward we need to choose some approach.

I will be glad of any opinions.

-- 
Pavel Luzanov
Postgres Professional: https://postgrespro.com

2024-01 Commitfest.

Hi, This patch has a CF status of "Needs Review" [1], but it seems
there were CFbot test failures last time it was run [2]. Please have a
look and post an updated version if necessary.

======
[1] https://commitfest.postgresql.org/46/4738/
[2] https://cirrus-ci.com/github/postgresql-cfbot/postgresql/commitfest/46/4738

Kind Regards,
Peter Smith.

Pavel Luzanov <p.luzanov@postgrespro.ru> writes:
> Another approach based on early suggestions.

I think expecting the pg_roles view to change for this is problematic.
You can't have that in the back branches, so with this patch psql
will show something different against a pre-17 server than later
versions.  At best, that's going to be confusing.  Can you get the
same result without changing pg_roles?

            regards, tom lane

I wrote:
> I think expecting the pg_roles view to change for this is problematic.
> You can't have that in the back branches, so with this patch psql
> will show something different against a pre-17 server than later
> versions.  At best, that's going to be confusing.

Actually, even more to the point: while this doesn't expose the
contents of a role's password, it does expose whether the role
*has* a password to every user in the installation.  I doubt
that that's okay from a security standpoint.  It'd need debate
at the least.

            regards, tom lane

I think expecting the pg_roles view to change for this is problematic.
You can't have that in the back branches, so with this patch psql
will show something different against a pre-17 server than later
versions.  At best, that's going to be confusing.

I've been thinking about it. Therefore, the column "Password?" is shown
only for version 17 and older.

  Can you get the same result without changing pg_roles?

Hm. I'm not sure if this is possible.

Actually, even more to the point: while this doesn't expose the
contents of a role's password, it does expose whether the role
*has* a password to every user in the installation.  I doubt
that that's okay from a security standpoint.  It'd need debate
at the least.

Yes, I remember your caution about security from the original post.
I'll try to explain why changing pg_roles is acceptable.
Now \du shows column "Valid until". We know that you can set
the password expiration date without having a password, but this is
more likely a mistake in role maintenance. In most cases, a non-null
value indicates that the password has been set. Therefore, security
should not suffer much, but it will help the administrator to see
incorrect values.

On 23.01.2024 05:22, David G. Johnston wrote:
> At present it seems like a createrole permissioned user is unable 
> to determine whether a given role has a password or not even in the case
> when that role would be allowed to alter a role they've created to set or
> remove said password.  Keeping with the changes made in v16 it does seem
> worthwhile modifying pg_roles to be sensitive to the role querying the view
> having both createrole and admin membership on the role being displayed.
> With now three possible outcomes: NULL if no password is in use, *********
> if a password is in use and the user has the ability to alter role, or
> <insufficient privileges> (alt. N/A).

Good point.
But what about "Valid until". Can roles without superuser or createrole
attributes see it? The same about "Connection limit"?

I'll think about it and try to implement in the next patch version within a few days.
Thank you for review.

-- 
Pavel Luzanov
Postgres Professional: https://postgrespro.com

The attribute names correspond to the keywords of the CREATE ROLE command.
The attributes are listed in the same order as in the documentation.
(I think that the LOGIN attribute should be moved to the first place,
both in the documentation and in the command.)

I'd just flip INHERIT and LOGIN

ok

I'm strongly in favor of using mixed-case for the attributes.  The SQL Command itself doesn't care about capitalization and it is much easier on the eyes.  I'm also strongly in favor of newlines, as can be seen by the default bootstrap superuser entry putting everything on one line eats up 65 characters.

                                    List of roles

 Role name | Attributes  | Password? | Valid until | Connection limit | Description
-----------+-------------+-----------+-------------+------------------+-------------
 davidj    | Superuser  +| no        |             |               -1 |
           | CreateDB   +|           |             |                  |
           | CreateRole +|           |             |                  |
           | Inherit    +|           |             |                  |
           | Login      +|           |             |                  |
           | Replication+|           |             |                  |
           | BypassRLS   |           |             |                  |
(1 row)

ok, I will continue with this display variant.

As noted by Peter this patch didn't update the two affected expected output files. psql.out and, due to the system view change, rules.out.  That particular change requires a documentation update to the pg_roles system view page. 

Yes, I was waiting for the direction of implementation to appear. Now it is there.

I'd suggest pulling out this system view change into its own patch.

But within this thread or new one?

On Sun, Jan 21, 2024 at 2:35 PM Pavel Luzanov <p.luzanov@postgrespro.ru> wrote:

                                  List of roles Role name | Attributes  | Password? |      Valid until       | Connection limit 
-----------+-------------+-----------+------------------------+------------------ admin     | INHERIT     | no        |                        |               -1 alice     | SUPERUSER  +| yes       | infinity               |                5

I think I'm in the minority on believing that these describe outputs should not be beholden to internal implementation details. 

Yes, I prefer real column values. But it can be discussed.

But seeing a -1 in the limit column is just jarring to my sensibilities.  I suggest displaying blank (not null, \pset should not influence this) if the connection limit is "no limit", only showing positive numbers when there is meaningful exceptional information for the user to absorb.

The connection limit can be set to 0. What should be displayed in this case, blank or 0?
The connection limit can be set for superusers. What should be displayed in this case,
blank or actual non-effective value? 
CREATE|ALTER ROLE commands allow incorrect values to be set for 'Conn limit' and 'Valid until'.
How can the administrator see them and fix them?

These are my reasons for real column values.

-- 
Pavel Luzanov
Postgres Professional: https://postgrespro.com

On 23.01.2024 04:18, Tom Lane wrote:

I think expecting the pg_roles view to change for this is problematic.
You can't have that in the back branches, so with this patch psql
will show something different against a pre-17 server than later
versions.  At best, that's going to be confusing.

  Can you get the same result without changing pg_roles?

Hm. I'm not sure if this is possible.

Probably there is a solution without changing pg_roles.
The \du command can execute different queries for superusers and other roles.
For superusers, the query is based on pg_authid, for other roles on pg_roles.
So superusers will see the 'Password?' column and the rest won't see him.
In this approach, the \du command will be able to work the same way for older
versions.

Is it worth going this way?

On 23.01.2024 05:22, David G. Johnston wrote:
> At present it seems like a createrole permissioned user is unable 
> to determine whether a given role has a password or not even in the case
> when that role would be allowed to alter a role they've created to set or
> remove said password.  Keeping with the changes made in v16 it does seem
> worthwhile modifying pg_roles to be sensitive to the role querying the view
> having both createrole and admin membership on the role being displayed.
> With now three possible outcomes: NULL if no password is in use, *********
> if a password is in use and the user has the ability to alter role, or
> <insufficient privileges> (alt. N/A).

Once again, this id a good point, but changes to pg_roles are required.
And the behavior of \du will be different for older versions.

-- 
Pavel Luzanov
Postgres Professional: https://postgrespro.com

I'll think about it and try to implement in the next patch version within a few days.

Sorry for delay.

Please look at v4.
I tried to implement all of David's suggestions.
The only addition - "Login" column. I still thinks this is important information to be highlighted.
Especially considering that the Attributes column small enough with a newline separator.

The changes are split into two patches.
0001 - pg_roles view. I plan to organize a new thread for discussion.
0002 - \du command. It depends on 0001 for "Password?" and "Valid until" columns.

Output for superuser:

postgres@postgres(17.0)=# \du+                                                                       List of roles    Role name     | Login | Attributes  | Password? |           Valid until           | Connection limit |                   Description                    
------------------+-------+-------------+-----------+---------------------------------+------------------+-------------------------------------------------- postgres         | yes   | Superuser  +| no        |                                 |                  |                   |       | Create DB  +|           |                                 |                  |                   |       | Create role+|           |                                 |                  |                   |       | Inherit    +|           |                                 |                  |                   |       | Replication+|           |                                 |                  |                   |       | Bypass RLS  |           |                                 |                  |  regress_du_admin | yes   | Create role+| yes       | infinity                        |                  | User createrole attribute                  |       | Inherit     |           |                                 |                  |  regress_du_role0 | yes   | Create DB  +| yes       | 2024-12-31 00:00:00+03          |                  |                   |       | Inherit    +|           |                                 |                  |                   |       | Replication+|           |                                 |                  |                   |       | Bypass RLS  |           |                                 |                  |  regress_du_role1 | no    | Inherit     | no        | 2024-12-31 00:00:00+03(invalid) | 50               | Group role without password but with valid until regress_du_role2 | yes   | Inherit     | yes       |                                 | Not allowed      | No connections allowed regress_du_role3 | yes   |             | yes       |                                 | 10               | User without attributes regress_du_su    | yes   | Superuser  +| yes       |                                 | 3(ignored)       | Superuser but with connection limit                  |       | Create DB  +|           |                                 |                  |                   |       | Create role+|           |                                 |                  |                   |       | Inherit    +|           |                                 |                  |                   |       | Replication+|           |                                 |                  |                   |       | Bypass RLS  |           |                                 |                  | 
(7 rows)

Output for regress_du_admin (can see password for regress_du_role[0,1,2]
but not for regress_du_role3):

regress_du_admin@postgres(17.0)=> \du regress_du_role*                                              List of roles    Role name     | Login | Attributes  | Password? |           Valid until           | Connection limit 
------------------+-------+-------------+-----------+---------------------------------+------------------ regress_du_role0 | yes   | Create DB  +| yes       | 2024-12-31 00:00:00+03          |                   |       | Inherit    +|           |                                 |                   |       | Replication+|           |                                 |                   |       | Bypass RLS  |           |                                 |  regress_du_role1 | no    | Inherit     | no        | 2024-12-31 00:00:00+03(invalid) | 50 regress_du_role2 | yes   | Inherit     | yes       |                                 | Not allowed regress_du_role3 | yes   |             |           |                                 | 10
(4 rows)

Output for regress_du_role3 (no password information):

regress_du_role3@postgres(17.0)=> \du regress_du_role*                                         List of roles    Role name     | Login | Attributes  | Password? |      Valid until       | Connection limit 
------------------+-------+-------------+-----------+------------------------+------------------ regress_du_role0 | yes   | Create DB  +|           | 2024-12-31 00:00:00+03 |                   |       | Inherit    +|           |                        |                   |       | Replication+|           |                        |                   |       | Bypass RLS  |           |                        |  regress_du_role1 | no    | Inherit     |           | 2024-12-31 00:00:00+03 | 50 regress_du_role2 | yes   | Inherit     |           |                        | Not allowed regress_du_role3 | yes   |             |           |                        | 10
(4 rows)


Any comments. What did I miss?

-- 
Pavel Luzanov
Postgres Professional: https://postgrespro.com

The changes are split into two patches.
0001 - pg_roles view. I plan to organize a new thread for discussion.

Please see it here:
https://www.postgresql.org/message-id/db1d94ba-1e6e-4e86-baff-91e6e79071c1%40postgrespro.ru

-- 
Pavel Luzanov
Postgres Professional: https://postgrespro.com

"David G. Johnston" <david.g.johnston@gmail.com> writes:
> Per the recent bug report, we should probably add something like (ignored)
> after the 50 connections for role1 since they are not allowed to login so
> the value is indeed ignored.  It is ignored to zero as opposed to unlimited
> for the Superuser so maybe a different word (not allowed)?

Not sure it's worth worrying about, but if we do I'd not bother to
show the irrelevant value at all: it's just making the display wider
to little purpose.  We could make the column read as "(irrelevant)",
or leave it blank.  I'd argue the same for password expiration
time BTW.

            regards, tom lane

"David G. Johnston" <david.g.johnston@gmail.com> writes:

Per the recent bug report, we should probably add something like (ignored)
after the 50 connections for role1 since they are not allowed to login so
the value is indeed ignored.  It is ignored to zero as opposed to unlimited
for the Superuser so maybe a different word (not allowed)?

Not sure it's worth worrying about, but if we do I'd not bother to
show the irrelevant value at all: it's just making the display wider
to little purpose.  We could make the column read as "(irrelevant)",
or leave it blank.  I'd argue the same for password expiration
time BTW.

Please look at v5.

Changes:
- 'XXX(ignored)' replaced by '(irrelevant)' for 'Connection limit'.	for superusers with Connection limit	for roles without login and Connection limit
- 'XXX(invalid)' replaced by '(irrelevant)' for 'Valid until'.	for roles without password and Valid until
- 'Not allowed' replaced by '(not allowed)' for consistency.	for roles with Connection limit = 0

postgres@postgres(17.0)=# \du regress*                                         List of roles    Role name     | Login | Attributes  | Password? |      Valid until       | Connection limit 
------------------+-------+-------------+-----------+------------------------+------------------ regress_du_admin | yes   | Create role+| yes       | infinity               |                   |       | Inherit     |           |                        |  regress_du_role0 | yes   | Create DB  +| yes       | 2024-12-31 00:00:00+03 |                   |       | Inherit    +|           |                        |                   |       | Replication+|           |                        |                   |       | Bypass RLS  |           |                        |  regress_du_role1 | no    | Inherit     | no        | (irrelevant)           | (irrelevant) regress_du_role2 | yes   | Inherit     | yes       |                        | (not allowed) regress_du_role3 | yes   |             | yes       |                        | 10 regress_du_su    | yes   | Superuser  +| yes       |                        | (irrelevant)                  |       | Create DB  +|           |                        |                   |       | Create role+|           |                        |                   |       | Inherit    +|           |                        |                   |       | Replication+|           |                        |                   |       | Bypass RLS  |           |                        | 
(6 rows)

-- 
Pavel Luzanov
Postgres Professional: https://postgrespro.com

On Mon, Feb 12, 2024 at 2:29 PM Pavel Luzanov <p.luzanov@postgrespro.ru> wrote:

 regress_du_role1 | no    | Inherit     | no        | 2024-12-31 00:00:00+03(invalid) | 50               | Group role without password but with valid until regress_du_role2 | yes   | Inherit     | yes       |                                 | Not allowed      | No connections allowed regress_du_role3 | yes   |             | yes       |                                 | 10               | User without attributes regress_du_su    | yes   | Superuser  +| yes       |                                 | 3(ignored)       | Superuser but with connection limit

Per the recent bug report, we should probably add something like (ignored) after the 50 connections for role1 since they are not allowed to login so the value is indeed ignored. 

Hm, but the same logic applies to "Password?" and "Valid until" for role1 without login attribute.
The challenge is how to display it for unprivileged users. But they can't see password information.
So, displaying 'Valid until' as '(irrelevant)' for privileged users and real value for others looks badly.

What can be done in this situation.
0. Show different values as described above.
1. Don't show 'Valid until' for unprivileged users at all. The same logic as for 'Password?'.
With possible exception: user can see 'Valid until' for himself. 
May be too complicated?

2. Tom's advise:  

Not sure it's worth worrying about

Show real values for 'Valid until' and 'Connection limit' without any hints.

3. The best solution, which I can't see now.

-- 
Pavel Luzanov
Postgres Professional: https://postgrespro.com

> On 18 Mar 2024, at 14:29, Sean <iihero@qq.com> wrote:

> Need some document to make a clarification or suggestion to the user?

The suggestion is to not use password authentication but instead use SCRAM.

--
Daniel Gustafsson

> On 18 Mar 2024, at 14:43, Sean <iihero@qq.com> wrote:
>
> Hi,
> Thanks for your information.  Even using SCRAM,  when specified the content of "password",  still there is a basic
requestabout the length of it.  From the source code, seems there is no restriction, right?　 

SCRAM stores a hashed fixed-size representation of the password, so there is no
restriction in terms of length on the user supplied secret.

--
Daniel Gustafsson

I think we can change the output like this:

postgres=# \du
                                List of roles
 Role name | Login | Attributes  | Password | Valid until | Connection limit 
-----------+-------+-------------+----------+-------------+------------------
 test      |       | Inherit     |          |             | 
 test2     | Can   | Inherit     | Has      |             | 
 wenyi     | Can   | Superuser  +|          |             | 
           |       | Create DB  +|          |             | 
           |       | Create role+|          |             | 
           |       | Inherit    +|          |             | 
           |       | Replication+|          |             | 
           |       | Bypass RLS  |          |             | 
(3 rows)

And I submit my the patch, have a look?
Yours,
Wen Yi

------------------ Original ------------------
From:
    "Pavel Luzanov"
<p.luzanov@postgrespro.ru>;
Date: Sun, Feb 18, 2024 07:14 PM
To: "David G. Johnston"<david.g.johnston@gmail.com>;
Cc: "Tom Lane"<tgl@sss.pgh.pa.us>;"Jim Nasby"<jim.nasby@gmail.com>;"Robert
Haas"<robertmhaas@gmail.com>;"pgsql-hackers"<pgsql-hackers@lists.postgresql.org>;
Subject: Re: Things I don't like about \du's "Attributes" column



On 17.02.2024 00:37, David G. Johnston wrote:
 
 
 
On Mon, Feb 12, 2024 at 2:29 PM Pavel Luzanov <p.luzanov@postgrespro.ru> wrote:
 
 
     regress_du_role1 | no| Inherit | no| 2024-12-31 00:00:00+03(invalid) | 50 | Group role without password but with
validuntilregress_du_role2 | yes | Inherit | yes | | Not allowed| No connections allowedregress_du_role3 | yes | | yes
|| 10 | User without attributesregress_du_su| yes | Superuser+| yes | | 3(ignored) | Superuser but with connection
limit
 
   
 Per the recent bug report, we should probably add something like (ignored) after the 50 connections for role1 since
theyare not allowed to login so the value is indeed ignored. 
 
 
 
   Hm, but the same logic applies to "Password?" and "Valid until" for role1 without login attribute. The challenge is
howto display it for unprivileged users. But they can't see password information. So, displaying 'Valid until' as
'(irrelevant)'for privileged users and real value for others looks badly.What can be done in this situation. 0. Show
differentvalues as described above. 1. Don't show 'Valid until' for unprivileged users at all. The same logic as for
'Password?'.With possible exception: user can see 'Valid until' for himself.May be too complicated?2. Tom's advise:
 
  Not sure it's worth worrying about
  Show real values for 'Valid until' and 'Connection limit' without any hints.3. The best solution, which I can't see
now.
 --Pavel Luzanov Postgres Professional: https://postgrespro.com

I think the output need to change, like this:

postgres=# \du+
                                       List of roles
 Role name | Login | Attributes  | Password | Valid until | Connection limit | Description 
-----------+-------+-------------+----------+-------------+------------------+-------------
 test      |       | Inherit     |          |             |                  | 
 test2     | Can   | Inherit     | Has      |             |                  | 
 wenyi     | Can   | Superuser  +|          |             |                  | 
           |       | Create DB  +|          |             |                  | 
           |       | Create role+|          |             |                  | 
           |       | Inherit    +|          |             |                  | 
           |       | Replication+|          |             |                  | 
           |       | Bypass RLS  |          |             |                  | 
(3 rows)

On 16.04.2024 01:06, David G. Johnston wrote:

At this point I'm on board with retaining the \dr charter of simply being an easy way to access the detail exposed in pg_roles with some display formatting but without any attempt to convey how the system uses said information.  Without changing pg_roles.  Our level of effort here, and degree of dependence on superuser, doesn't seem to be bothering people enough to push more radical changes here through and we have good improvements that are being held up in the hope of possible perfection.

I have similar thoughts.
I decided to wait for the end of feature freeze and propose a simpler version
of the patch for v18, without changes in pg_roles.
I hope to send a new version soon.

But about \dr. Is it a typo and you mean \du & \dg?
If we were choosing a name for the command now, then \dr would be ideal:
\dr - display roles
\drg - display role grants

But the long history of \du & \dg prevents from doing so, and creating a third option is too excessive.

-- 
Pavel Luzanov
Postgres Professional: https://postgrespro.com

I think we can change the output like this:

postgres=# \du                                List of roles Role name | Login | Attributes  | Password | Valid until | Connection limit 
-----------+-------+-------------+----------+-------------+------------------ test      |       | Inherit     |          |             |  test2     | Can   | Inherit     | Has      |             |  wenyi     | Can   | Superuser  +|          |             |            |       | Create DB  +|          |             |            |       | Create role+|          |             |            |       | Inherit    +|          |             |            |       | Replication+|          |             |            |       | Bypass RLS  |          |             | 
(3 rows)

And I submit my the patch, have a look?

Thanks for the patch.

As I understand, your patch is based on my previous version.
The main thing I'm wondering about my patch is if we need to change pg_roles,
and it looks like we don't. So, in the next version of my patch,
the Password column will no longer be there.

As for the Login column and its values.
I'm not sure about using "Can" instead of "yes" to represent true.
In other psql commands, boolean values are always shown as yes/no.
NULL instead of false might be possible, but I'd rather check if this approach
has been used elsewhere. I prefer consistency everywhere.

-- 
Pavel Luzanov
Postgres Professional: https://postgrespro.com

On Tue, Apr 16, 2024 at 3:06 AM Pavel Luzanov <p.luzanov@postgrespro.ru> wrote:
> As for the Login column and its values.
> I'm not sure about using "Can" instead of "yes" to represent true.
> In other psql commands, boolean values are always shown as yes/no.
> NULL instead of false might be possible, but I'd rather check if this approach
> has been used elsewhere. I prefer consistency everywhere.

I don't think we can use "Can" to mean "yes". That's going to be
really confusing.

I don't like (irrelevant) either. I know Tom Lane suggested that, but
I think he's got the wrong idea: we should just display the
information we find in the catalogs and let the user decide what is
and isn't relevant. If I see that the connection limit is 40 but the
user can't log in, I can figure out that the value of 40 doesn't
matter. If I see that the connection limit is labelled as (irrelevant)
I don't know why it's labelled that way and, if it were me, I'd likely
end up looking at the source code to figure out why it's showing it
that way.

I think we should go back to the v4 version of this patch, minus the
(ignored) stuff.

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

I think we should go back to the v4 version of this patch, minus the
(ignored) stuff.

Thank you for looking into this.
I can assume that you support the idea of changing pg_roles. It's great.

By the way, I have attached a separate thread[1] about pg_roles to this commitfest entry[2].

I will return to work on the patch after my vacation.

1. https://www.postgresql.org/message-id/flat/db1d94ba-1e6e-4e86-baff-91e6e79071c1%40postgrespro.ru
2. https://commitfest.postgresql.org/48/4738/

-- 
Pavel Luzanov
Postgres Professional: https://postgrespro.com

On 16.04.2024 01:06, David G. Johnston wrote:

At this point I'm on board with retaining the \dr charter of simply being
an easy way to access the detail exposed in pg_roles with some display
formatting but without any attempt to convey how the system uses said
information.  Without changing pg_roles.  Our level of effort here, and
degree of dependence on superuser, doesn't seem to be bothering people
enough to push more radical changes here through and we have good
improvements that are being held up in the hope of possible perfection.

I have similar thoughts.
I decided to wait for the end of feature freeze and propose a simpler version
of the patch for v18, without changes in pg_roles

Since no votes for the changes in pg_roles, please look the simplified version.
We can return to this topic later.

But now there are no changes in pg_roles. Just a special interpretation
of the two values of the "Connection limit" column:  0 - Now allowed (changed from 'No connections') -1 - empty string

Full list of changes in commit message.

Example output:

\du+ regress_du*                                                List of roles    Role name     | Login | Attributes  |         Valid until          | Connection limit |   Description    
------------------+-------+-------------+------------------------------+------------------+------------------ regress_du_admin | yes   | Superuser  +|                              |                  | some description                  |       | Create DB  +|                              |                  |                   |       | Create role+|                              |                  |                   |       | Inherit    +|                              |                  |                   |       | Replication+|                              |                  |                   |       | Bypass RLS  |                              |                  |  regress_du_role0 | yes   | Inherit     | Tue Jun 04 00:00:00 2024 PDT | Not allowed      |  regress_du_role1 | no    | Create role+| infinity                     |                  |                   |       | Inherit     |                              |                  |  regress_du_role2 | yes   | Inherit    +|                              | 42               |                   |       | Replication+|                              |                  |                   |       | Bypass RLS  |                              |                  | 
(4 rows)

Data:
CREATE ROLE regress_du_role0 LOGIN PASSWORD '123' VALID UNTIL '2024-06-04' CONNECTION LIMIT 0;
CREATE ROLE regress_du_role1 CREATEROLE CONNECTION LIMIT -1 VALID UNTIL 'infinity';
CREATE ROLE regress_du_role2 LOGIN REPLICATION BYPASSRLS CONNECTION LIMIT 42;
CREATE ROLE regress_du_admin LOGIN SUPERUSER CREATEROLE CREATEDB BYPASSRLS REPLICATION INHERIT;
COMMENT ON ROLE regress_du_admin IS 'some description';

-- 
Pavel Luzanov
Postgres Professional: https://postgrespro.com

On Thu, Jun 6, 2024 at 5:08 AM Pavel Luzanov <p.luzanov@postgrespro.ru> wrote:
> But now there are no changes in pg_roles. Just a special interpretation
> of the two values of the "Connection limit" column:
>   0 - Now allowed (changed from 'No connections')
>  -1 - empty string

I think the first of these special interpretations is unnecessary and
should be removed. It seems pretty clear what 0 means.

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

I think the first of these special interpretations is unnecessary and
should be removed. It seems pretty clear what 0 means.

Agree.
There is an additional technical argument for removing this replacement.
I don't like explicit cast to text of the "Connection limit" column.
Without 'Not allowed' it is no longer required.
Value -1 can be replaced by NULL with an implicit cast to integer.

Next version with this change attached.

Example output:

\du+ regress_du*                                                List of roles    Role name     | Login | Attributes  |         Valid until          | Connection limit |   Description    
------------------+-------+-------------+------------------------------+------------------+------------------ regress_du_admin | yes   | Superuser  +|                              |                  | some description                  |       | Create DB  +|                              |                  |                   |       | Create role+|                              |                  |                   |       | Inherit    +|                              |                  |                   |       | Replication+|                              |                  |                   |       | Bypass RLS  |                              |                  |  regress_du_role0 | yes   | Inherit     | Tue Jun 04 00:00:00 2024 PDT |                0 |  regress_du_role1 | no    | Create role+| infinity                     |                  |                   |       | Inherit     |                              |                  |  regress_du_role2 | yes   | Inherit    +|                              |               42 |                   |       | Replication+|                              |                  |                   |       | Bypass RLS  |                              |                  | 
(4 rows)

Current version for comparison:
                                          List of roles    Role name     |                         Attributes                         |   Description    
------------------+------------------------------------------------------------+------------------ regress_du_admin | Superuser, Create role, Create DB, Replication, Bypass RLS | some description regress_du_role0 | No connections                                            +|                   | Password valid until 2024-06-04 00:00:00+03                |  regress_du_role1 | Create role, Cannot login                                 +|                   | Password valid until infinity                              |  regress_du_role2 | Replication, Bypass RLS                                   +|                   | 42 connections                                             | 


Data:
CREATE ROLE regress_du_role0 LOGIN PASSWORD '123' VALID UNTIL '2024-06-04' CONNECTION LIMIT 0;
CREATE ROLE regress_du_role1 CREATEROLE CONNECTION LIMIT -1 VALID UNTIL 'infinity';
CREATE ROLE regress_du_role2 LOGIN REPLICATION BYPASSRLS CONNECTION LIMIT 42;
CREATE ROLE regress_du_admin LOGIN SUPERUSER CREATEROLE CREATEDB BYPASSRLS REPLICATION INHERIT;
COMMENT ON ROLE regress_du_admin IS 'some description';

-- 
Pavel Luzanov
Postgres Professional: https://postgrespro.com

On Thu, Jun 6, 2024 at 5:10 PM Pavel Luzanov <p.luzanov@postgrespro.ru> wrote:
> Agree.
> There is an additional technical argument for removing this replacement.
> I don't like explicit cast to text of the "Connection limit" column.
> Without 'Not allowed' it is no longer required.
> Value -1 can be replaced by NULL with an implicit cast to integer.

Yeah, +1 for that idea.

> Example output:
>
> \du+ regress_du*
>                                                 List of roles
>     Role name     | Login | Attributes  |         Valid until          | Connection limit |   Description
> ------------------+-------+-------------+------------------------------+------------------+------------------
>  regress_du_admin | yes   | Superuser  +|                              |                  | some description
>                   |       | Create DB  +|                              |                  |
>                   |       | Create role+|                              |                  |
>                   |       | Inherit    +|                              |                  |
>                   |       | Replication+|                              |                  |
>                   |       | Bypass RLS  |                              |                  |
>  regress_du_role0 | yes   | Inherit     | Tue Jun 04 00:00:00 2024 PDT |                0 |
>  regress_du_role1 | no    | Create role+| infinity                     |                  |
>                   |       | Inherit     |                              |                  |
>  regress_du_role2 | yes   | Inherit    +|                              |               42 |
>                   |       | Replication+|                              |                  |
>                   |       | Bypass RLS  |                              |                  |
> (4 rows)

This seems unobjectionable to me. I am not sure whether it is better
than the current verison, or whether it is what we want. But it seems
reasonable.

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

On 07.06.2024 15:35, Robert Haas wrote:

This seems unobjectionable to me. I am not sure whether it is better
than the current verison, or whether it is what we want. But it seems
reasonable.

I consider this patch as a continuation of the work on \drg command,
when it was decided to remove the "Member of" column from \du command. 

Without "Member of" column, the output of the \du command looks very short.
Only two columns: "Role name" and "Attributes". All the information about
the role is collected in just one "Attributes" column and it is not presented
in the most convenient and obvious way. What exactly is wrong with
the Attribute column Tom wrote in the first message of this thread and I agree
with these arguments.

The current implementation offers some solutions for 3 of the 4 issues
mentioned in Tom's initial message. Issue about display of rolvaliduntil
can't be resolved without changing pg_roles (or executing different queries
for different users).

Therefore, I think the current patch offers a better version of the \du command.
However, I admit that these improvements are not enough to accept the patch.
I would like to hear other opinions.

-- 
Pavel Luzanov
Postgres Professional: https://postgrespro.com

On Sat, Jun 8, 2024 at 10:02 AM Pavel Luzanov <p.luzanov@postgrespro.ru> wrote:
> Therefore, I think the current patch offers a better version of the \du command.
> However, I admit that these improvements are not enough to accept the patch.
> I would like to hear other opinions.

Hmm, I don't think I quite agree with this. If people like this
version better than what we have now, that's all we need to accept the
patch. I just don't really want to be the one to decide all by myself
whether this is, in fact, better. So, like you, I would like to hear
other opinions.

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

At Sat, 8 Jun 2024 14:09:11 -0400, Robert Haas <robertmhaas@gmail.com> wrote in 
> On Sat, Jun 8, 2024 at 10:02 AM Pavel Luzanov <p.luzanov@postgrespro.ru> wrote:
> > Therefore, I think the current patch offers a better version of the \du command.
> > However, I admit that these improvements are not enough to accept the patch.
> > I would like to hear other opinions.
> 
> Hmm, I don't think I quite agree with this. If people like this
> version better than what we have now, that's all we need to accept the
> patch. I just don't really want to be the one to decide all by myself
> whether this is, in fact, better. So, like you, I would like to hear
> other opinions.

>  regress_du_role0 | yes   | Inherit     | Tue Jun 04 00:00:00 2024 PDT |                0 |
>  regress_du_role1 | no    | Create role+| infinity                     |                  |

I guess that in English, when written as "'Login' = 'yes/no'", it can
be easily understood. However, in Japanese, "'ログイン' = 'はい/いいえ'"
looks somewhat awkward and is a bit difficult to understand at a
glance. "'ログイン' = '可/不可'" (equivalent to "Login is
'can/cannot'") sounds more natural in Japanese, but it was rejected
upthread, and I also don't like 'can/cannot'. To give further
candidates, "allowed/not allowed" or "granted/denied" can be
mentioned, and they would be easier to translate, at least to
Japanese. However, is there a higher likelihood that 'granted/denied'
will be misunderstood as referring to database permissions?

Likewise, "'Valid until' = 'infinity'" (equivalent to "'有効期限' = '
無限'") also sounds awkward. Maybe that's the same in English. I guess
that 'unbounded' or 'indefinite' sounds better, and their Japanese
translation '無期限' also sounds natural. However, I'm not sure we
want to go to that extent in transforming the table.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

On 10.06.2024 09:25, Kyotaro Horiguchi wrote:

I guess that in English, when written as "'Login' = 'yes/no'", it can
be easily understood. However, in Japanese, "'ログイン' = 'はい/いいえ'"
looks somewhat awkward and is a bit difficult to understand at a
glance. "'ログイン' = '可/不可'" (equivalent to "Login is
'can/cannot'") sounds more natural in Japanese, but it was rejected
upthread, and I also don't like 'can/cannot'. To give further
candidates, "allowed/not allowed" or "granted/denied" can be
mentioned, and they would be easier to translate, at least to
Japanese. However, is there a higher likelihood that 'granted/denied'
will be misunderstood as referring to database permissions?

Thank you for looking into this, translation is important.

What do you think about the following options?

1. Try to find a more appropriate name for the column.
Maybe "Can login?" is better suited for yes/no and Japanese translation?

2. Show the value only for true, for example "Granted" as you suggested. 
Do not show the "false" value at all. This will be consistent
with the "Attributes" column, which shows only enabled values.

I would prefer the first option and look for the best name for the column.
The second option can also be implemented if we сhoose a value for 'true'.

BTW, I went through all the \d* commands and looked at how columns with
logical values are displayed. There are two approaches: yes/no and t/f.

yes/no
\dAc "Default?"
\dc "Default?"
\dC "Implicit?"
\dO "Deterministic?"

t/f
\dL "Trusted", "Internal language"
\dRp "All tables", "Inserts" "Updates" "Deletes" "Truncates" "Via root"
\dRs "Enabled", "Binary", "Disable on error", "Password required", "Run as owner?", "Failover"

Likewise, "'Valid until' = 'infinity'" (equivalent to "'有効期限' = '
無限'") also sounds awkward. Maybe that's the same in English. I guess
that 'unbounded' or 'indefinite' sounds better, and their Japanese
translation '無期限' also sounds natural. However, I'm not sure we
want to go to that extent in transforming the table.

'infinity' is the value in the table as any other dates.
As far as I understand, it is not translatable.
So you'll see '有効期限' = 'infinity'.


But this can be implemented using the following expression:

       case when rolvaliduntil = 'infinity' or rolvaliduntil is null then         'unbounded' -- translatable value       else         rolvaliduntil::pg_catalog.text       end

Or we can hide 'infinity':
       case when rolvaliduntil = 'infinity' then         null       else         rolvaliduntil       end

This is a little bit better, but I don't like both.
We will not be able to distinguish between null and infinity
values in the table.

After all, I think 'infinity' is a rare case for "Valid until".
What is the reason to set 'Valid until' = 'infinity' if the password
is unlimited by default?

Therefore, my opinion here is to leave "infinity" as is, but I am open
to better alternatives.

-- 
Pavel Luzanov
Postgres Professional: https://postgrespro.com

On Thu, 6 Jun 2024 at 23:10, Pavel Luzanov <p.luzanov@postgrespro.ru> wrote:
>
> On 06.06.2024 17:29, Robert Haas wrote:
>
> I think the first of these special interpretations is unnecessary and
> should be removed. It seems pretty clear what 0 means.
>
> Agree.
> There is an additional technical argument for removing this replacement.
> I don't like explicit cast to text of the "Connection limit" column.
> Without 'Not allowed' it is no longer required.
> Value -1 can be replaced by NULL with an implicit cast to integer.
>
> Next version with this change attached.
>
> Example output:
>
> \du+ regress_du*
>                                                 List of roles
>     Role name     | Login | Attributes  |         Valid until          | Connection limit |   Description
> ------------------+-------+-------------+------------------------------+------------------+------------------
>  regress_du_admin | yes   | Superuser  +|                              |                  | some description
>                   |       | Create DB  +|                              |                  |
>                   |       | Create role+|                              |                  |
>                   |       | Inherit    +|                              |                  |
>                   |       | Replication+|                              |                  |
>                   |       | Bypass RLS  |                              |                  |
>  regress_du_role0 | yes   | Inherit     | Tue Jun 04 00:00:00 2024 PDT |                0 |
>  regress_du_role1 | no    | Create role+| infinity                     |                  |
>                   |       | Inherit     |                              |                  |
>  regress_du_role2 | yes   | Inherit    +|                              |               42 |
>                   |       | Replication+|                              |                  |
>                   |       | Bypass RLS  |                              |                  |
> (4 rows)
>
> Current version for comparison:
This looks much better than the current version. Only thing is, I find
the column name Valid until confusing. With that name I am in danger
of taking it as the role's validity and not the passwords'.
How about naming it to something like Password validity...?
>
>                                           List of roles
>     Role name     |                         Attributes                         |   Description
> ------------------+------------------------------------------------------------+------------------
>  regress_du_admin | Superuser, Create role, Create DB, Replication, Bypass RLS | some description
>  regress_du_role0 | No connections                                            +|
>                   | Password valid until 2024-06-04 00:00:00+03                |
>  regress_du_role1 | Create role, Cannot login                                 +|
>                   | Password valid until infinity                              |
>  regress_du_role2 | Replication, Bypass RLS                                   +|
>                   | 42 connections                                             |
>
>
> Data:
> CREATE ROLE regress_du_role0 LOGIN PASSWORD '123' VALID UNTIL '2024-06-04' CONNECTION LIMIT 0;
> CREATE ROLE regress_du_role1 CREATEROLE CONNECTION LIMIT -1 VALID UNTIL 'infinity';
> CREATE ROLE regress_du_role2 LOGIN REPLICATION BYPASSRLS CONNECTION LIMIT 42;
> CREATE ROLE regress_du_admin LOGIN SUPERUSER CREATEROLE CREATEDB BYPASSRLS REPLICATION INHERIT;
> COMMENT ON ROLE regress_du_admin IS 'some description';
>
> --
> Pavel Luzanov
> Postgres Professional: https://postgrespro.com

-- 
Regards,
Rafia Sabih

This looks much better than the current version. 

Thank you, for looking into this.

Only thing is, I find
the column name Valid until confusing. With that name I am in danger
of taking it as the role's validity and not the passwords'.
How about naming it to something like Password validity...?

Yes, my first attempt was to name this column "Password expiration date"
for the same reason.

But then I decided that the column name should match the attribute name.
Otherwise, you need to make some effort to understand which columns
of the table correspond to which attributes of the roles.

It is also worth considering translation into other languages.
If the role attribute and the column have the same name, then they will
probably be translated the same way. But the translation may be different
for different terms, which will further confuse the situation.

We can probably change the column name, but still the root of the confusion
is caused by the attribute name, not the column name.

What do you think?

-- 
Pavel Luzanov
Postgres Professional: https://postgrespro.com

On Fri, 12 Jul 2024 at 09:06, Pavel Luzanov <p.luzanov@postgrespro.ru> wrote:
>
> On 11.07.2024 15:07, Rafia Sabih wrote:
>
> This looks much better than the current version.
>
> Thank you, for looking into this.
>
> Only thing is, I find
> the column name Valid until confusing. With that name I am in danger
> of taking it as the role's validity and not the passwords'.
> How about naming it to something like Password validity...?
>
> Yes, my first attempt was to name this column "Password expiration date"
> for the same reason.
>
> But then I decided that the column name should match the attribute name.
> Otherwise, you need to make some effort to understand which columns
> of the table correspond to which attributes of the roles.
>
> It is also worth considering translation into other languages.
> If the role attribute and the column have the same name, then they will
> probably be translated the same way. But the translation may be different
> for different terms, which will further confuse the situation.
>
> We can probably change the column name, but still the root of the confusion
> is caused by the attribute name, not the column name.
>
> What do you think?
Yes you are right in this. I too carry the opinion that column names
should be the same as attribute names as much as possible.
So, then it is good that way.

Other thoughts came to my mind, should we have this column in \du+
instead, maybe connection limit too.
I know in the current version we have all this in \du itself, but then
all clubbed in one column. But now since
our table has got wider, it might be aesthetic to have it in the
extended version. Also, their usage wise might not
be the first thing to be looked at for a user/role.

What are your thoughts on that?
>
> --
> Pavel Luzanov
> Postgres Professional: https://postgrespro.com



-- 
Regards,
Rafia Sabih

Other thoughts came to my mind, should we have this column in \du+
instead, maybe connection limit too.
I know in the current version we have all this in \du itself, but then
all clubbed in one column. But now since
our table has got wider, it might be aesthetic to have it in the
extended version. Also, their usage wise might not
be the first thing to be looked at for a user/role.

In the first version of the patch, "Valid until" (named "Password expire time")
and "Connection limit" ("Max connections") columns were in extended mode. [1]

Later we decided to place each attribute in the "Attributes" column on a separate
line. This allowed us to significantly reduce the overall width of the output.
So, I decided to move "Valid until" and "Connection limit" from extended mode
to normal mode.

Just compare output from patched \du and popular \list command:

postgres@demo(17.0)=# \du                                List of roles Role name | Login | Attributes  |      Valid until       | Connection limit 
-----------+-------+-------------+------------------------+------------------ alice     | yes   | Inherit     | 2024-06-30 00:00:00+03 |                  bob       | yes   | Inherit     | infinity               |                  charlie   | yes   | Inherit     |                        |               11 postgres  | yes   | Superuser  +|                        |                            |       | Create DB  +|                        |            |       | Create role+|                        |            |       | Inherit    +|                        |            |       | Replication+|                        |            |       | Bypass RLS  |                        | 
(4 rows)

postgres@demo(17.0)=# \list                                                     List of databases   Name    |  Owner   | Encoding | Locale Provider |   Collate   |    Ctype    | Locale | ICU Rules |   Access privileges   
-----------+----------+----------+-----------------+-------------+-------------+--------+-----------+----------------------- demo      | postgres | UTF8     | libc            | en_US.UTF-8 | en_US.UTF-8 |        |           |  postgres  | postgres | UTF8     | libc            | en_US.UTF-8 | en_US.UTF-8 |        |           |  template0 | postgres | UTF8     | libc            | en_US.UTF-8 | en_US.UTF-8 |        |           | =c/postgres          +           |          |          |                 |             |             |        |           | postgres=CTc/postgres template1 | postgres | UTF8     | libc            | en_US.UTF-8 | en_US.UTF-8 |        |           | =c/postgres          +           |          |          |                 |             |             |        |           | postgres=CTc/postgres
(4 rows)

If we decide to move "Valid until" and "Connection limit" to extended mode,
then the role attributes should be returned to their previous form by placing
them on one line separated by commas.

1. https://www.postgresql.org/message-id/27f87cb9-229b-478b-81b2-157f94239d55%40postgrespro.ru

-- 
Pavel Luzanov
Postgres Professional: https://postgrespro.com

On Sat, 13 Jul 2024 at 14:21, Pavel Luzanov <p.luzanov@postgrespro.ru> wrote:
>
> On 12.07.2024 12:22, Rafia Sabih wrote:
>
> Other thoughts came to my mind, should we have this column in \du+
> instead, maybe connection limit too.
> I know in the current version we have all this in \du itself, but then
> all clubbed in one column. But now since
> our table has got wider, it might be aesthetic to have it in the
> extended version. Also, their usage wise might not
> be the first thing to be looked at for a user/role.
>
> In the first version of the patch, "Valid until" (named "Password expire time")
> and "Connection limit" ("Max connections") columns were in extended mode. [1]
>
> Later we decided to place each attribute in the "Attributes" column on a separate
> line. This allowed us to significantly reduce the overall width of the output.
> So, I decided to move "Valid until" and "Connection limit" from extended mode
> to normal mode.
>
> Just compare output from patched \du and popular \list command:
>
> postgres@demo(17.0)=# \du
>                                 List of roles
>  Role name | Login | Attributes  |      Valid until       | Connection limit
> -----------+-------+-------------+------------------------+------------------
>  alice     | yes   | Inherit     | 2024-06-30 00:00:00+03 |
>  bob       | yes   | Inherit     | infinity               |
>  charlie   | yes   | Inherit     |                        |               11
>  postgres  | yes   | Superuser  +|                        |
>            |       | Create DB  +|                        |
>            |       | Create role+|                        |
>            |       | Inherit    +|                        |
>            |       | Replication+|                        |
>            |       | Bypass RLS  |                        |
> (4 rows)
>
> postgres@demo(17.0)=# \list
>                                                      List of databases
>    Name    |  Owner   | Encoding | Locale Provider |   Collate   |    Ctype    | Locale | ICU Rules |   Access
privileges
>
-----------+----------+----------+-----------------+-------------+-------------+--------+-----------+-----------------------
>  demo      | postgres | UTF8     | libc            | en_US.UTF-8 | en_US.UTF-8 |        |           |
>  postgres  | postgres | UTF8     | libc            | en_US.UTF-8 | en_US.UTF-8 |        |           |
>  template0 | postgres | UTF8     | libc            | en_US.UTF-8 | en_US.UTF-8 |        |           | =c/postgres
    +
 
>            |          |          |                 |             |             |        |           |
postgres=CTc/postgres
>  template1 | postgres | UTF8     | libc            | en_US.UTF-8 | en_US.UTF-8 |        |           | =c/postgres
    +
 
>            |          |          |                 |             |             |        |           |
postgres=CTc/postgres
> (4 rows)
>
> If we decide to move "Valid until" and "Connection limit" to extended mode,
> then the role attributes should be returned to their previous form by placing
> them on one line separated by commas.
>
> 1. https://www.postgresql.org/message-id/27f87cb9-229b-478b-81b2-157f94239d55%40postgrespro.ru
>
> --
> Pavel Luzanov
> Postgres Professional: https://postgrespro.com

Well, it was just my opinion of how I would have liked it better, but
of course you may decide against it, there is no strong feeling around
it.
And if you are on the fence with the opinion of having them in normal
or extended mode, then maybe we can ask more people to chip in.

I certainly would not want you to update the patch back and forth for
almost no reason.
-- 
Regards,
Rafia Sabih

Well, it was just my opinion of how I would have liked it better, but
of course you may decide against it, there is no strong feeling around
it.
And if you are on the fence with the opinion of having them in normal
or extended mode, then maybe we can ask more people to chip in.

I am not against moving "Valid until" and "Connection limit" to extended mode.
It just seemed to me that without these two columns, the output of the command
is too short, so there is no reason in hiding them.

But you are right, we need more opinions.

-- 
Pavel Luzanov
Postgres Professional: https://postgrespro.com

On Tue, Jul 16, 2024 at 4:53 AM Pavel Luzanov <p.luzanov@postgrespro.ru> wrote:
> On 15.07.2024 12:50, Rafia Sabih wrote:
> Well, it was just my opinion of how I would have liked it better, but
> of course you may decide against it, there is no strong feeling around
> it.
> And if you are on the fence with the opinion of having them in normal
> or extended mode, then maybe we can ask more people to chip in.
>
> I am not against moving "Valid until" and "Connection limit" to extended mode.
> It just seemed to me that without these two columns, the output of the command
> is too short, so there is no reason in hiding them.
>
> But you are right, we need more opinions.

Which version of the patch is currently under discussion?

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

Which version of the patch is currently under discussion?

I believe we are talking about the latest v8 patch version. [1]

1. https://www.postgresql.org/message-id/5341835b-e7be-44dc-b6e5-400e9e3f3c64%40postgrespro.ru

-- 
Pavel Luzanov
Postgres Professional: https://postgrespro.com

On Tue, Jul 16, 2024 at 9:48 AM Pavel Luzanov <p.luzanov@postgrespro.ru> wrote:
> Which version of the patch is currently under discussion?
>
> I believe we are talking about the latest v8 patch version. [1]
>
> 1. https://www.postgresql.org/message-id/5341835b-e7be-44dc-b6e5-400e9e3f3c64%40postgrespro.ru

Thanks. For some reason (likely me being dumb) I was having a hard
time finding that in the thread.

On the question of display width, my personal opinion is that the
current patch is worse than what we have now. Right now, if I type
\du, the output fits in an 80-column terminal unless the role names
are quite long, and \du+ fits except for the Description field, which
wraps. With the patch, even \du wraps in an 80-column terminal. "Role
name", "Login," "Attributes," and "Valid until" fit, but "Connection
limit" doesn't. I know some people think optimizing for 80-column
terminals is obsolete in 2024, and I often use a wider window myself,
but I do still appreciate it when I don't have to make the window
wider to read stuff. Especially, if I didn't use + when running a psql
command, I'd prefer for it to fit.

One solution could be to move "Valid until" or "Connection limit" or
both to verbose mode, as proposed by Rafia. But that's also a
regression over what we have now, where the output fits in 80 columns
and includes that information.

I'm starting to have some doubts about whether this effort is really
worthwhile. It seems like what we have right now is a patch which uses
both more horizontal space and more vertical space than the current
implementation, without (IMHO) really offering any clear advantage. I
know this started out as an effort to address Tom's complaints in the
original post, but it feels like we're losing as much as we're
gaining, and Tom seems to have lost interest in the thread, too.

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

On 16.07.2024 18:00, Robert Haas wrote:

On the question of display width, my personal opinion is that the
current patch is worse than what we have now.

Robert, David, thanks for the detailed explanation.

I tried to remember all the thoughts that led to this  version of the patch.

So the main issue that Robert points out is that the output of the command
takes up more space compared to the current version.
(But I'm ready to debate that too :-), see below.)

In the proposed version, columns for rolconnlimit and rolvaliduntil occupy
a significant place. It really is. We can hide them in extended mode, but
they still take up a lot of space. In the current command, these attributes
are very compactly arranged in the "Attributes" column on separate lines.

However, the current placement of rolconnlimit and rolvaliduntil on separate
lines is very bad, which Tom noted in the first letter and I completely
agree with this. Also, I don't like that the values appear only if they
differ from the default values. It's more compact, but less intuitive.
It seems to me that this approach is not used anywhere else in other
\d* commands (but I may be wrong, I did not check).

Let me explain why I think rolconnlimit and rolvaliduntil are worthy
of being placed as separate columns.

1. Logical attributes (rolsuper, rolinherit, rolcreaterole, rolcreatedb,
rolcanlogin, rolreplication, rolbypassrls) are uniform in nature and
presenting them as a list in one column looks logical.
But rolconnlimit and rolvaliduntil do not fit into this company in any way.
They are strangers here in terms of data type and meaning.

2. Logical attributes give the role additional capabilities,
while rolconnlimit and rolvaliduntil rather limit the use of the role.

3. After switching to a role with the SET ROLE command, you can use
the capabilities of logical attributes, but the restrictions of rolconnlimit
and rolvaliduntil do not apply to SET ROLE:

postgres@demo(17.0)=# grant bob to alice;
grant bob to alice;
GRANT ROLE
postgres@demo(17.0)=# alter role bob connection limit 0;
alter role bob connection limit 0;
ALTER ROLE
postgres@demo(17.0)=# \c - bob
connection to server on socket "/tmp/.s.PGSQL.5401" failed: FATAL:  too many connections for role "bob"
Previous connection kept
postgres@demo(17.0)=# \c - alice 
You are now connected to database "demo" as user "alice".
alice@demo(17.0)=> set role bob;
set role bob;
SET

This makes it reasonable to consider rolconnlimit and rolvaliduntil
as separate properties of a role, rather than together with logical
attributes.

Now the hard part. What to do with the width of the command output?
I also think that it is desirable to fit the output of any command in 80
characters. And I was calm when I saw the 78-character output in my test
system:

postgres@demo(17.0)=# \du                                List of roles Role name | Login | Attributes  |      Valid until       | Connection limit 
-----------+-------+-------------+------------------------+------------------ alice     | yes   | Inherit     | 2024-06-30 00:00:00+03 |                  bob       | yes   | Inherit     | infinity               |                  charlie   | yes   | Inherit     |                        |                1 postgres  | yes   | Superuser  +|                        |                            |       | Create DB  +|                        |            |       | Create role+|                        |            |       | Inherit    +|                        |            |       | Replication+|                        |            |       | Bypass RLS  |                        | 
(4 rows)

But, really, the width can exceed 80 with longer role names, as well as with
a wider default date output. Compare with the date output in the patch regression
tests:

2024-06-30 00:00:00+03
Tue Jun 04 00:00:00 2024 PDT

To be fair, I must say that among the \d* commands there are many commands
whose output width exceeds 80 characters.
Namely: \da, \dAc, \dAf, \dAo, \dAp, \dC, \df, \di, \do, \dO, \dRp, \dT, \l

But let's go back to the current version. I consider this patch as
a continuation of the work on the \drg command that appeared in version 16.
As part of that work, we removed the "Member of" column from the \du command
and introduced a new \drg command to show membership in roles.
From my point of view, the \du command is currently in an intermediate and
unfinished state. Therefore, it is more correct to compare the proposed patch
with psql 15, rather than 16.
(I know there is nothing more permanent than a temporary solution,
but give me hope :-).)

In version 15, the output of the \du command is wider than the proposed version!

15=# \du                                   List of roles Role name |                         Attributes                         | Member of 
-----------+------------------------------------------------------------+----------- postgres  | Superuser, Create role, Create DB, Replication, Bypass RLS | {}

And this is with only one role. I can assume that there are usually several
roles in systems and role membership is actively used to organize roles within
groups. Therefore, in real systems, the output of the \du command in version 15
is probably much wider. For example, output together with system objects:

15=# \duS                                                                     List of roles         Role name         |                         Attributes                         |                          Member of                           
---------------------------+------------------------------------------------------------+-------------------------------------------------------------- pg_execute_server_program | Cannot login                                               | {} pg_monitor                | Cannot login                                               | {pg_read_all_settings,pg_read_all_stats,pg_stat_scan_tables} pg_read_all_settings      | Cannot login                                               | {} pg_read_all_stats         | Cannot login                                               | {} pg_read_server_files      | Cannot login                                               | {} pg_signal_backend         | Cannot login                                               | {} pg_stat_scan_tables       | Cannot login                                               | {} pg_write_server_files     | Cannot login                                               | {} postgres                  | Superuser, Create role, Create DB, Replication, Bypass RLS | {}

All this allows me to believe that the proposed version has advantages over
the current version of the \du command:
- Solutions have been proposed for 3 of the 4 Tom's complaints.
- The new "Login" column separates users from group roles, which is very useful (imho).
- Tabular output is convenient to view both in normal mode and in expanded mode (\x).  The last line contains information about the number of roles.
- Refactoring: code has become much simpler and clearer.

But if we don't find a compromise and just leave it as it is, then that's fine.
So the time for change has not come yet. In any case, this discussion may be useful
in the future. But who knows, maybe now we can come to some kind of agreement.

-- 
Pavel Luzanov
Postgres Professional: https://postgrespro.com

Hi Pavel,

First, thanks for your dedication to this effort. I always find it
hard to make time for things like psql backslash command improvements,
but I'm glad that we have people in our community who work on such
things.

Second, I think that the threshold question for this patch is: will
users, on average, be happier if this patch gets committed? If the
answer is yes, then the patch should be committed, and if the answer
is no, the patch should not be committed. But I actually don't really
have any clear idea of what users in general are likely to think. My
own reaction is essentially ... meh. I do not think that the proposed
new output is massively worse than what we have now, but I also don't
think it's a whole lot better. Now, if a bunch of other people show up
and vote, well then we'll have a much better view of what the typical
user is likely to think. But right now, I can't hazard a guess as to
what the general opinion will be, and therefore I'm unprepared to
commit anything. Because, and I can't say this often enough, it's not
all about me. Even if I thought that this patch was way better than
what we have now, I still wouldn't commit it unless I was relatively
confident that other people would agree.

Third, if I can back away from this particular patch for a moment, I
feel like roles and permissions are one of the weaker areas in psql. I
feel, and I suspect most users agree, that the output of "\d my_table"
is top notch. It tells you everything that you need to know about a
particular table -- all the columns, column types, default values,
triggers, indexes, and whatever else there is. If someone adds a new
object that attaches to a table, they're going to understand that they
need to add a listing for that object to the \d output, and users are
going to understand that they should look for it there. That unstated
contract between developers and users is a thing of beauty: without
any words being said, there is a meeting of the minds. But I don't
think the same thing can be said around roles.

For a long time, one of my big gripes in this area has been \z. It
displays information about the permissions of table-like objects. But
I don't really imagine that being a thing that a user is actually
going to want to see. I feel like there are two typical use cases
here. One is you want to see all the privileges associated with a
table. In that case, you'd like the information to show up in the
output of \d or \d+. The other is that you want to see the privileges
associated with a particular user -- and in that case you want to see
not just the table privileges but the privileges on every other kind
of database object, too. I'm not saying there's a single PostgreSQL
user anywhere who has wanted to list information about tables with
names matching a certain wildcard and see just the privilege
information for each one, but I bet it's rare. I also suspect that
only truly hard-core PostgreSQL fans want to see incantations like
"robert.haas"=arwdDxtm/"robert.haas" in their output.

So, personally, if I were going to work on a redesign in this area, I
would look into making \du <username> work like \d <tablename>. That
is, it would tell you every single thing there is to know about a
user. Role attributes. Roles in which this role has membership. Roles
that are a member of this row. Objects of all sorts this object owns.
Permissions this role has on objects of all sorts. Role settings. All
of it in SQL-ish format like we do with the footer when you run \d.
Then I would make \du work like \d: a minimal amount of basic
information about every role in the list, like whether it's a
superuser and whether they can log in. Then, I would consider removing
\dp \drds \drg and \z entirely. I don't know if a plan like that would
actually work out well. A particular problem is that if a user owns
10k objects, we don't want to just list them all one per line. Maybe
when the number of objects owned is large, we could just give a count
per object type unless + is used,  or something like that. But even if
all of this could be sorted out, would users in general like it
better, or would it just make Robert happy? Would even Robert end up
liking the result?

I don't really know, but I think that my general discontent with \du
\dg \dp \drds \drg \z is part of why I find it hard to evaluate a
patch like this. I look forward to seeing the output of "\d <table>"
on whatever table is causing some customer a problem today. I never
look forward to seeing the output of \du. Is that just me?

...Robert

Robert,

I am pleased that you are paying so much attention to this patch.

Second, I think that the threshold question for this patch is: will
users, on average, be happier if this patch gets committed? If the
answer is yes, then the patch should be committed, and if the answer
is no, the patch should not be committed. But I actually don't really
have any clear idea of what users in general are likely to think. My
own reaction is essentially ... meh. I do not think that the proposed
new output is massively worse than what we have now, but I also don't
think it's a whole lot better. Now, if a bunch of other people show up
and vote, well then we'll have a much better view of what the typical
user is likely to think.

I share your opinion that the need for a patch should be decided
by the votes (or lack of votes) of practicing experts. I am mainly
involved in educational projects, so in most cases I work with
demo systems. Therefore, I'm not sure that the patch I'm offering
will make users happy. Perhaps it should be withdrawn.

Third, if I can back away from this particular patch for a moment, I
feel like roles and permissions are one of the weaker areas in psql.

So, personally, if I were going to work on a redesign in this area, I
would look into making \du <username> work like \d <tablename>. That
is, it would tell you every single thing there is to know about a
user. Role attributes. Roles in which this role has membership. Roles
that are a member of this row. Objects of all sorts this object owns.
Permissions this role has on objects of all sorts. Role settings. All
of it in SQL-ish format like we do with the footer when you run \d.

Oh, that's very interesting. I will think about this approach,
but I do not know when and what result can be obtained...

But let me share my thoughts on roles, privileges and system catalogs
from a different angle. This has nothing to do with the current patch,
I just want to share my thoughts.

I came to PostgreSQL from Oracle and it was unexpected for me that users
had almost complete access to the contents of the system сatalogs.
With rare exceptions (pg_authid, pg_statistic), any unprivileged user sees
the full contents of any system сatalog. (I'm not saying that access to system
catalogs needs to be reworked, it's probably impossible or very difficult.)

Visible but inaccessible objects in system catalogs increase the volume
of command output unnecessarily. Why do I need to know the list of all
schemas in the database if I only have access to the public schema?
The same applies to inaccessible tables, views, functions, etc.

Not for safety, but for convenience, it might be worth having a set of views
that show only those rows of the system catalog (with *acl column) that
the user has access to. Either as the object owner, or through the privileges.
Directly or indirectly through role membership.

By the way, this is exactly the approach implemented for the information
schema. Here is a code fragment of the information_schema.schemata view:

SELECT ...   FROM pg_namespace n,    pg_authid u  WHERE n.nspowner = u.oid AND        (pg_has_role(n.nspowner, 'USAGE'::text) OR        has_schema_privilege(n.oid, 'CREATE, USAGE'::text))

Then the commands like \dt, \df, \dn, \l, etc might use these views and show
only the objects accessible to the user. To do this, a new modifier to
the commands can be implemented, similar to the S modifier for system objects.

For example:
\dn  - list of all schemas
\dnA - list of accessible schemas

In some way this approach can resolve your issue about roles and privileges.
Familiar psql commands will be able to display only the objects accessible
for current role, without pushing the whole output into \du.

Such a set of views can be useful not only in psql, but also for third-party
applications.

I think I'm not the first one trying to bikeshedding in this area.
It's probably been discussed many times why this should not be done.
But such thoughts do come, and I don't know the answer yet.

-- 
Pavel Luzanov
Postgres Professional: https://postgrespro.com

On Mon, Jul 22, 2024 at 5:19 PM Pavel Luzanov <p.luzanov@postgrespro.ru> wrote:
> Visible but inaccessible objects in system catalogs increase the volume
> of command output unnecessarily. Why do I need to know the list of all
> schemas in the database if I only have access to the public schema?
> The same applies to inaccessible tables, views, functions, etc.
>
> Not for safety, but for convenience, it might be worth having a set of views
> that show only those rows of the system catalog (with *acl column) that
> the user has access to. Either as the object owner, or through the privileges.
> Directly or indirectly through role membership.

So, I wasn't actually aware that anyone had a big problem in this
area. I thought that most of the junk you might see in \d<whatever>
output would be hidden either because the objects you don't care about
are not in your search_path or because they are system objects. I
agree that doesn't help with schemas, but most people don't have a
huge number of schemas, and even if you do, you don't necessarily need
to look at the list all that frequently.

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

On Mon, Jul 22, 2024 at 5:19 PM Pavel Luzanov <p.luzanov@postgrespro.ru> wrote:

Visible but inaccessible objects in system catalogs increase the volume
of command output unnecessarily. Why do I need to know the list of all
schemas in the database if I only have access to the public schema?
The same applies to inaccessible tables, views, functions, etc.

Not for safety, but for convenience, it might be worth having a set of views
that show only those rows of the system catalog (with *acl column) that
the user has access to. Either as the object owner, or through the privileges.
Directly or indirectly through role membership.

So, I wasn't actually aware that anyone had a big problem in this
area. I thought that most of the junk you might see in \d<whatever>
output would be hidden either because the objects you don't care about
are not in your search_path or because they are system objects. I
agree that doesn't help with schemas, but most people don't have a
huge number of schemas, and even if you do, you don't necessarily need
to look at the list all that frequently.

Maybe.
But it would be better not to see unnecessary objects in the system catalogs.
Especially for GUI tools.

Back to the subject.

So, personally, if I were going to work on a redesign in this area, I
would look into making \du <username> work like \d <tablename>. That
is, it would tell you every single thing there is to know about a
user. Role attributes. Roles in which this role has membership. Roles
that are a member of this row. Objects of all sorts this object owns.
Permissions this role has on objects of all sorts. Role settings. All
of it in SQL-ish format like we do with the footer when you run \d.
Then I would make \du work like \d: a minimal amount of basic
information about every role in the list, like whether it's a
superuser and whether they can log in.

Yes, I still like this idea.
A little later I will try to make a patch in this direction.

-- 
Pavel Luzanov
Postgres Professional: https://postgrespro.com

However, I do think we are at something committable, though I'd make one, maybe two, changes to v8.

Valid until -> Password valid until: the timestamp value already forces a wide column, adding the word Password to the header to clarify what is valid simply provides the same context that the create role page provides when it shows the valid until attribute immediately below the password attribute.  Leaving "valid until" alone retains the attribute name tieback.

Connection limit -> Con. limit: maybe this gets rejected on translation grounds but the abbreviation here seems obvious and shaves 7 characters off the mandatory width for a column that occupies 12 characters more than the values require.

Even without those changes I as reviewer would concur with the proposal and try to move this on from bike-shedding to a go/no-go decision (by marking it Ready to Commit) as to whether this is an improvement over the status quo.

-- 
Pavel Luzanov
Postgres Professional: https://postgrespro.com