Обсуждение: 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