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

From: Pavel Luzanov <p(dot)luzanov(at)postgrespro(dot)ru>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Things I don't like about \du's "Attributes" column
Date: 2023-12-30 14:23:40
Message-ID: 27f87cb9-229b-478b-81b2-157f94239d55@postgrespro.ru
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Now I'm ready for discussion.

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

Agree. The negative form looks strange.

Fresh look suggests to move login attribute to own column.
The attribute separates users and group roles, this is very important
information,
it deserves to be placed in a separate column. Of course, it can be
returned back to "Attributes" if such change is very radical.

On the other hand, rolinherit attribute lost its importance in v16.
I don't see serious reasons in changing the default value, so we can
leave it
in the "Attributes" column. In most cases it will be hidden.

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

connlimit attribute moved from "Attributes" column to separate column
"Max connections" in extended mode. But without any modifications to
it's values.
For me it looks normal.

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

Moved from "Attributes" column to separate column  "Password expire time"
in extended mode (+).

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

Done.
The same changes to pg_user.passwd for consistency.
> 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).

I think that writing the value "infinity" in places where there is no
value is
not a good thing. This hides the real value of the column. In addition,
there is no reason to set "infinity" when the password is always valid with
default NULL.

My suggestion to add new column "Has password?" in extended mode with
yes/no values and leave rolvaliduntil values as is.

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

Implemented this approach.

In a result describeRoles function significantly simplified and
rewritten for the convenience
of printing the whole query result. All the magic of building
"Attributes" column
moved to SELECT statement for easy viewing by users via ECHO_HIDDEN
variable.

Here is an example output.

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

v1 of the patch attached. There are no tests and documentation yet.
make check fall in 2 existing tests due changes in pg_roles and \du.
Will be corrected.

Any opinions?

I plan to add a patch to the January commitfest.

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

Attachment Content-Type Size
v1-0001-psql-Rethinking-of-du-command.patch text/x-patch 8.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Isaac Morland 2023-12-30 14:33:59 Re: Things I don't like about \du's "Attributes" column
Previous Message Ranier Vilela 2023-12-30 13:40:11 Re: Windows sockets (select missing events?)