Re: Things I don't like about \du's "Attributes" column
On Mon, Jul 22, 2024 at 5:19 PM Pavel Luzanov wrote: > Visible but inaccessible objects in system catalogs increase the volume > of command output unnecessarily. Why do I need to know the list of all > schemas in the database if I only have access to the public schema? > The same applies to inaccessible tables, views, functions, etc. > > Not for safety, but for convenience, it might be worth having a set of views > that show only those rows of the system catalog (with *acl column) that > the user has access to. Either as the object owner, or through the privileges. > Directly or indirectly through role membership. So, I wasn't actually aware that anyone had a big problem in this area. I thought that most of the junk you might see in \d output would be hidden either because the objects you don't care about are not in your search_path or because they are system objects. I agree that doesn't help with schemas, but most people don't have a huge number of schemas, and even if you do, you don't necessarily need to look at the list all that frequently. -- Robert Haas EDB: http://www.enterprisedb.com
Re: Things I don't like about \du's "Attributes" column
Robert, Iam pleasedthatyouare payingso muchattentionto thispatch. On 19.07.2024 16:26, Robert Haas wrote: Second, I think that the threshold question for this patch is: will users, on average, be happier if this patch gets committed? If the answer is yes, then the patch should be committed, and if the answer is no, the patch should not be committed. But I actually don't really have any clear idea of what users in general are likely to think. My own reaction is essentially ... meh. I do not think that the proposed new output is massively worse than what we have now, but I also don't think it's a whole lot better. Now, if a bunch of other people show up and vote, well then we'll have a much better view of what the typical user is likely to think. Ishareyouropinionthatthe needfor a patchshouldbe decidedby the votes(orlackof votes)of practicingexperts. Iam mainly involvedineducationalprojects,soinmostcasesI workwithdemosystems.Therefore, I'm notsurethatthe patch I'm offeringwill makeusershappy. Perhaps it should be withdrawn. Third, if I can back away from this particular patch for a moment, I feel like roles and permissions are one of the weaker areas in psql. So, personally, if I were going to work on a redesign in this area, I would look into making \du work like \d . That is, it would tell you every single thing there is to know about a user. Role attributes. Roles in which this role has membership. Roles that are a member of this row. Objects of all sorts this object owns. Permissions this role has on objects of all sorts. Role settings. All of it in SQL-ish format like we do with the footer when you run \d. Oh, that's very interesting. I will think about this approach, but I do not know when and what result can be obtained... But let me share my thoughts on roles, privileges and system catalogs from a different angle. This has nothing to do with the current patch, I just want to share my thoughts. I came to PostgreSQL from Oracle and it was unexpected for me that users had almost complete access to the contents of the system сatalogs. With rare exceptions (pg_authid, pg_statistic), any unprivileged user sees the full contents of any system сatalog. (I'm not saying that access to system catalogs needs to be reworked, it's probably impossible or very difficult.) Visible but inaccessible objects in system catalogs increase the volume of command output unnecessarily. Why do I need to know the list of all schemas in the database if I only have access to the public schema? The same applies to inaccessible tables, views, functions, etc. Not for safety, but for convenience, it might be worth having a set of views that show only those rows of the system catalog (with *acl column) that the user has access to. Either as the object owner, or through the privileges. Directly or indirectly through role membership. By the way, this is exactly the approach implemented for the information schema. Here is a code fragment of the information_schema.schemata view: SELECT ... FROM pg_namespace n, pg_authid u WHERE n.nspowner = u.oid AND (pg_has_role(n.nspowner, 'USAGE'::text) OR has_schema_privilege(n.oid, 'CREATE, USAGE'::text)) Then the commands like \dt, \df, \dn, \l, etc might use these views and show only the objects accessible to the user. To do this, a new modifier to the commands can be implemented, similar to the S modifier for system objects. For example: \dn - list of all schemas \dnA - list of accessible schemas In some way this approach can resolve your issue about roles and privileges. Familiar psql commands will be able to display only the objects accessible for current role,withoutpushingthe whole output into \du. Such a set of views can be useful not only in psql, but also for third-party applications. I think I'm not the first one trying to bikeshedding in this area. It's probably been discussed many times whythisshouldnotbe done. But such thoughts do come, and I don't know the answer yet. -- Pavel Luzanov Postgres Professional:https://postgrespro.com
Re: Things I don't like about \du's "Attributes" column
Hi Pavel, First, thanks for your dedication to this effort. I always find it hard to make time for things like psql backslash command improvements, but I'm glad that we have people in our community who work on such things. Second, I think that the threshold question for this patch is: will users, on average, be happier if this patch gets committed? If the answer is yes, then the patch should be committed, and if the answer is no, the patch should not be committed. But I actually don't really have any clear idea of what users in general are likely to think. My own reaction is essentially ... meh. I do not think that the proposed new output is massively worse than what we have now, but I also don't think it's a whole lot better. Now, if a bunch of other people show up and vote, well then we'll have a much better view of what the typical user is likely to think. But right now, I can't hazard a guess as to what the general opinion will be, and therefore I'm unprepared to commit anything. Because, and I can't say this often enough, it's not all about me. Even if I thought that this patch was way better than what we have now, I still wouldn't commit it unless I was relatively confident that other people would agree. Third, if I can back away from this particular patch for a moment, I feel like roles and permissions are one of the weaker areas in psql. I feel, and I suspect most users agree, that the output of "\d my_table" is top notch. It tells you everything that you need to know about a particular table -- all the columns, column types, default values, triggers, indexes, and whatever else there is. If someone adds a new object that attaches to a table, they're going to understand that they need to add a listing for that object to the \d output, and users are going to understand that they should look for it there. That unstated contract between developers and users is a thing of beauty: without any words being said, there is a meeting of the minds. But I don't think the same thing can be said around roles. For a long time, one of my big gripes in this area has been \z. It displays information about the permissions of table-like objects. But I don't really imagine that being a thing that a user is actually going to want to see. I feel like there are two typical use cases here. One is you want to see all the privileges associated with a table. In that case, you'd like the information to show up in the output of \d or \d+. The other is that you want to see the privileges associated with a particular user -- and in that case you want to see not just the table privileges but the privileges on every other kind of database object, too. I'm not saying there's a single PostgreSQL user anywhere who has wanted to list information about tables with names matching a certain wildcard and see just the privilege information for each one, but I bet it's rare. I also suspect that only truly hard-core PostgreSQL fans want to see incantations like "robert.haas"=arwdDxtm/"robert.haas" in their output. So, personally, if I were going to work on a redesign in this area, I would look into making \du work like \d . That is, it would tell you every single thing there is to know about a user. Role attributes. Roles in which this role has membership. Roles that are a member of this row. Objects of all sorts this object owns. Permissions this role has on objects of all sorts. Role settings. All of it in SQL-ish format like we do with the footer when you run \d. Then I would make \du work like \d: a minimal amount of basic information about every role in the list, like whether it's a superuser and whether they can log in. Then, I would consider removing \dp \drds \drg and \z entirely. I don't know if a plan like that would actually work out well. A particular problem is that if a user owns 10k objects, we don't want to just list them all one per line. Maybe when the number of objects owned is large, we could just give a count per object type unless + is used, or something like that. But even if all of this could be sorted out, would users in general like it better, or would it just make Robert happy? Would even Robert end up liking the result? I don't really know, but I think that my general discontent with \du \dg \dp \drds \drg \z is part of why I find it hard to evaluate a patch like this. I look forward to seeing the output of "\d " on whatever table is causing some customer a problem today. I never look forward to seeing the output of \du. Is that just me? ...Robert
Re: Things I don't like about \du's "Attributes" column
On 16.07.2024 18:00, Robert Haas wrote:
On the question of display width, my personal opinion is that the
current patch is worse than what we have now.
Robert, David, thanks for the detailed explanation.
I tried to remember all the thoughts that led to this version of the patch.
So the main issue that Robert points out is that the output of the command
takes up more space compared to the current version.
(But I'm ready to debate that too :-), see below.)
In the proposed version, columns for rolconnlimit and rolvaliduntil occupy
a significant place. It really is. We can hide them in extended mode, but
they still take up a lot of space. In the current command, these attributes
are very compactly arranged in the "Attributes" column on separate lines.
However, the current placement of rolconnlimit and rolvaliduntil on separate
lines is very bad, which Tom noted in the first letter and I completely
agree with this. Also, I don't like that the values appear only if they
differ from the default values. It's more compact, but less intuitive.
It seems to me that this approach is not used anywhere else in other
\d* commands (but I may be wrong, I did not check).
Let me explain why I think rolconnlimit and rolvaliduntil are worthy
of being placed as separate columns.
1. Logical attributes (rolsuper, rolinherit, rolcreaterole, rolcreatedb,
rolcanlogin, rolreplication, rolbypassrls) are uniform in nature and
presenting them as a list in one column looks logical.
But rolconnlimit and rolvaliduntil do not fit into this company in any way.
They are strangers here in terms of data type and meaning.
2. Logical attributes give the role additional capabilities,
while rolconnlimit and rolvaliduntil rather limit the use of the role.
3. After switching to a role with the SET ROLE command, you can use
the capabilities of logical attributes, but the restrictions of rolconnlimit
and rolvaliduntil do not apply to SET ROLE:
postgres@demo(17.0)=# grant bob to alice;
grant bob to alice;
GRANT ROLE
postgres@demo(17.0)=# alter role bob connection limit 0;
alter role bob connection limit 0;
ALTER ROLE
postgres@demo(17.0)=# \c - bob
connection to server on socket "/tmp/.s.PGSQL.5401" failed: FATAL: too many connections
for role "bob"
Previous connection kept
postgres@demo(17.0)=# \c - alice
You are now connected to database "demo" as user "alice".
alice@demo(17.0)=> set role bob;
set role bob;
SET
This makes it reasonable to consider rolconnlimit and rolvaliduntil
as separate properties of a role, rather than together with logical
attributes.
Now the hard part. What to do with the width of the command output?
I also think that it is desirable to fit the output of any command in 80
characters. And I was calm when I saw the 78-character output in my test
system:
postgres@demo(17.0)=# \du
List of roles
Role name | Login | Attributes | Valid until | Connection limit
---+---+-++--
alice | yes | Inherit | 2024-06-30 00:00:00+03 |
bob | yes | Inherit | infinity |
charlie | yes | Inherit ||1
postgres | yes | Superuser +||
| | Create DB +||
| | Create role+||
| | Inherit+||
| | Replication+||
| | Bypass RLS ||
(4 rows)
But, really, the width can exceed 80 with longer role names, as well as with
a wider default date output. Compare with the date output in the patch
regression
tests:
2024-06-30 00:00:00+03
Tue Jun 04 00:00:00 2024 PDT
To be fair, I must say that among the \d* commands there are many commands
whose output width exceeds 80 characters.
Namely: \da, \dAc, \dAf, \dAo, \dAp, \dC, \df, \di, \do, \dO, \dRp, \dT, \l
But let's go back to the current version. I consider this patch as
a continuation of the work on the \drg command that appeared in version 16.
As part of that work, we removed the "Member of" column from the \du command
and introduced a new \drg command to show membership in roles.
From my point of view, the \du command is currently in an intermediate and
unfinished state. Therefore, it is more correct to compare the proposed patch
with psql 15, rather than 16.
(I know there is nothing more permanent than a temporary solution,
but give me hope :-).)
In version 15, the output of the \du command is wider than the proposed version!
15=# \du
List of roles
Role name | Attributes |
Member of
---++---
postgres | Superuser, Create role, Create DB, Replication, Bypass RLS | {}
And this is with only one role. I can assume
Re: Things I don't like about \du's "Attributes" column
On Tue, Jul 16, 2024 at 8:00 AM Robert Haas wrote: > I'm starting to have some doubts about whether this effort is really > worthwhile. It seems like what we have right now is a patch which uses > both more horizontal space and more vertical space than the current > implementation, without (IMHO) really offering any clear advantage. It's simple enough to only add the mandatory vertical space here (and definitely reduce the width) by leaving the current presentation of "N connections" and "password valid until" unchanged but give them their own line in Attributes like all of the rest. The choice to move them to their own columns seems like where the contention lies. e.g., regress_du_role0 | yes | Inherit | Tue Jun 04 00:00:00 2024 PDT |0 | becomes No connections allowed +| Inherit +| Password valid until Tue Jun 04 ... | (pending confirmation of where the new inherit label fits slot-wise) versus: regress_du_role0 | No connections +| | Password valid until 2024-06-04 00:00:00+03 | (Which actually look the same because of the automatic wrapping on the current version versus explicit one-per-line in the proposed.) In short, fix the complaint about comma-separated attributes and leave the rest alone as being accurate reflections of the catalog. In short, from the original message, do "a" without "b". Tom suggested "either" as well and I agree with Robert that having done both we've made it both wider and taller which ends up not being a great outcome. Also, address the N connections [allowed] to make it clear this is a static configuration and not some derivation of current state. Can also deal with Password valid until infinity if a better phrasing can be thought up that doesn't require knowledge of whether a password has been set or not. David J.
Re: Things I don't like about \du's "Attributes" column
On Tue, Jul 16, 2024 at 9:48 AM Pavel Luzanov wrote: > Which version of the patch is currently under discussion? > > I believe we are talking about the latest v8 patch version. [1] > > 1. > https://www.postgresql.org/message-id/5341835b-e7be-44dc-b6e5-400e9e3f3c64%40postgrespro.ru Thanks. For some reason (likely me being dumb) I was having a hard time finding that in the thread. On the question of display width, my personal opinion is that the current patch is worse than what we have now. Right now, if I type \du, the output fits in an 80-column terminal unless the role names are quite long, and \du+ fits except for the Description field, which wraps. With the patch, even \du wraps in an 80-column terminal. "Role name", "Login," "Attributes," and "Valid until" fit, but "Connection limit" doesn't. I know some people think optimizing for 80-column terminals is obsolete in 2024, and I often use a wider window myself, but I do still appreciate it when I don't have to make the window wider to read stuff. Especially, if I didn't use + when running a psql command, I'd prefer for it to fit. One solution could be to move "Valid until" or "Connection limit" or both to verbose mode, as proposed by Rafia. But that's also a regression over what we have now, where the output fits in 80 columns and includes that information. I'm starting to have some doubts about whether this effort is really worthwhile. It seems like what we have right now is a patch which uses both more horizontal space and more vertical space than the current implementation, without (IMHO) really offering any clear advantage. I know this started out as an effort to address Tom's complaints in the original post, but it feels like we're losing as much as we're gaining, and Tom seems to have lost interest in the thread, too. -- Robert Haas EDB: http://www.enterprisedb.com
Re: Things I don't like about \du's "Attributes" column
On 16.07.2024 16:24, Robert Haas wrote: Which version of the patch is currently under discussion? I believe we are talking about the latest v8 patch version. [1] 1.https://www.postgresql.org/message-id/5341835b-e7be-44dc-b6e5-400e9e3f3c64%40postgrespro.ru -- Pavel Luzanov Postgres Professional:https://postgrespro.com
Re: Things I don't like about \du's "Attributes" column
On Tue, Jul 16, 2024 at 4:53 AM Pavel Luzanov wrote: > On 15.07.2024 12:50, Rafia Sabih wrote: > Well, it was just my opinion of how I would have liked it better, but > of course you may decide against it, there is no strong feeling around > it. > And if you are on the fence with the opinion of having them in normal > or extended mode, then maybe we can ask more people to chip in. > > I am not against moving "Valid until" and "Connection limit" to extended mode. > It just seemed to me that without these two columns, the output of the command > is too short, so there is no reason in hiding them. > > But you are right, we need more opinions. Which version of the patch is currently under discussion? -- Robert Haas EDB: http://www.enterprisedb.com
Re: Things I don't like about \du's "Attributes" column
On 15.07.2024 12:50, Rafia Sabih wrote: Well, it was just my opinion of how I would have liked it better, but of course you may decide against it, there is no strong feeling around it. And if you are on the fence with the opinion of having them in normal or extended mode, then maybe we can ask more people to chip in. I am not against moving "Valid until" and "Connection limit" to extended mode. It just seemed to me that without these two columns, the output of the command is too short, so there is no reason in hiding them. But you are right, we need more opinions. -- Pavel Luzanov Postgres Professional:https://postgrespro.com
Re: Things I don't like about \du's "Attributes" column
On Sat, 13 Jul 2024 at 14:21, Pavel Luzanov wrote:
>
> On 12.07.2024 12:22, Rafia Sabih wrote:
>
> Other thoughts came to my mind, should we have this column in \du+
> instead, maybe connection limit too.
> I know in the current version we have all this in \du itself, but then
> all clubbed in one column. But now since
> our table has got wider, it might be aesthetic to have it in the
> extended version. Also, their usage wise might not
> be the first thing to be looked at for a user/role.
>
> In the first version of the patch, "Valid until" (named "Password expire
> time")
> and "Connection limit" ("Max connections") columns were in extended mode. [1]
>
> Later we decided to place each attribute in the "Attributes" column on a
> separate
> line. This allowed us to significantly reduce the overall width of the output.
> So, I decided to move "Valid until" and "Connection limit" from extended mode
> to normal mode.
>
> Just compare output from patched \du and popular \list command:
>
> postgres@demo(17.0)=# \du
> List of roles
> Role name | Login | Attributes | Valid until | Connection limit
> ---+---+-++--
> alice | yes | Inherit | 2024-06-30 00:00:00+03 |
> bob | yes | Inherit | infinity |
> charlie | yes | Inherit || 11
> postgres | yes | Superuser +||
>| | Create DB +||
>| | Create role+||
>| | Inherit+||
>| | Replication+||
>| | Bypass RLS ||
> (4 rows)
>
> postgres@demo(17.0)=# \list
> List of databases
>Name| Owner | Encoding | Locale Provider | Collate |Ctype
> | Locale | ICU Rules | Access privileges
> ---+--+--+-+-+-++---+---
> demo | postgres | UTF8 | libc| en_US.UTF-8 |
> en_US.UTF-8 || |
> postgres | postgres | UTF8 | libc| en_US.UTF-8 |
> en_US.UTF-8 || |
> template0 | postgres | UTF8 | libc| en_US.UTF-8 |
> en_US.UTF-8 || | =c/postgres +
>| | | | |
> || | postgres=CTc/postgres
> template1 | postgres | UTF8 | libc| en_US.UTF-8 |
> en_US.UTF-8 || | =c/postgres +
>| | | | |
> || | postgres=CTc/postgres
> (4 rows)
>
> If we decide to move "Valid until" and "Connection limit" to extended mode,
> then the role attributes should be returned to their previous form by placing
> them on one line separated by commas.
>
> 1.
> https://www.postgresql.org/message-id/27f87cb9-229b-478b-81b2-157f94239d55%40postgrespro.ru
>
> --
> Pavel Luzanov
> Postgres Professional: https://postgrespro.com
Well, it was just my opinion of how I would have liked it better, but
of course you may decide against it, there is no strong feeling around
it.
And if you are on the fence with the opinion of having them in normal
or extended mode, then maybe we can ask more people to chip in.
I certainly would not want you to update the patch back and forth for
almost no reason.
--
Regards,
Rafia Sabih
Re: Things I don't like about \du's "Attributes" column
On 12.07.2024 12:22, Rafia Sabih wrote:
Other thoughts came to my mind, should we have this column in \du+
instead, maybe connection limit too.
I know in the current version we have all this in \du itself, but then
all clubbed in one column. But now since
our table has got wider, it might be aesthetic to have it in the
extended version. Also, their usage wise might not
be the first thing to be looked at for a user/role.
In the first version of the patch, "Valid until" (named "Password expire time")
and "Connection limit" ("Max connections") columns were in extended mode. [1]
Later we decided to place each attribute in the "Attributes" column on a
separate
line. This allowed us to significantly reduce the overall width of the output.
So, I decided to move "Valid until" and "Connection limit" from extended mode
to normal mode.
Just compare output from patched \du and popular \list command:
postgres@demo(17.0)=# \du
List of roles
Role name | Login | Attributes | Valid until | Connection limit
---+---+-++--
alice | yes | Inherit | 2024-06-30 00:00:00+03 |
bob | yes | Inherit | infinity |
charlie | yes | Inherit || 11
postgres | yes | Superuser +||
| | Create DB +||
| | Create role+||
| | Inherit+||
| | Replication+||
| | Bypass RLS ||
(4 rows)
postgres@demo(17.0)=# \list
List of databases
Name| Owner | Encoding | Locale Provider | Collate |Ctype
| Locale | ICU Rules | Access privileges
---+--+--+-+-+-++---+---
demo | postgres | UTF8 | libc| en_US.UTF-8 | en_US.UTF-8
|| |
postgres | postgres | UTF8 | libc| en_US.UTF-8 | en_US.UTF-8
|| |
template0 | postgres | UTF8 | libc| en_US.UTF-8 | en_US.UTF-8
|| | =c/postgres +
| | | | |
|| | postgres=CTc/postgres
template1 | postgres | UTF8 | libc| en_US.UTF-8 | en_US.UTF-8
|| | =c/postgres +
| | | | |
|| | postgres=CTc/postgres
(4 rows)
If we decide to move "Valid until" and "Connection limit" to extended mode,
then the role attributes should be returned to their previous form by placing
them on one line separated by commas.
1.https://www.postgresql.org/message-id/27f87cb9-229b-478b-81b2-157f94239d55%40postgrespro.ru
--
Pavel Luzanov
Postgres Professional:https://postgrespro.com
Re: Things I don't like about \du's "Attributes" column
On Fri, 12 Jul 2024 at 09:06, Pavel Luzanov wrote: > > On 11.07.2024 15:07, Rafia Sabih wrote: > > This looks much better than the current version. > > Thank you, for looking into this. > > Only thing is, I find > the column name Valid until confusing. With that name I am in danger > of taking it as the role's validity and not the passwords'. > How about naming it to something like Password validity...? > > Yes, my first attempt was to name this column "Password expiration date" > for the same reason. > > But then I decided that the column name should match the attribute name. > Otherwise, you need to make some effort to understand which columns > of the table correspond to which attributes of the roles. > > It is also worth considering translation into other languages. > If the role attribute and the column have the same name, then they will > probably be translated the same way. But the translation may be different > for different terms, which will further confuse the situation. > > We can probably change the column name, but still the root of the confusion > is caused by the attribute name, not the column name. > > What do you think? Yes you are right in this. I too carry the opinion that column names should be the same as attribute names as much as possible. So, then it is good that way. Other thoughts came to my mind, should we have this column in \du+ instead, maybe connection limit too. I know in the current version we have all this in \du itself, but then all clubbed in one column. But now since our table has got wider, it might be aesthetic to have it in the extended version. Also, their usage wise might not be the first thing to be looked at for a user/role. What are your thoughts on that? > > -- > Pavel Luzanov > Postgres Professional: https://postgrespro.com -- Regards, Rafia Sabih
Re: Things I don't like about \du's "Attributes" column
On 11.07.2024 15:07, Rafia Sabih wrote: This looks much better than the current version. Thank you, for looking into this. Only thing is, I find the column name Valid until confusing. With that name I am in danger of taking it as the role's validity and not the passwords'. How about naming it to something like Password validity...? Yes, my first attempt was to name this column "Passwordexpirationdate" for the same reason. But then I decided that the column name should match the attribute name. Otherwise, you need to make some effort to understand which columns of the table correspond to which attributes of the roles. It is also worth considering translation into other languages. If the role attribute and the column have the same name, then they will probably be translated the same way. But the translation may be different for different terms, which will further confuse the situation. We can probably change the column name, but still the root of the confusion is caused by the attribute name, not the column name. What do you think? -- Pavel Luzanov Postgres Professional:https://postgrespro.com
Re: Things I don't like about \du's "Attributes" column
On Thu, 6 Jun 2024 at 23:10, Pavel Luzanov wrote: > > On 06.06.2024 17:29, Robert Haas wrote: > > I think the first of these special interpretations is unnecessary and > should be removed. It seems pretty clear what 0 means. > > Agree. > There is an additional technical argument for removing this replacement. > I don't like explicit cast to text of the "Connection limit" column. > Without 'Not allowed' it is no longer required. > Value -1 can be replaced by NULL with an implicit cast to integer. > > Next version with this change attached. > > Example output: > > \du+ regress_du* > List of roles > Role name | Login | Attributes | Valid until | > Connection limit | Description > --+---+-+--+--+-- > regress_du_admin | yes | Superuser +| | > | some description > | | Create DB +| | > | > | | Create role+| | > | > | | Inherit+| | > | > | | Replication+| | > | > | | Bypass RLS | | > | > regress_du_role0 | yes | Inherit | Tue Jun 04 00:00:00 2024 PDT | > 0 | > regress_du_role1 | no| Create role+| infinity | > | > | | Inherit | | > | > regress_du_role2 | yes | Inherit+| | > 42 | > | | Replication+| | > | > | | Bypass RLS | | > | > (4 rows) > > Current version for comparison: This looks much better than the current version. Only thing is, I find the column name Valid until confusing. With that name I am in danger of taking it as the role's validity and not the passwords'. How about naming it to something like Password validity...? > > List of roles > Role name | Attributes > | Description > --++-- > regress_du_admin | Superuser, Create role, Create DB, Replication, Bypass > RLS | some description > regress_du_role0 | No connections > +| > | Password valid until 2024-06-04 00:00:00+03 > | > regress_du_role1 | Create role, Cannot login > +| > | Password valid until infinity > | > regress_du_role2 | Replication, Bypass RLS > +| > | 42 connections > | > > > Data: > CREATE ROLE regress_du_role0 LOGIN PASSWORD '123' VALID UNTIL '2024-06-04' > CONNECTION LIMIT 0; > CREATE ROLE regress_du_role1 CREATEROLE CONNECTION LIMIT -1 VALID UNTIL > 'infinity'; > CREATE ROLE regress_du_role2 LOGIN REPLICATION BYPASSRLS CONNECTION LIMIT 42; > CREATE ROLE regress_du_admin LOGIN SUPERUSER CREATEROLE CREATEDB BYPASSRLS > REPLICATION INHERIT; > COMMENT ON ROLE regress_du_admin IS 'some description'; > > -- > Pavel Luzanov > Postgres Professional: https://postgrespro.com -- Regards, Rafia Sabih
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
