Re: Things I don't like about \du's "Attributes" column
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, translationis 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'. Butthis can be implemented usingthe followingexpression: 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. Wewill notbe ableto distinguishbetween nullandinfinity values inthe table. After all, I think 'infinity' is a rare case for "Valid until". Whatis the reasonto set'Validuntil'='infinity'ifthe passwordisunlimitedbydefault? Therefore, my opinion here is to leave "infinity" as is, but I am open to better alternatives. -- Pavel Luzanov Postgres Professional:https://postgrespro.com
Re: Things I don't like about \du's "Attributes" column
At Sat, 8 Jun 2024 14:09:11 -0400, Robert Haas wrote in > On Sat, Jun 8, 2024 at 10:02 AM Pavel Luzanov > 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
Re: Things I don't like about \du's "Attributes" column
On Sat, Jun 8, 2024 at 10:02 AM Pavel Luzanov 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
Re: Things I don't like about \du's "Attributes" column
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
Re: Things I don't like about \du's "Attributes" column
On Thu, Jun 6, 2024 at 5:10 PM Pavel Luzanov 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
Re: Things I don't like about \du's "Attributes" column
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 longerrequired.
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
From fd3fb8a4bea89f870789fe63a270f872c945980c Mon Sep 17 00:00:00 2001
From: Pavel Luzanov
Date: Thu, 6 Jun 2024 23:48:32 +0300
Subject: [PATCH v8] psql: Rethinking of \du command
Cnanges in the \du command
- "Login", "Connection limit" and "Valid until" attributes are placed
in separate columns.
to understand that there is no limit.
- The "Attributes" column includes only the enabled logical attributes.
- The attribute names correspond to the keywords of the CREATE ROLE command.
- The attributes are listed in the same order as in the documentation.
- Value -1 for "Connection limit" replaced by NULL to make it easier
- General refactoring of describeRoles function in describe.c.
---
src/bin/psql/describe.c| 146 -
src/test/regress/expected/psql.out | 40 +---
src/test/regress/sql/psql.sql | 12 ++-
3 files changed, 72 insertions(+), 126 deletions(-)
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index f67bf0b892..31cc40b38f 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -36,7 +36,6 @@ static bool describeOneTableDetails(const char *schemaname,
bool verbose);
static void add_tablespace_footer(printTableContent *const cont, char relkind,
Oid tablespace, const bool newline);
-static void add_role_attribute(PQExpBuffer buf, const char *const str);
static bool listTSParsersVerbose(const char *pattern);
static bool describeOneTSParser(const char *oid, const char *nspname,
const char *prsname);
@@ -3615,34 +3614,47 @@ describeRoles(const char *pattern, bool verbose, bool showSystem)
{
PQExpBufferData buf;
PGresult *res;
- printTableContent cont;
-
Re: Things I don't like about \du's "Attributes" column
On Thu, Jun 6, 2024 at 5:08 AM Pavel Luzanov 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
Re: Things I don't like about \du's "Attributes" column
On 16.04.2024 09:15, Pavel Luzanov wrote:
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
featurefreeze 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.
Butnowthere are nochangesinpg_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
From 8cd9a815de36a40d450029d327e013cf69827499 Mon Sep 17 00:00:00 2001
From: Pavel Luzanov
Date: Thu, 6 Jun 2024 11:30:23 +0300
Subject: [PATCH v7] psql: Rethinking of \du command
Cnanges in the \du command
- "Login", "Connection limit" and "Valid until" attributes are placed
in separate columns.
- The "Attributes" column includes only the enabled logical attributes.
- The attribute names correspond to the keywords of the CREATE ROLE command.
- The attributes are listed in the same order as in the documentation.
- A special interpretation of the two values of the "Connection limit" column:
0 - Now allowed
-1 - empty string
- General refactoring of describeRoles function in describe.c.
---
src/bin/psql/describe.c| 149 -
src/test/regress/expected/psql.out | 40 +---
src/test/regress/sql/psql.sql | 12 ++-
3 files changed, 75 insertions(+), 126 deletions(-)
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index f67bf0b892..8967102261 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -36,7 +36,6 @@ static bool describeOneTableDetails(const char *schemaname,
bool verbose);
static void add_tablespace_footer(printTableContent *const cont, char relkind,
Oid tablespace, const bool newline);
-static void add_role_attribute(PQExpBuffer buf, const char *const str);
static bool listTSParsersVerbose(const char *pattern);
static bool describeOneTSParser(const char *oid, const char *nspname,
const char *prsname);
@@ -3615,34 +3614,50 @@ describeRoles(const char *pattern, bool verbose, bool showSystem)
{
PQExpBufferData buf;
PGresult *res;
- printTableContent cont;
- printTableOpt myopt = pset.popt.topt;
- int ncols = 2;
- int nrows = 0;
- int i;
- int conns;
- const char align = 'l';
- char **attr;
-
- myopt.default_footer = false;
+ printQueryOpt myopt = pset.popt;
initPQExpBuffer();
-
Re: Things I don't like about \du's "Attributes" column
On 14.05.2024 19:03, Robert Haas wrote: 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
Re: Things I don't like about \du's "Attributes" column
On Tue, May 14, 2024 at 9:03 AM Robert Haas wrote: > On Tue, Apr 16, 2024 at 3:06 AM Pavel Luzanov > 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. > Agreed > 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. > Or we'd document what we've done and users that don't want to go looking at source code can just read our specification. > I think we should go back to the v4 version of this patch, minus the > (ignored) stuff. > > Agreed, I'm past the point of wanting to have this behave more intelligently rather than a way for people to avoid having to go write a catalog using query themselves. David J.
Re: Things I don't like about \du's "Attributes" column
On Tue, Apr 16, 2024 at 3:06 AM Pavel Luzanov 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
Re: Things I don't like about \du's "Attributes" column
Hi, On 14.04.2024 05:02, Wen Yi wrote: 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
Re: Things I don't like about \du's "Attributes" column
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 featurefreeze 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
Re: Things I don't like about \du's "Attributes" column
On Sun, Feb 18, 2024 at 4:14 AM Pavel Luzanov wrote: > 2. Tom's advise: > > Not sure it's worth worrying about > > Show real values for 'Valid until' and 'Connection limit' without any hints. > > 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. David J.
Re: Things I don't like about \du's "Attributes" column
On Sat, Apr 13, 2024 at 7:02 PM Wen Yi wrote: > 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? > > Why? I actually am generally open to false being encoded as blank where there are only two possible values, but there is no precedence of choosing something besides 'yes' or 'true' to represent the boolean true value. Whether Password is truly two-valued is debatable per the ongoing discussion. David J.
Re: Things I don't like about \du's "Attributes" column
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.luza...@postgrespro.ru; Date:Sun, Feb 18, 2024 07:14 PM To:"David G. Johnston"david.g.johns...@gmail.com; Cc:"Tom Lane"t...@sss.pgh.pa.us;"Jim Nasby"jim.na...@gmail.com;"Robert Haas"robertmh...@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?6?2PM Pavel Luzanov p.luza...@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 untilregress_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 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 v6-0002-psql-Rethinking-of-du-command.patch Description: Binary data
Re: Things I don't like about \du's "Attributes" column
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)
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 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
Re: Things I don't like about \du's "Attributes" column
On 17.02.2024 00:44, Tom Lane wrote: "David G. Johnston" 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 From 80d7038e72d36659fc8453567ce38660fc3a6364 Mon Sep 17 00:00:00 2001 From: Pavel Luzanov Date: Sat, 17 Feb 2024 20:46:32 +0300 Subject: [PATCH v5 1/2] Show password presence in pg_roles 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 . --- src/backend/catalog/system_views.sql | 43 ++-- src/test/regress/expected/rules.out | 37 +++- 2 files changed, 51 insertions(+), 29 deletions(-) diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql index 04227a72d1..acb6668e58 100644 --- a/src/backend/catalog/system_views.sql +++ b/src/backend/catalog/system_views.sql @@ -16,21 +16,34 @@ CREATE VIEW pg_roles AS SELECT -rolname, -rolsuper, -rolinherit, -rolcreaterole, -rolcreatedb, -rolcanlogin, -rolreplication, -rolconnlimit, -''::text as rolpassword, -rolvaliduntil, -rolbypassrls, -setconfig as rolconfig, -pg_authid.oid -FROM pg_authid LEFT JOIN pg_db_role_setting s -ON (pg_authid.oid = setrole AND setdatabase = 0); +r.rolname, +r.rolsuper, +r.rolinherit, +r.rolcreaterole, +r.rolcreatedb, +r.rolcanlogin, +r.rolreplication, +r.rolconnlimit, +CASE WHEN curr_user.rolsuper OR + (curr_user.rolcreaterole AND m.admin_option) + THEN + CASE WHEN r.rolpassword IS NULL + THEN NULL::pg_catalog.text + ELSE ''::pg_catalog.text + END + ELSE ''::pg_catalog.text +END rolpassword, +r.rolvaliduntil, +r.rolbypassrls, +s.setconfig AS rolconfig, +r.oid +FROM pg_catalog.pg_authid r +JOIN pg_catalog.pg_authid curr_user +ON (curr_user.rolname = CURRENT_USER) +LEFT JOIN pg_catalog.pg_auth_members m +ON (curr_user.oid = m.member AND r.oid = m.roleid AND m.admin_option) +LEFT JOIN
Re: Things I don't like about \du's "Attributes" column
"David G. Johnston" 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
Re: Things I don't like about \du's "Attributes" column
On Mon, Feb 12, 2024 at 2:29 PM Pavel Luzanov 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. It is ignored to zero as opposed to unlimited for the Superuser so maybe a different word (not allowed)? David J.
Re: Things I don't like about \du's "Attributes" column
On 13.02.2024 00:29, Pavel Luzanov wrote: 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
Re: Things I don't like about \du's "Attributes" column
On 28.01.2024 22:51, Pavel Luzanov wrote: 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 |
Re: Things I don't like about \du's "Attributes" column
On 28.01.2024 22:51, Pavel Luzanov wrote: 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 > (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
Re: Things I don't like about \du's "Attributes" column
On Sun, Jan 28, 2024 at 1:29 PM Pavel Luzanov wrote: > I'd suggest pulling out this system view change into its own patch. > > > But within this thread or new one? > > > Thread. The subject line needs to make clear we are proposing changing a system view. The connection limit can be set to 0. What should be displayed in this case, blank or 0? > > 0 or even "not allowed" to make it clear > The connection limit can be set for superusers. What should be displayed in > this case, > blank or actual non-effective value? > > print "# (ignored)" ? 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? > > That is unfortunate...but they can always go look at the actual system view. Or do what i showed above and add (invalid) after the real value. Note I'm only really talking about -1 here being the value that is simply hidden from display since it means unlimited and not actually -1 I'd be more inclined to print "forever" for valid until since the existing presentation of a timestamp is already multiple characters. Using a word for a column that is typically a number is less appealing. David J.
Re: Things I don't like about \du's "Attributes" column
On 23.01.2024 01:59, David G. Johnston wrote: 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 23.01.2024 05:30, David G. Johnston wrote: On Sun, Jan 21, 2024 at 2:35 PM Pavel Luzanov 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
Re: Things I don't like about \du's "Attributes" column
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. 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 (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
Re: Things I don't like about \du's "Attributes" column
On Sun, Jan 21, 2024 at 2:35 PM Pavel Luzanov 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. 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. David J.
Re: Things I don't like about \du's "Attributes" column
On Mon, Jan 22, 2024 at 6:26 PM Tom Lane wrote: > 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. > > Makes sense, more reason to put it within its own patch. 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 (alt. N/A). David J.
Re: Things I don't like about \du's "Attributes" column
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
Re: Things I don't like about \du's "Attributes" column
Pavel Luzanov 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
Re: Things I don't like about \du's "Attributes" column
On Sun, Jan 21, 2024 at 2:35 PM Pavel Luzanov wrote: > 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. > > > 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 > 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) > > > Small modification with newline separator for Attributes column: > > Patch v3 with newlines: > =# \du > List of roles > Role name | Attributes | Password? | Valid until | Connection > limit > ---+-+---++-- > postgres | SUPERUSER +| no|| > -1 >| CREATEDB +| || >| CREATEROLE +| || >| INHERIT+| || >| LOGIN +| || >| REPLICATION+| || >| BYPASSRLS | || > (5 rows) > > 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) 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. I'd suggest pulling out this system view change into its own patch. I will take another pass later when I get some more time. I want to re-review some of the older messages. But the tweaks I show and breaking out the view changes in to a separate patch both appeal to me right now. David J.
Re: Things I don't like about \du's "Attributes" column
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.
Re: Things I don't like about \du's "Attributes" column
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 From 454ff68b64ca2ebcc2ba2e8d176f55ab018606cd Mon Sep 17 00:00:00 2001 From: Pavel Luzanov Date: Sun, 21 Jan 2024 23:44:33 +0300 Subject: [PATCH v3] psql: Rethinking of \du command The Attributes column includes only the enabled logical attributes. The attribute names correspond to the keywords of the CREATE ROLE command.
Re: Things I don't like about \du's "Attributes" column
On 03.01.2024 02:37, Jim Nasby wrote: 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 From 2ac990e66ce91efa319fce970ea37e98c6e89fa2 Mon Sep 17 00:00:00 2001 From: Pavel Luzanov Date: Tue, 9 Jan 2024 23:46:31 +0300 Subject: [PATCH v2] psql: Rethinking of \du command In this version, all attributes have been moved to separate columns --- src/backend/catalog/system_views.sql | 6 +- src/bin/psql/describe.c | 144 +++ 2 files changed, 41 insertions(+), 109 deletions(-) diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql index 058fc47c91..fed221f787 100644 --- a/src/backend/catalog/system_views.sql +++ b/src/backend/catalog/system_views.sql @@ -24,10 +24,10 @@ CREATE VIEW pg_roles AS rolcanlogin, rolreplication, rolconnlimit, -''::text as rolpassword, +CASE WHEN rolpassword IS NOT NULL THEN ''::text END AS rolpassword, rolvaliduntil, rolbypassrls, -setconfig as rolconfig, +setconfig AS rolconfig, pg_authid.oid FROM pg_authid LEFT JOIN pg_db_role_setting s ON (pg_authid.oid = setrole AND setdatabase = 0); @@ -65,7 +65,7 @@ CREATE VIEW pg_user AS usesuper, userepl, usebypassrls, -''::text as passwd, +CASE WHEN passwd IS NOT NULL THEN ''::text END AS passwd, valuntil, useconfig FROM pg_shadow; diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c index 5077e7b358..c79cfd0b97 100644 --- a/src/bin/psql/describe.c +++ b/src/bin/psql/describe.c @@ -36,7 +36,6 @@ static bool describeOneTableDetails(const char *schemaname, bool verbose); static void add_tablespace_footer(printTableContent *const cont, char relkind, Oid tablespace, const bool newline); -static void add_role_attribute(PQExpBuffer buf, const char *const str); static bool
Re: Things I don't like about \du's "Attributes" column
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
Re: Things I don't like about \du's "Attributes" column
On 1/2/24 1:38 PM, Robert Haas wrote: 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. In my consulting experience, it's extremely rare for users to do anything remotely sophisticated with roles (I was always happy just to see apps weren't connecting as a superuser...). Like you, I view \du and friends as more of a "helping hand" to seeing the state of things, without the expectation that every tiny nuance will always be visible, because I don't think it's practical to do that in psql. While that behavior might surprise some users, the good news is once they start exploring non-default options the behavior becomes self-evident. 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'm on the fence when it comes to SQL syntax vs what we have now. What we currenly have is more readable, but off-hand I think the other places we list attributes we do it in SQL syntax. It might be worth changing just for consistency sake. -- Jim Nasby, Data Architect, Austin TX
Re: Things I don't like about \du's "Attributes" column
On Tue, Jan 2, 2024 at 1:46 PM Tom Lane 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
Re: Things I don't like about \du's "Attributes" column
Robert Haas 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
Re: Things I don't like about \du's "Attributes" column
On Tue, Jan 2, 2024 at 1:17 PM Tom Lane 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
Re: Things I don't like about \du's "Attributes" column
Robert Haas 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
Re: Things I don't like about \du's "Attributes" column
On Thu, Jun 22, 2023 at 8:50 PM Tom Lane 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
Re: Things I don't like about \du's "Attributes" column
On 30.12.2023 17:33, Isaac Morland wrote: Would it make sense to make the column non-nullable and always set it to infinity when there is no expiry? A password is not required for roles. In many cases, external authentication is used in ph_hba.conf. I think it would be strange to have 'infinity' for roles without a password. Tom suggested to have 'infinity' in the \du output for roles with a password. My doubt is that this will hide the real values (absence of values). So I suggested a separate column 'Has password?' to show roles with password and unmodified column 'Password expire time'. Yes, it's easy to replace NULL with "infinity" for roles with a password, but why? What is the reason for this action? Absence of value for 'expire time' clear indicates that there is no time limit. Also I don't see a practical reasons to execute next command, since it do nothing: ALTER ROLE .. PASSWORD 'infinity'; So I think that in most cases there is no "infinity" in the rolvaliduntil column. But of course, I can be wrong. Thank you for giving your opinion. -- Pavel Luzanov Postgres Professional:https://postgrespro.com
Re: Things I don't like about \du's "Attributes" column
On Sat, 30 Dec 2023 at 09:23, Pavel Luzanov wrote: > 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. > Would it make sense to make the column non-nullable and always set it to infinity when there is no expiry? In this case, I think NULL simply means infinity, so why not write that? If the timestamp type didn't have infinity, then NULL would be a natural way of saying that there is no expiry, but with infinity as a possible value, I don't see any reason to think of no expiry as being the absence of an expiry time rather than an infinite expiry time.
Re: Things I don't like about \du's "Attributes" column
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
Re: Things I don't like about \du's "Attributes" column
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
Re: Things I don't like about \du's "Attributes" column
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
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
