Re: Things I don't like about \du's "Attributes" column
On 23.07.2024 15:53, Robert Haas wrote: 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. Maybe. But it would be better not to see unnecessary objects in the system catalogs. Especially for GUI tools. Back to the subject. So, personally, if I were going to work on a redesign in this area, I would look into making \du 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. Yes, I still like this idea. A little later I will try to make a patch in this direction. -- Pavel Luzanov Postgres Professional:https://postgrespro.com
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
, Bypass RLS | {}
And this is with only one role. I can assume that there are usually several
roles in systems and role membership is actively used to organize roles within
groups. Therefore, in real systems, the output of the \du command in version 15
is probably much wider. For example, output together with system objects:
15=# \duS
List of
roles
Role name | Attributes
| Member of
---++--
pg_execute_server_program | Cannot login
| {}
pg_monitor| Cannot login
| {pg_read_all_settings,pg_read_all_stats,pg_stat_scan_tables}
pg_read_all_settings | Cannot login
| {}
pg_read_all_stats | Cannot login
| {}
pg_read_server_files | Cannot login
| {}
pg_signal_backend | Cannot login
| {}
pg_stat_scan_tables | Cannot login
| {}
pg_write_server_files | Cannot login
| {}
postgres | Superuser, Create role, Create DB, Replication,
Bypass RLS | {}
All this allows me to believe that the proposed version has advantages over
the current version of the \du command:
- Solutions have been proposed for 3 of the 4 Tom's complaints.
- The new "Login" column separates users from group roles, which is very useful
(imho).
- Tabular output is convenient to view both in normal mode and in expanded mode
(\x).
The last line contains information about the number of roles.
- Refactoring: code has become much simpler and clearer.
But if we don't find a compromise and just leave it as it is, then that's fine.
So the time for change has not come yet. In any case, this discussion may be
useful
in the future.Butwhoknows,maybenowwecancometosomekindof agreement.
--
Pavel Luzanov
Postgres Professional:https://postgrespro.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 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 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 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 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
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 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)
{
PQEx
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 = f
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
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 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_op
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
Show password presence in pg_roles for authorized roles
Hello, Currently, a role with the createrole attribute can create roles, set and change their password, but can't see the password. Can't even see if the password is set or not. In this case, you can mistakenly set the Valid until attribute to roles without a password. And there is no way to detect such a situation. In the patch for changing the \du command, I want to give the opportunity to show incorrect values of the Valid until attribute. [1] I suggest changing the pg_roles view to allow a role with the createrole attribute to see information about the password of the roles that this role manages (has membership with admin option). There are several ways to implement it. 1. Change the values of the rolpassword column. Now it always shows ''. The values should depend on the role executing the query. If the query is executed by a superuser or a role with create role and admin membership, then show '' instead of password or NULL (no password). For other roles, show ''. This is implemented in the attached patch. 2. Change the values of the rolpassword column. If the query is executed by a superuser or a role with create role and admin membership, then show real password or NULL (no password). For other roles, show ''. 3. Leave the rolpassword column as it is for backward compatibility, but add a new logical rolhaspassword column. If the query is executed by a superuser or a role with create role and admin membership, then show true/false depending on the password existence. For other roles, show NULL. Although it is possible that for security reasons such changes should not be made. 1.https://www.postgresql.org/message-id/ef4d000f-6766-4ae1-9f69-0d0caa8130d6%40postgrespro.ru -- Pavel Luzanov Postgres Professional:https://postgrespro.com From 78a1b38386634becc6c82749c1e7e19c4f1cc94f Mon Sep 17 00:00:00 2001 From: Pavel Luzanov Date: Mon, 12 Feb 2024 23:46:15 +0300 Subject: [PATCH v4 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 | 45 ++-- src/test/regress/expected/rules.out | 37 ++- 2 files changed, 52 insertions(+), 30 deletions(-) diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql index 6791bff9dd..2de6802cf7 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 pg_catalog.pg_db_role_setting s +ON (r.oid = s.setrole AND s.setdatabase = 0); CREATE VIEW pg_shadow AS SELECT @@ -65,7 +78,7 @@ CREATE VIEW pg_user AS usesuper, userepl, usebypassrls, -''::text as passwd, +''::text AS passwd, valuntil, useconfig FROM pg_shadow; diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out index abc944e8b8..a704015de3 100644 --- a/src/test/regress/expected/rules.out +++ b/src/test/regress/expected/rules.out @@ -1477,21 +1477,30 @@ pg_replication_slots| SELECT l.slot_name, l.failover FROM (pg_get_replication_slots() l(slot_name, plugin, slot_type, datoid, temporary, active, active_pid, xmin, catalog_xmin, restart_lsn, confirmed_flush_lsn, wal_status, safe_wal_size
Re: Psql meta-command conninfo+
Hi, On 12.02.2024 17:16, Maiquel Grassi wrote: The "if (db == NULL)" has been removed, as well as the message inside it. To always maintain the same error messages, I changed the validation of "\conninfo" to match the validation used for "\conninfo+". Therefore, now "\conninfo" and "\conninfo+" return the same error when "pg_terminate_backend()" terminates this session through another session. This make sense to me. Thank you for this work. -- 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 +
Re: Add semi-join pushdown to postgres_fdw
Hi, Alexander! On 12.02.2024 05:27, Alexander Korotkov wrote: But the worst thing is that replacing AND with OR causes breaking session and server restart: I haven't managed to reproduce this yet. Could you give more details: machine, OS, compile options, backtrace? We already had off-list conversation with Alexander Pyhalov. Yesterday, after rebuilding the server, I can't reproduce the error. I have good reason to believe that the problem was on my side. On Friday, I tested another patch and built the server several times. Most likely, I just made a mistake during the server build. Sorry for the noise. -- Pavel Luzanov Postgres Professional:https://postgrespro.com
Re: Add semi-join pushdown to postgres_fdw
Hello, While playing with this feature I found the following. Two foreign tables: postgres@demo_postgres_fdw(17.0)=# \det aircrafts|seats List of foreign tables Schema | Table | Server +---+- public | aircrafts | demo_server public | seats | demo_server (2 rows) This query uses optimization: postgres@demo_postgres_fdw(17.0)=# EXPLAIN (costs off, verbose) SELECT * FROM aircrafts a WHERE a.aircraft_code = '320' AND EXISTS ( SELECT * FROM seats s WHERE s.aircraft_code = a.aircraft_code ); QUERY PLAN > --> Foreign Scan Output: a.aircraft_code, a.model, a.range Relations: (public.aircrafts a) SEMI JOIN (public.seats s) Remote SQL: SELECT r1.aircraft_code, r1.model, r1.range FROM bookings.aircrafts r1 WHERE ((r1.aircraft_code = '320')) AND EXISTS (SELECT NULL FROM bookings.seats r2 WHERE ((r2.aircraft_code => (4 rows) But optimization not used for NOT EXISTS: postgres@demo_postgres_fdw(17.0)=# EXPLAIN (costs off, verbose) SELECT * FROM aircrafts a WHERE a.aircraft_code = '320' AND NOT EXISTS ( SELECT * FROM seats s WHERE s.aircraft_code = a.aircraft_code ); QUERY PLAN Nested Loop Anti Join Output: a.aircraft_code, a.model, a.range -> Foreign Scan on public.aircrafts a Output: a.aircraft_code, a.model, a.range Remote SQL: SELECT aircraft_code, model, range FROM bookings.aircrafts WHERE ((aircraft_code = '320')) -> Materialize Output: s.aircraft_code -> Foreign Scan on public.seats s Output: s.aircraft_code Remote SQL: SELECT aircraft_code FROM bookings.seats WHERE ((aircraft_code = '320')) (10 rows) Also, optimization not used after deleting first condition (a.aircraft_code = '320'): postgres@demo_postgres_fdw(17.0)=# EXPLAIN (costs off, verbose) SELECT * FROM aircrafts a WHERE EXISTS ( SELECT * FROM seats s WHERE s.aircraft_code = a.aircraft_code ); QUERY PLAN Hash Join Output: a.aircraft_code, a.model, a.range Inner Unique: true Hash Cond: (a.aircraft_code = s.aircraft_code) -> Foreign Scan on public.aircrafts a Output: a.aircraft_code, a.model, a.range Remote SQL: SELECT aircraft_code, model, range FROM bookings.aircrafts -> Hash Output: s.aircraft_code -> HashAggregate Output: s.aircraft_code Group Key: s.aircraft_code -> Foreign Scan on public.seats s Output: s.aircraft_code Remote SQL: SELECT aircraft_code FROM bookings.seats (15 rows) But the worst thing is that replacing AND with OR causes breaking session and server restart: postgres@demo_postgres_fdw(17.0)=# EXPLAIN (costs off, verbose) SELECT * FROM aircrafts a WHERE a.aircraft_code = '320' OR EXISTS ( SELECT * FROM seats s WHERE s.aircraft_code = a.aircraft_code ); server closed the connection unexpectedly This probably means the server terminated abnormally before or while processing the request. The connection to the server was lost. Attempting reset: Failed. The connection to the server was lost. Attempting reset: Failed. -- Pavel Luzanov Postgres Professional:https://postgrespro.com
Re: Psql meta-command conninfo+
Hi, Maiquel!
The patch v10 build ends with a warning:
$ make -j --silent
describe.c:911:1: warning: no previous prototype for
‘listConnectionInformation’ [-Wmissing-prototypes]
911 | listConnectionInformation()
| ^
About terms.
postgres@postgres(17.0)=# \x \conninfo+
Expanded display is on.
Current Connection Information
-[ RECORD 1 ]--+-
Database | postgres
Authenticated User | postgres
System User|
Current User | postgres
Session User | postgres
*Session PID | 951112 *Server Version | 17devel
Server Address |
Server Port| 5401
Client Address |
Client Port|
Socket Directory | /tmp
Host |
It looks like "Session PID" is a new term for the server process identifier.
How about changing the name to "Backend PID" (from pg_backend_pid) or even PID
(from pg_stat_activity)?
On 08.02.2024 17:58, Maiquel Grassi wrote:
> 1. > + if (db == NULL) > + printf(_("You are currently not connected to
a database.\n")); > > This check is performed for \conninfo, but not
for \conninfo+.
1. The connection check for the case of \conninfo+ is handled by
"describe.c" itself since it deals with queries. I might be mistaken,
but I believe that by using "printQuery()" via "describe.c", this is
already ensured, and there is no need to evaluate the connection status.
I found that \conninfo and \conninfo+ act differently when the connection is
broken.
I used pg_terminate_backend function from another session to terminate an open
psql session.
After that, \conninfo does not see the connection break (surprisingly!), and
\conninfo+ returns an error:
postgres@postgres(17.0)=# \conninfo+
FATAL: terminating connection due to administrator command
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: Succeeded.
postgres@postgres(17.0)=# \conninfo
You are connected to database "postgres" as user "postgres" via socket in "/tmp" at port
"5401".
Another surprise is that this check:if (db == NULL) did not work in both cases.
--
Pavel Luzanov
Postgres Professional:https://postgrespro.com
Re: Psql meta-command conninfo+
Hi,
On 07.02.2024 23:13, Maiquel Grassi wrote:
Regarding the "system_user" function, as it is relatively new, I added
the necessary handling to avoid conflicts with versions lower than
version 16.
Yes, that's rights.
A couple of doubts about the implementation details.
But keep in mind that I'm not very good at programming in the C language.
I hope for the help of more experienced developers.
1.
+ if (db == NULL)
+ printf(_("You are currently not connected to a
database.\n"));
This check is performed for \conninfo, but not for \conninfo+.
2.
Some values (address, socket) are evaluated separately for \conninfo
(via C functions) and for \conninfo+ (via sql functions).
It may be worth evaluating them in one place. But I'm not sure about that.
The final version of the patch will require changes to the documentation and
tests.
--
Pavel Luzanov
Postgres Professional:https://postgrespro.com
Re: Psql meta-command conninfo+
Hi,Maiquel! On 07.02.2024 17:47, Maiquel Grassi wrote: "Also, it seems that the verbose parameter in the listConnectionInformation is unnecessary." Could you point out exactly the line or code snippet you are referring to? +bool +listConnectionInformation(const char *pattern,*bool verbose*) I mean that parameter verbose is not used in the function body and listConnectionInformation called only in verbose mode. -- Pavel Luzanov Postgres Professional:https://postgrespro.com
Re: Psql meta-command conninfo+
This is a good idea about extended connection info. On 07.02.2024 07:13, Maiquel Grassi wrote: SELECT ... current_user AS "User", This will be inconsistent with \conninfo. \conninfo returns authenticated user (PQuser), not the current_user. It might be worth showing current_user, session_user, and authenticated user, but I can't find the appropriate sql function for PQuser. What about to include system_user function? It shows useful authentication details. Also, it seems that the verbose parameter in the listConnectionInformation is unnecessary. -- 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: 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 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
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 keywo
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 listTSPars
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 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
ss RLS =# \du+ List of roles Role name | Attributes | Description ---++- admin | Cannot login | Group role without login alice | Superuser, No inheritance +| Superuser but with connection limit and with no inheritance | 5 connections +| | Password valid until infinity | bob | Create DB, Replication, Bypass RLS+| No password but with expire time | Password valid until 2022-01-01 00:00:00+03| charlie | Create role +| No connections allowed | No connections | postgres | Superuser, Create role, Create DB, Replication, Bypass RLS | Patched. =# \du List of roles Role name | Can login? | Attributes ---++ admin | no | alice | yes| Superuser, No inheritance bob | yes| Create DB, Replication, Bypass RLS charlie | yes| Create role postgres | yes| Superuser, Create role, Create DB, Replication, Bypass RLS (5 rows) =# \du+ List of roles Role name | Can login? | Attributes | Has password? | Password expire time | Max connections | Description ---+++---++-+- admin | no | | no|| -1 | Group role without login alice | yes| Superuser, No inheritance | yes | infinity | 5 | Superuser but with connection limit and with no inheritance bob | yes| Create DB, Replication, Bypass RLS | no| 2022-01-01 00:00:00+03 | -1 | No password but with expire time charlie | yes| Create role | yes || 0 | No connections allowed postgres | yes| Superuser, Create role, Create DB, Replication, Bypass RLS | yes || -1 | (5 rows) v1 of the patch attached. There are no tests and documentation yet. make check fall in 2 existing tests due changes in pg_roles and \du. Will be corrected. Any opinions? I plan to add a patch to the January commitfest. -- Pavel Luzanov Postgres Professional:https://postgrespro.com From 78caf093e68d6166477baf59934d53f50103836a Mon Sep 17 00:00:00 2001 From: Pavel Luzanov Date: Sat, 30 Dec 2023 15:48:18 +0300 Subject: [PATCH v1] psql: Rethinking of \du command Changes: * login attribute moved from "Attributes" column to separate column "Can login?" with yes/no values. * connlimit attribute moved from "Attributes" column to separate column "Max connections" in extended mode(+). * 'valid until' attribute moved from "Attributes" column to separate column "Password expire time" in extended mode. * Added new column "Has password?" in extended mode based on changed pg_roles.rolpassword. The rolpassword column of the pg_roles view modified so that it is easy to identify the presence of a password. The same changes made for pg_user.passwd for consistency. * describeRoles function 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. Per suggestions from Tom Lane. Author: Pavel Luzanov Discussion: https://www.postgresql.org/message-id/4133242.1687481416%40sss.pgh.pa.us --- src/backend/catalog/system_views.sql | 6 +- src/bin/psql/describe.c | 146 --- 2 files changed, 43 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,
Re: Trigger violates foreign key constraint
On 22.12.2023 14:39, Laurenz Albe wrote: Yes, that is better - shorter and avoids passive mode. Changed. Thanks. Also I don't really like "This is not considered a bug" part, since it looks like an excuse. In a way, it is an excuse, so why not be honest about it. I still think that the "this is not a bug" style is not suitable for documentation. But I checked the documentation and found 3-4 more such places. Therefore, it's ok from me, especially since it really looks like a bug. The example you provided in your other message (cascading triggers fail if the table ovner has revoked the required permissions from herself) is not really about breaking foreign keys. You hit a surprising error, but referential integrity will be maintained. Yes, referential integrity will be maintained. But perhaps it is worth adding a section to the documentation about system triggers that are used to implement foreign keys. Now it is not mentioned anywhere, but questions and problems arise from time to time. Such a section named "System triggers" may be added as a separate chapter to Part VII. Internals or as a subsection to Chapter 38.Triggers. I thought about this after your recent excellent article [1], which has an introduction to system triggers. This does not negate the need for the patch being discussed. Patch v3 is attached. For me, it is ready for committer. 1. https://www.cybertec-postgresql.com/en/broken-foreign-keys-postgresql/ -- Pavel Luzanov Postgres Professional:https://postgrespro.com
Re: Trigger violates foreign key constraint
One more not documented issue with system triggers. It might be worth considering together. CREATE ROLE app_owner; CREATE TABLE t ( id int PRIMARY KEY, parent_id int REFERENCES t(id) ); ALTER TABLE t OWNER TO app_owner; -- No actions by application owner REVOKE ALL ON t FROM app_owner; INSERT INTO t VALUES (1,NULL); DELETE FROM t; ERROR: permission denied for table t CONTEXT: SQL statement "SELECT 1 FROM ONLY "public"."t" x WHERE "id" OPERATOR(pg_catalog.=) $1 FOR KEY SHARE OF x" It is not at all obvious why the superuser cannot delete the row that he just added. The reason is that system triggers are executed with the rights of the table owner, not the current role. But I can't find a description of this behavior in the documentation. -- Pavel Luzanov Postgres Professional: https://postgrespro.com
Re: Trigger violates foreign key constraint
I fully support this addition to the documentation. The legal possibility of breaking data consistency must be documented at least. Please, consider small suggestion to replace last sentence. - This is not considered a bug, and it is the responsibility of the user to write triggers so that such problems are avoided. + It is the trigger programmer's responsibility to avoid such scenarios. To be consistent with the sentence about recursive trigger calls: [1] "It is the trigger programmer's responsibility to avoid infinite recursion in such scenarios." Also I don't really like "This is not considered a bug" part, since it looks like an excuse. 1. https://www.postgresql.org/docs/devel/trigger-definition.html -- Pavel Luzanov Postgres Professional: https://postgrespro.com
Re: WIP: new system catalog pg_wait_event
Please, consider this small fix for the query example in the docs[1]: SELECT a.pid, a.wait_event, w.description FROM pg_stat_activity a JOIN pg_wait_events w ON (a.wait_event_type = w.type AND a.wait_event = w.name) WHERE wait_event is NOT NULL and a.state = 'active'; I propose to add a table alias for the wait_event column in the WHERE clause for consistency. 1.https://www.postgresql.org/docs/devel/monitoring-stats.html#MONITORING-PG-STAT-ACTIVITY-VIEW -- Pavel Luzanov Postgres Professional:https://postgrespro.com From 64e9aa8ddde392b97bff66d53a5d09e0a820380c Mon Sep 17 00:00:00 2001 From: Pavel Luzanov Date: Mon, 23 Oct 2023 13:00:16 +0300 Subject: [PATCH] Fix pg_wait_events usage example This makes the use of column aliases consistent throughout the query. --- doc/src/sgml/monitoring.sgml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml index 1149093a8a..3f49ff79f3 100644 --- a/doc/src/sgml/monitoring.sgml +++ b/doc/src/sgml/monitoring.sgml @@ -1119,7 +1119,7 @@ SELECT a.pid, a.wait_event, w.description FROM pg_stat_activity a JOIN pg_wait_events w ON (a.wait_event_type = w.type AND a.wait_event = w.name) - WHERE wait_event is NOT NULL and a.state = 'active'; + WHERE a.wait_event is NOT NULL and a.state = 'active'; -[ RECORD 1 ]-- pid | 686674 wait_event | WALInitSync -- 2.34.1
Re: PG 16 draft release notes ready
On 22.08.2023 00:58, Bruce Momjian wrote: Attached is an applied patch that moves the inherit item into incompatibilities. clarifies it, and splits out the ADMIN syntax item. > The role's default inheritance behavior can be overridden with the new GRANT ... WITH INHERIT clause. The only question about "can be". Why not "will be"? The inheritance behavior will be changed anyway. Please let me know if I need any other changes. Thanks. * Allow psql's access privilege commands to show system objects (Nathan Bossart, Pavel Luzanov) The options are \dpS, \zS, and \drg. I think that this description correct only for the \dpS and \zS commands. (By the way, unfortunately after reverting MAINTAIN privilege this commands are not much useful in v16.) But the \drg command is a different thing. This is a full featured replacement for "Member of" column of the \du, \dg commands. It shows not only members, but granted options (admin, inherit, set) and grantor. This is important information for membership usage and administration. IMO, removing the "Member of" column from the \du & \dg commands also requires attention in release notes. So, I suggest new item in the psql section for \drg. Something like this: * Add psql \drg command to display role grants and remove the "Member of" column from \du & \dg altogether. -- Pavel Luzanov Postgres Professional: https://postgrespro.com
Re: PG 16 draft release notes ready
On 17.08.2023 05:36, Bruce Momjian wrote: On Wed, Aug 9, 2023 at 08:35:21PM -0400, Bruce Momjian wrote: On Sat, Aug 5, 2023 at 04:08:47PM -0700, Noah Misch wrote: Author: Robert Haas 2022-08-25 [e3ce2de09] Allow grant-level control of role inheritance behavior. --> Allow GRANT to control role inheritance behavior (Robert Haas) By default, role inheritance is controlled by the inheritance status of the member role. The new GRANT clauses WITH INHERIT and WITH ADMIN can now override this. Allow roles that create other roles to automatically inherit the new role's rights or SET ROLE to the new role (Robert Haas, Shi Yu) This is controlled by server variable createrole_self_grant. Similarly, v16 radically changes the CREATE ROLE ... WITH INHERIT clause. The clause used to "change the behavior of already-existing grants." Let's merge these two and move the combination to the incompatibilities section. I need help with this. I don't understand how they can be combined, and I don't understand the incompatibility text in commit e3ce2de09d: If a GRANT does not specify WITH INHERIT, the behavior based on whether the member role is marked INHERIT or NOINHERIT. This means that if all roles are marked INHERIT or NOINHERIT before any role grants are performed, the behavior is identical to what we had before; otherwise, it's different, because ALTER ROLE [NO]INHERIT now only changes the default behavior of future grants, and has no effect on existing ones. I am waiting for an answer to this question, or can I assume the release notes are acceptable? I can try to explain how I understand it myself. In v15 and early, inheritance of granted to role privileges depends on INHERIT attribute of a role: create user alice; grant pg_read_all_settings to alice; By default privileges inherited: \c - alice show data_directory; data_directory - /var/lib/postgresql/15/main (1 row) After disabling the INHERIT attribute, privileges are not inherited: \c - postgres alter role alice noinherit; \c - alice show data_directory; ERROR: must be superuser or have privileges of pg_read_all_settings to examine "data_directory" In v16 changing INHERIT attribute on alice role doesn't change inheritance behavior of already granted roles. If we repeat the example, Alice still inherits pg_read_all_settings privileges after disabling the INHERIT attribute for the role. Information for making decisions about role inheritance has been moved from the role attribute to GRANT role TO role [WITH INHERIT|NOINHERIT] command and can be viewed by the new \drg command: \drg List of role grants Role name | Member of | Options | Grantor ---+--+--+-- alice | pg_read_all_settings | INHERIT, SET | postgres (1 row) Changing the INHERIT attribute for a role now will affect (as the default value) only future GRANT commands without an INHERIT clause. -- Pavel Luzanov Postgres Professional: https://postgrespro.com
Re: PG 16 draft release notes ready
On 09.08.2023 21:06, Bruce Momjian wrote: On Sun, Jul 23, 2023 at 02:09:17PM +0300, Pavel Luzanov wrote: Please consider to add item to the psql section: Add psql \drg command to display role grants and remove the "Member of" column from \du & \dg altogether (d65ddaca) The release notes are only current as of 2023-06-26 and I will consider this when I updated them next week, thanks. This item is a part of Beta 3 scheduled for August 10, 2023 (today). [1] It might be worth updating the release notes before the release. But I'm not familiar with the release process in detail, so I could be wrong. 1. https://www.postgresql.org/message-id/93c00ac3-08b3-33ba-5d77-6ceb6ab20254%40postgresql.org -- Pavel Luzanov Postgres Professional: https://postgrespro.com
Re: multiple membership grants and information_schema.applicable_roles
On 24.07.2023 09:42, Pavel Luzanov wrote: Is IS_GRANTABLE a key column for ROLE_AUTHORIZATION_DESCRIPTORS? If not, duplicates is not possible. Right? The answer is: no. Duplicate pairs (grantee, role_name) is impossible only with defined key with this two columns. If there is no such key or key contain another column, for example grantor, then the information_schema.applicable_roles view definition is correct in this part. -- Pavel Luzanov Postgres Professional: https://postgrespro.com
Re: multiple membership grants and information_schema.applicable_roles
On 23.07.2023 23:03, Tom Lane wrote: CREATE RECURSIVE VIEW APPLICABLE_ROLES ( GRANTEE, ROLE_NAME, IS_GRANTABLE ) AS ( ( SELECT GRANTEE, ROLE_NAME, IS_GRANTABLE FROM DEFINITION_SCHEMA.ROLE_AUTHORIZATION_DESCRIPTORS WHERE ( GRANTEE IN ( CURRENT_USER, 'PUBLIC' ) OR GRANTEE IN ( SELECT ROLE_NAME FROM ENABLED_ROLES ) ) ) UNION ( SELECT RAD.GRANTEE, RAD.ROLE_NAME, RAD.IS_GRANTABLE FROM DEFINITION_SCHEMA.ROLE_AUTHORIZATION_DESCRIPTORS RAD JOIN APPLICABLE_ROLES R ON RAD.GRANTEE = R.ROLE_NAME ) ); The UNION would remove rows only when they are duplicates across all three columns. Hm, I think there is one more thing to check in the SQL standard. Is IS_GRANTABLE a key column for ROLE_AUTHORIZATION_DESCRIPTORS? If not, duplicates is not possible. Right? Can't check now, since I don't have access to the SQL standard definition. I do see what seems like a different issue: the standard appears to expect that indirect role grants should also be shown (via the recursive CTE), and we are not doing that. I noticed this, but the view stays unchanged so long time. I thought it was done intentionally. -- Pavel Luzanov Postgres Professional: https://postgrespro.com
multiple membership grants and information_schema.applicable_roles
I found that multiple membership grants added in v16 affects the information_schema.applicable_roles view. Examples on a master, but they works for v16 too. Setup multiple membership alice in bob: postgres@postgres(17.0)=# \drg alice List of role grants Role name | Member of | Options | Grantor ---+---+--+-- alice | bob | INHERIT, SET | alice alice | bob | INHERIT, SET | charlie alice | bob | ADMIN | postgres (3 rows) The application_roles view shows duplicates: postgres@postgres(17.0)=# SELECT * FROM information_schema.applicable_roles WHERE grantee = 'alice'; grantee | role_name | is_grantable -+---+-- alice | bob | NO alice | bob | YES (2 rows) View definition: postgres@postgres(17.0)=# \sv information_schema.applicable_roles CREATE OR REPLACE VIEW information_schema.applicable_roles AS SELECT a.rolname::information_schema.sql_identifier AS grantee, b.rolname::information_schema.sql_identifier AS role_name, CASE WHEN m.admin_option THEN 'YES'::text ELSE 'NO'::text END::information_schema.yes_or_no AS is_grantable FROM ( SELECT pg_auth_members.member, pg_auth_members.roleid, pg_auth_members.admin_option FROM pg_auth_members UNION SELECT pg_database.datdba, pg_authid.oid, false FROM pg_database, pg_authid WHERE pg_database.datname = current_database() AND pg_authid.rolname = 'pg_database_owner'::name) m JOIN pg_authid a ON m.member = a.oid JOIN pg_authid b ON m.roleid = b.oid WHERE pg_has_role(a.oid, 'USAGE'::text); I think that only one row with admin option should be returned. This can be achieved by adding group by + bool_or to the inner select from pg_auth_members. BEGIN; BEGIN postgres@postgres(17.0)=*# CREATE OR REPLACE VIEW information_schema.applicable_roles AS SELECT a.rolname::information_schema.sql_identifier AS grantee, b.rolname::information_schema.sql_identifier AS role_name, CASE WHEN m.admin_option THEN 'YES'::text ELSE 'NO'::text END::information_schema.yes_or_no AS is_grantable FROM ( SELECT pg_auth_members.member, pg_auth_members.roleid, bool_or(pg_auth_members.admin_option) AS admin_option FROM pg_auth_members GROUP BY 1, 2 UNION SELECT pg_database.datdba, pg_authid.oid, false FROM pg_database, pg_authid WHERE pg_database.datname = current_database() AND pg_authid.rolname = 'pg_database_owner'::name) m JOIN pg_authid a ON m.member = a.oid JOIN pg_authid b ON m.roleid = b.oid WHERE pg_has_role(a.oid, 'USAGE'::text); CREATE VIEW postgres@postgres(17.0)=*# SELECT * FROM information_schema.applicable_roles WHERE grantee = 'alice'; grantee | role_name | is_grantable -+---+-- alice | bob | YES (1 row) postgres@postgres(17.0)=*# ROLLBACK; ROLLBACK Should we add group by + bool_or to the applicable_roles view? -- Pavel Luzanov Postgres Professional: https://postgrespro.com
Re: PG 16 draft release notes ready
Please consider to add item to the psql section: Add psql \drg command to display role grants and remove the "Member of" column from \du & \dg altogether (d65ddaca) -- Pavel Luzanov Postgres Professional: https://postgrespro.com
Re: psql: Add role's membership options to the \du+ command
On 19.07.2023 19:47, Tom Lane wrote: And done, with some minor editorialization. Thanks to everyone who participated in the work. Special thanks to David for moving forward this patch for a long time, and to Tom for taking commit responsibilities. -- Pavel Luzanov Postgres Professional: https://postgrespro.com
Re: psql: Add role's membership options to the \du+ command
On 13.07.2023 11:26, Pavel Luzanov wrote:
And for versions <16 I forget to simplify expression for 'Options'
column after removing LEFT JOIN on pam:
SELECT m.rolname AS "Role name", r.rolname AS "Member of",
pg_catalog.concat_ws(', ',
CASE WHEN pam.admin_option THEN 'ADMIN' END,
CASE WHEN pam.roleid IS NOT NULL AND m.rolinherit THEN 'INHERIT' END,
CASE WHEN pam.roleid IS NOT NULL THEN 'SET' END
) AS "Options",
g.rolname AS "Grantor"
FROM pg_catalog.pg_roles m
JOIN pg_catalog.pg_auth_members pam ON (pam.member = m.oid)
LEFT JOIN pg_catalog.pg_roles r ON (pam.roleid = r.oid)
LEFT JOIN pg_catalog.pg_roles g ON (pam.grantor = g.oid)
WHERE m.rolname !~ '^pg_'
ORDER BY 1, 2, 4;
I plan to replace it to:
pg_catalog.concat_ws(', ',
CASE WHEN pam.admin_option THEN 'ADMIN' END,
CASE WHEN m.rolinherit THEN 'INHERIT' END,
'SET'
) AS "Options",
The new version contains only this change.
--
-
Pavel Luzanov
From d25743f87462ef89f9c098f0b907ac1fff473a99 Mon Sep 17 00:00:00 2001
From: Pavel Luzanov
Date: Fri, 14 Jul 2023 00:08:03 +0300
Subject: [PATCH v10] psql: Add \drg command to show role memberships
New command shows assigned privileges for each membership
ADMIN, INHERIT (added by e3ce2de0) or SET (3d14e171) and
who is grantor. This is important to know
how privileges can be used and who can revoke membership.
Without this command you need to query pg_auth_members directly.
This patch also removes the "Member of" column from the \du & \dg
commands, as it does not provide enough information about membership.
---
doc/src/sgml/ref/psql-ref.sgml | 21
src/bin/psql/command.c | 2 +
src/bin/psql/describe.c| 87 +-
src/bin/psql/describe.h| 3 ++
src/bin/psql/help.c| 1 +
src/bin/psql/tab-complete.c| 6 ++-
src/test/regress/expected/psql.out | 46 ++--
src/test/regress/sql/psql.sql | 26 +
8 files changed, 174 insertions(+), 18 deletions(-)
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 35aec6d3ce..e09fd06105 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -1883,6 +1883,27 @@ INSERT INTO tbl1 VALUES ($1, $2) \bind 'first value' 'second value' \g
+
+
+\drg[S] [ pattern ]
+
+
+Lists detailed information about each role membership, including
+assigned options (ADMIN,
+INHERIT or SET) and grantor.
+See the GRANT
+command for their meaning.
+
+
+By default, only user-created roles are shown; supply the
+S modifier to include system roles.
+If pattern is specified,
+only those roles whose names match the pattern are listed.
+
+
+
+
+
\drds [ role-pattern [ database-pattern ] ]
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 511debbe81..88a653a709 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -918,6 +918,8 @@ exec_command_d(PsqlScanState scan_state, bool active_branch, const char *cmd)
free(pattern2);
}
+else if (strncmp(cmd, "drg", 3) == 0)
+ success = describeRoleGrants(pattern, show_system);
else
status = PSQL_CMD_UNKNOWN;
break;
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 9325a46b8f..722ba95420 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -3617,7 +3617,7 @@ describeRoles(const char *pattern, bool verbose, bool showSystem)
PGresult *res;
printTableContent cont;
printTableOpt myopt = pset.popt.topt;
- int ncols = 3;
+ int ncols = 2;
int nrows = 0;
int i;
int conns;
@@ -3631,11 +3631,7 @@ describeRoles(const char *pattern, bool verbose, bool showSystem)
printfPQExpBuffer(,
"SELECT r.rolname, r.rolsuper, r.rolinherit,\n"
" r.rolcreaterole, r.rolcreatedb, r.rolcanlogin,\n"
- " r.rolconnlimit, r.rolvaliduntil,\n"
- " ARRAY(SELECT b.rolname\n"
- "FROM pg_catalog.pg_auth_members m\n"
- "JOIN pg_catalog.pg_roles b ON (m.roleid = b.oid)\n"
- "WHERE m.member = r.oid) as memberof");
+ " r.rolconnlimit, r.rolvaliduntil\n");
if (verbose)
{
@@ -3675,8 +3671,6 @@ describeRoles(const char *pattern, bool verbose, bool showSystem)
printTableAddHeader(, gettext_noop("Role name"), true, align);
printTableAddHeader(, gettext_noop("Attributes"), true, align);
- /* ignores implicit memberships from superuser & pg_database_owner */
- printTableAddHeader(, gettext_noop("Member of"), true, align);
if (verbose)
printTableAddHeader(, gettext_noop("Description
Re: psql: Add role's membership options to the \du+ command
On 13.07.2023 18:01, Tom Lane wrote: The idea with that, IMO, is to do something at least minimally sane if there's a bogus role OID in pg_auth_members. With plain joins, the output row would disappear and you'd have no clue that anything is wrong. With left joins, you get a row with a null column and there's reason to investigate why. Since such a case should not happen in normal use, I don't think it counts for discussions about compactness of output. However, this is also an argument for using a plain not left join between pg_roles and pg_auth_members: if we do it as per the earlier patch, then nulls in the output are common and wouldn't draw your attention. (Indeed, I think broken and not-broken pg_auth_members contents would be indistinguishable.) It reminds me about defensive programming practices. That's great, thanks for explanation. That does not seem right. Is it impossible for pam.set_option to be false? Even if it is, should this code assume that? For versions before 16, including one role to another automatically gives possibility to issue SET ROLE. IMO, the only question is whether it is correct to show IMPLICIT and SET options in versions where they are not actually present in pg_auth_members, but can be determined. -- Pavel Luzanov Postgres Professional: https://postgrespro.com
Re: psql: Add role's membership options to the \du+ command
On 08.07.2023 20:07, Tom Lane wrote
3. Not sure about use of LEFT JOIN in the query. That will mean we
get a row out even for roles that have no grants, which seems like
clutter. The LEFT JOINs to r and g are fine, but I suggest changing
the first join to a plain join.
Hm.
Can you explain why LEFT JOIN to r and g are fine after removing LEFT
JOIN to pam?
Why not to change all three left joins to plain join?
The query for v16+ now looks like:
SELECT m.rolname AS "Role name", r.rolname AS "Member of",
pg_catalog.concat_ws(', ',
CASE WHEN pam.admin_option THEN 'ADMIN' END,
CASE WHEN pam.inherit_option THEN 'INHERIT' END,
CASE WHEN pam.set_option THEN 'SET' END
) AS "Options",
g.rolname AS "Grantor"
FROM pg_catalog.pg_roles m
JOIN pg_catalog.pg_auth_members pam ON (pam.member = m.oid)
LEFT JOIN pg_catalog.pg_roles r ON (pam.roleid = r.oid)
LEFT JOIN pg_catalog.pg_roles g ON (pam.grantor = g.oid)
WHERE m.rolname !~ '^pg_'
ORDER BY 1, 2, 4;
And for versions <16 I forget to simplify expression for 'Options'
column after removing LEFT JOIN on pam:
SELECT m.rolname AS "Role name", r.rolname AS "Member of",
pg_catalog.concat_ws(', ',
CASE WHEN pam.admin_option THEN 'ADMIN' END,
CASE WHEN pam.roleid IS NOT NULL AND m.rolinherit THEN 'INHERIT' END,
CASE WHEN pam.roleid IS NOT NULL THEN 'SET' END
) AS "Options",
g.rolname AS "Grantor"
FROM pg_catalog.pg_roles m
JOIN pg_catalog.pg_auth_members pam ON (pam.member = m.oid)
LEFT JOIN pg_catalog.pg_roles r ON (pam.roleid = r.oid)
LEFT JOIN pg_catalog.pg_roles g ON (pam.grantor = g.oid)
WHERE m.rolname !~ '^pg_'
ORDER BY 1, 2, 4;
I plan to replace it to:
pg_catalog.concat_ws(', ',
CASE WHEN pam.admin_option THEN 'ADMIN' END,
CASE WHEN m.rolinherit THEN 'INHERIT' END,
'SET'
) AS "Options",
--
Pavel Luzanov
Postgres Professional: https://postgrespro.com
Re: psql: Add role's membership options to the \du+ command
On 09.07.2023 13:56, Pavel Luzanov wrote:
On 08.07.2023 20:07, Tom Lane wrote:
1. I was thinking in terms of dropping the "Member of" column entirely
in \du and \dg. It doesn't tell you enough, and the output of those
commands is often too wide already.
2. You have describeRoleGrants() set up to localize "ADMIN", "INHERIT",
and "SET". Since those are SQL keywords, our usual practice is to not
localize them; this'd simplify the code.
3. Not sure about use of LEFT JOIN in the query. That will mean we
get a row out even for roles that have no grants, which seems like
clutter. The LEFT JOINs to r and g are fine, but I suggest changing
the first join to a plain join.
So, I accepted all three suggestions. I will wait for other opinions and
plan to implement discussed changes within a few days.
Please review the updated version with suggested changes.
--
Pavel Luzanov
Postgres Professional: https://postgrespro.com
From ca303798c14b75cbd0131f850d93c68508ac62e9 Mon Sep 17 00:00:00 2001
From: Pavel Luzanov
Date: Wed, 12 Jul 2023 13:00:07 +0300
Subject: [PATCH v9] psql: add \drg command to show role memberships and grants
information.
New command shows assigned privileges for each membership
(ADMIN, INHERIT or SET) and who is grantor. This is important to know
which privileges can be used and how to revoke membership.
Without this command you need to query pg_auth_members directly.
This patch also removes the "Member of" column from the \du & \dg
commands, as it does not provide enough information about membership.
---
doc/src/sgml/ref/psql-ref.sgml | 21
src/bin/psql/command.c | 2 +
src/bin/psql/describe.c| 87 +-
src/bin/psql/describe.h| 3 ++
src/bin/psql/help.c| 1 +
src/bin/psql/tab-complete.c| 6 ++-
src/test/regress/expected/psql.out | 46 ++--
src/test/regress/sql/psql.sql | 26 +
8 files changed, 174 insertions(+), 18 deletions(-)
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 35aec6d3ce..e09fd06105 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -1883,6 +1883,27 @@ INSERT INTO tbl1 VALUES ($1, $2) \bind 'first value' 'second value' \g
+
+
+\drg[S] [ pattern ]
+
+
+Lists detailed information about each role membership, including
+assigned options (ADMIN,
+INHERIT or SET) and grantor.
+See the GRANT
+command for their meaning.
+
+
+By default, only user-created roles are shown; supply the
+S modifier to include system roles.
+If pattern is specified,
+only those roles whose names match the pattern are listed.
+
+
+
+
+
\drds [ role-pattern [ database-pattern ] ]
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 511debbe81..88a653a709 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -918,6 +918,8 @@ exec_command_d(PsqlScanState scan_state, bool active_branch, const char *cmd)
free(pattern2);
}
+else if (strncmp(cmd, "drg", 3) == 0)
+ success = describeRoleGrants(pattern, show_system);
else
status = PSQL_CMD_UNKNOWN;
break;
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 9325a46b8f..434043594a 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -3617,7 +3617,7 @@ describeRoles(const char *pattern, bool verbose, bool showSystem)
PGresult *res;
printTableContent cont;
printTableOpt myopt = pset.popt.topt;
- int ncols = 3;
+ int ncols = 2;
int nrows = 0;
int i;
int conns;
@@ -3631,11 +3631,7 @@ describeRoles(const char *pattern, bool verbose, bool showSystem)
printfPQExpBuffer(,
"SELECT r.rolname, r.rolsuper, r.rolinherit,\n"
" r.rolcreaterole, r.rolcreatedb, r.rolcanlogin,\n"
- " r.rolconnlimit, r.rolvaliduntil,\n"
- " ARRAY(SELECT b.rolname\n"
- "FROM pg_catalog.pg_auth_members m\n"
- "JOIN pg_catalog.pg_roles b ON (m.roleid = b.oid)\n"
- "WHERE m.member = r.oid) as memberof");
+ " r.rolconnlimit, r.rolvaliduntil\n");
if (verbose)
{
@@ -3675,8 +3671,6 @@ describeRoles(const char *pattern, bool verbose, bool showSystem)
printTableAddHeader(, gettext_noop("Role name"), true, align);
printTableAddHeader(, gettext_noop("Attributes"), true, align);
- /* ignores implicit memberships from superuser & pg_database_owner */
- printTableAddHeader(, gettext_noop("Member of"), true, align);
if (verbose)
printTableAddHeader(, gettext_noop("Description"), true, align);
@@ -3701,11 +3695,11 @@ desc
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: psql: Add role's membership options to the \du+ command
On 08.07.2023 20:07, Tom Lane wrote: 1. I was thinking in terms of dropping the "Member of" column entirely in \du and \dg. It doesn't tell you enough, and the output of those commands is often too wide already. I understood it that way that the dropping "Member of" column will be done as part of another work for v17. [1] But I'm ready to do it now. 2. You have describeRoleGrants() set up to localize "ADMIN", "INHERIT", and "SET". Since those are SQL keywords, our usual practice is to not localize them; this'd simplify the code. The reason is that \du has translatable all attributes of the role, including NOINHERIT. I decided to make a new command the same way. But I'm ready to leave them untranslatable, if no objections. 3. Not sure about use of LEFT JOIN in the query. That will mean we get a row out even for roles that have no grants, which seems like clutter. The LEFT JOINs to r and g are fine, but I suggest changing the first join to a plain join. It was David's suggestion: On 24.06.2023 18:57, David G. Johnston wrote: On Sat, Jun 24, 2023 at 8:11 AM Pavel Luzanov wrote: * The new meta-command will not show all roles. It will only show the roles included in other roles. To show all roles you need to add an outer join between pg_roles and pg_auth_members. But all columns except "role" will be left blank. Is it worth doing this? I'm inclined to want this. I would be good when specifying a role to filter upon that all rows that do exist matching that filter end up in the output regardless if they are standalone or not. Personally, I tend to think that left join is not needed. No memberships - nothing shown. So, I accepted all three suggestions. I will wait for other opinions and plan to implement discussed changes within a few days. 1. https://www.postgresql.org/message-id/4133242.1687481416%40sss.pgh.pa.us -- Pavel Luzanov Postgres Professional:https://postgrespro.com
Re: psql: Add role's membership options to the \du+ command
Please find attached new patch version.
It implements \drg command and hides duplicates in \du & \dg commands.
--
Pavel Luzanov
Postgres Professional: https://postgrespro.com
From a117f13fd497bf6ff8a504bcda6cb10d34dd22a7 Mon Sep 17 00:00:00 2001
From: Pavel Luzanov
Date: Mon, 26 Jun 2023 22:10:31 +0300
Subject: [PATCH v8] psql: add \drg command to show role memberships and grants
information.
New command shows assigned privileges for each membership (ADMIN, INHERIT
or SET) and who is grantor. This is important to know which privileges can
be used and how to revoke membership. Without this command you need to query
pg_auth_members directly.
Since v16 it is possible to include one role to another several times with
different grantors. Therefore, the 'Member of' column of the \du
and \dg commands now shows a sorted array of distinct roles to hide duplicates.
---
doc/src/sgml/ref/psql-ref.sgml | 27 +-
src/bin/psql/command.c | 2 +
src/bin/psql/describe.c| 80 +-
src/bin/psql/describe.h| 3 ++
src/bin/psql/help.c| 1 +
src/bin/psql/tab-complete.c| 6 ++-
src/test/regress/expected/psql.out | 41 +++
src/test/regress/sql/psql.sql | 26 ++
8 files changed, 180 insertions(+), 6 deletions(-)
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 35aec6d3ce..2d2e394a84 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -1722,7 +1722,8 @@ INSERT INTO tbl1 VALUES ($1, $2) \bind 'first value' 'second value' \g
Lists database roles.
(Since the concepts of users and groups have been
unified into roles, this command is now equivalent to
-\du.)
+\du.) For each role a sorted list of distinct roles
+of which it is a member is shown in array format.
By default, only user-created roles are shown; supply the
S modifier to include system roles.
If pattern is specified,
@@ -1883,6 +1884,27 @@ INSERT INTO tbl1 VALUES ($1, $2) \bind 'first value' 'second value' \g
+
+
+\drg[S] [ pattern ]
+
+
+Lists detailed information about each role membership, including
+assigned options (ADMIN,
+INHERIT or SET) and grantor.
+See the GRANT
+command for their meaning.
+
+
+By default, only user-created roles are shown; supply the
+S modifier to include system roles.
+If pattern is specified,
+only those roles whose names match the pattern are listed.
+
+
+
+
+
\drds [ role-pattern [ database-pattern ] ]
@@ -1957,7 +1979,8 @@ INSERT INTO tbl1 VALUES ($1, $2) \bind 'first value' 'second value' \g
Lists database roles.
(Since the concepts of users and groups have been
unified into roles, this command is now equivalent to
-\dg.)
+\dg.) For each role a sorted list of distinct roles
+of which it is a member is shown in array format.
By default, only user-created roles are shown; supply the
S modifier to include system roles.
If pattern is specified,
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 511debbe81..88a653a709 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -918,6 +918,8 @@ exec_command_d(PsqlScanState scan_state, bool active_branch, const char *cmd)
free(pattern2);
}
+else if (strncmp(cmd, "drg", 3) == 0)
+ success = describeRoleGrants(pattern, show_system);
else
status = PSQL_CMD_UNKNOWN;
break;
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 9325a46b8f..e4101fe36b 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -3632,10 +3632,11 @@ describeRoles(const char *pattern, bool verbose, bool showSystem)
"SELECT r.rolname, r.rolsuper, r.rolinherit,\n"
" r.rolcreaterole, r.rolcreatedb, r.rolcanlogin,\n"
" r.rolconnlimit, r.rolvaliduntil,\n"
- " ARRAY(SELECT b.rolname\n"
+ " ARRAY(SELECT DISTINCT b.rolname\n"
"FROM pg_catalog.pg_auth_members m\n"
"JOIN pg_catalog.pg_roles b ON (m.roleid = b.oid)\n"
- "WHERE m.member = r.oid) as memberof");
+ "WHERE m.member = r.oid\n"
+ "ORDER BY 1) as memberof");
if (verbose)
{
@@ -3753,6 +3754,81 @@ describeRoles(const char *pattern, bool verbose, bool showSystem)
return true;
}
+/*
+ * \drg
+ * Describes role grants.
+ */
+bool
+describeRoleGrants(const char *pattern, bool showSystem)
+{
+ PQExpBufferData buf;
+ PGresult *res;
+ printQueryOpt myopt = pset.popt;
+
+ initPQExpBuffer();
+ printfPQExpBuffer(,
+ "SELECT
Re: psql: Add role's membership options to the \du+ command
On 24.06.2023 18:57, David G. Johnston wrote: On Sat, Jun 24, 2023 at 8:11 AM Pavel Luzanov wrote: There are two commands showing the same information about roles: \du and \dr. I would add \dr as the new official command to complement adding \drg and deprecate both \du and \dg. Though actual removal and de-documenting doesn't seem like a good idea. But if we ever did assign something non-role to \dr it would be very confusing. It's my mistake and inattention. I was thinking about '\du' and '\dg', and wrote about '\du' and '\dr'. I agree that \dr and \drg the best names. So, now concentrating on implementing \drg. * The new meta-command will also make sense for versions <16. The ADMIN OPTION is available in all supported versions. Doesn't every role pre-16 gain SET permission? We can also deduce whether the grant provides INHERIT based upon the attribute of the role in question. Indeed! I will do so. * The new meta-command will not show all roles. It will only show the roles included in other roles. To show all roles you need to add an outer join between pg_roles and pg_auth_members. But all columns except "role" will be left blank. Is it worth doing this? I'm inclined to want this. I would be good when specifying a role to filter upon that all rows that do exist matching that filter end up in the output regardless if they are standalone or not. Ok -- 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
Re: psql: Add role's membership options to the \du+ command
Thank you for all valuable comments. I can now continue working on the
patch.
Here's what I plan to do in the next version.
Changes for \du & \dg commands
* showing distinct roles in the "Member of" column
* explicit order for list of roles
* no changes for extended mode (\du+)
New meta-command \drg
* showing info from pg_auth_members based on a query:
SELECT r.rolname role, m.rolname member,
pg_catalog.concat_ws(', ',
CASE WHEN pam.admin_option THEN 'ADMIN' END,
CASE WHEN pam.inherit_option THEN 'INHERIT' END,
CASE WHEN pam.set_option THEN 'SET' END
) AS options,
g.rolname grantor
FROM pg_catalog.pg_auth_members pam
JOIN pg_catalog.pg_roles r ON (pam.roleid = r.oid)
JOIN pg_catalog.pg_roles m ON (pam.member = m.oid)
JOIN pg_catalog.pg_roles g ON (pam.grantor = g.oid)
WHERE r.rolname !~ '^pg_'
ORDER BY role, member, grantor;
role | member | options | grantor
--+--+-+--
regress_du_role0 | regress_du_admin | ADMIN, INHERIT, SET | postgres
regress_du_role0 | regress_du_role1 | ADMIN, INHERIT, SET |
regress_du_admin
regress_du_role0 | regress_du_role1 | INHERIT | regress_du_role1
regress_du_role0 | regress_du_role1 | SET | regress_du_role2
regress_du_role0 | regress_du_role2 | ADMIN | regress_du_admin
regress_du_role0 | regress_du_role2 | INHERIT, SET | regress_du_role1
regress_du_role0 | regress_du_role2 | | regress_du_role2
regress_du_role1 | regress_du_admin | ADMIN, INHERIT, SET | postgres
regress_du_role1 | regress_du_role2 | ADMIN, SET | regress_du_admin
regress_du_role2 | regress_du_admin | ADMIN, INHERIT, SET | postgres
(10 rows)
Notes
* The name of the new command. It's a good name, if not for the history.
There are two commands showing the same information about roles: \du and
\dr.
The addition of \drg may be misinterpreted: if there is \drg, then there
is also \dug.
Maybe it's time to think about deprecating of the \du command and leave
only \dg in the next versions?
* 'empty'. I suggest thinking about forbidding the situation with empty
options.
If we prohibit them, the issue will be resolved automatically.
* The new meta-command will also make sense for versions <16.
The ADMIN OPTION is available in all supported versions.
* The new meta-command will not show all roles. It will only show the
roles included in other roles.
To show all roles you need to add an outer join between pg_roles and
pg_auth_members.
But all columns except "role" will be left blank. Is it worth doing this?
--
Pavel Luzanov
Postgres Professional: https://postgrespro.com
Re: psql: Add role's membership options to the \du+ command
On 18.05.2023 05:42, Jonathan S. Katz wrote: That said, from a readability standpoint, it was easier for me to follow the tabular form vs. the sentence form. May be possible to reach a agreement on the sentence form. Similar descriptions used for referential constraints in the \d command: create table t1 (id int primary key);create table t2 (id int references t1(id));\d t2 Table "public.t2" Column | Type | Collation | Nullable | Default +-+---+--+- id | integer | | | Foreign-key constraints: "t2_id_fkey" FOREIGN KEY (id) REFERENCES t1(id)As for tabular form it looks more natural to have a separate psql command for pg_auth_members system catalog. Something based on this query:SELECT r.rolname role, m.rolname member, admin_option admin, inherit_option inherit, set_option set, g.rolname grantorFROM pg_catalog.pg_auth_members pam JOIN pg_catalog.pg_roles r ON (pam.roleid = r.oid) JOIN pg_catalog.pg_roles m ON (pam.member = m.oid) JOIN pg_catalog.pg_roles g ON (pam.grantor = g.oid)WHERE r.rolname !~ '^pg_'ORDER BY role, member, grantor; role | member | admin | inherit | set | grantor --+--+---+-+-+-- regress_du_role0 | regress_du_admin | t | t | t | postgres regress_du_role0 | regress_du_role1 | t | t | t | regress_du_admin regress_du_role0 | regress_du_role1 | f | t | f | regress_du_role1 regress_du_role0 | regress_du_role1 | f | f | t | regress_du_role2 regress_du_role0 | regress_du_role2 | t | f | f | regress_du_admin regress_du_role0 | regress_du_role2 | f | t | t | regress_du_role1 regress_du_role0 | regress_du_role2 | f | f | f | regress_du_role2 regress_du_role1 | regress_du_admin | t | t | t | postgres regress_du_role1 | regress_du_role2 | t | f | t | regress_du_admin regress_du_role2 | regress_du_admin | t | t | t | postgres(10 rows)But is it worth inventing a new psql command for this? - Pavel Luzanov
Re: psql: Add role's membership options to the \du+ command
On 05.05.2023 19:51, David G. Johnston wrote: But if it is really a blocker then maybe we should produce 3 separate newline separated columns, one for the member of role, one for the list of attributes, and one with the grantor. The column headers can be translated more easily as single nouns. The readability quite probably would end up being equivalent (maybe even better) in tabular form instead of sentence form. Just to visualize this approach. Below are the output for the tabular form and the sentence form from last patch version (sql script attached): Tabular form rolname | memberof | options | grantor --+--+-+-- postgres | | | regress_du_admin | regress_du_role0+| admin, inherit, set+| postgres + | regress_du_role1+| admin, inherit, set+| postgres + | regress_du_role2 | admin, inherit, set | postgres regress_du_role0 | | | regress_du_role1 | regress_du_role0+| admin, inherit, set+| regress_du_admin+ | regress_du_role0+| inherit +| regress_du_role1+ | regress_du_role0 | set | regress_du_role2 regress_du_role2 | regress_du_role0+| admin +| regress_du_admin+ | regress_du_role0+| inherit, set +| regress_du_role1+ | regress_du_role0+| empty +| regress_du_role2+ | regress_du_role1 | admin, set | regress_du_admin(5 rows)Sentence form from patch v7 rolname | memberof --+-- postgres | regress_du_admin | regress_du_role0 from postgres (admin, inherit, set) + | regress_du_role1 from postgres (admin, inherit, set) + | regress_du_role2 from postgres (admin, inherit, set) regress_du_role0 | regress_du_role1 | regress_du_role0 from regress_du_admin (admin, inherit, set)+ | regress_du_role0 from regress_du_role1 (inherit) + | regress_du_role0 from regress_du_role2 (set) regress_du_role2 | regress_du_role0 from regress_du_admin (admin) + | regress_du_role0 from regress_du_role1 (inherit, set) + | regress_du_role0 from regress_du_role2 (empty) + | regress_du_role1 from regress_du_admin (admin, set)(5 rows) The tabular form solves the latest patch translation problems mentioned by Kyotaro. But it requires mapping elements between 3 array-like columns. To move forward, needs more opinions? - Pavel Luzanov t.sql Description: application/sql
Re: pg_stat_bgwriter.buffers_backend is pretty meaningless (and more?)
On 24.04.2023 23:53, Melanie Plageman wrote: I copied the committer who most recently touched pg_stat_io (Michael Paquier) to see if we could get someone interested in committing this docs update. I can explain my motivation by suggesting this update. pg_stat_io is a very impressive feature. So I decided to try it. I see 4 rows for some 'standalone backend' out of 30 total rows of the view. The attempt to find description of 'standalone backend' in the docs did not result in anything. pg_stat_io page references pg_stat_activity for backend types. But pg_stat_activity page doesn't say anything about 'standalone backend'. I think this question will be popular without clarifying in docs. -- Pavel Luzanov Postgres Professional: https://postgrespro.com
Re: psql: Add role's membership options to the \du+ command
On 14.04.2023 10:28, Kyotaro Horiguchi wrote: As David-G appears to express concern in upthread, I don't think a direct Japanese translation from "rolename from rolename" works well, as the "from" needs accompanying verb. I, as a Japanese speaker, I prefer a more non-sentence-like notation, similar to David's suggestion but with slight differences: "pg_read_all_stats (grantor: postgres, inherit, set)" In this form, it confuses me that 'postgres' and 'inherit, set' look like a common list. Come to think of this, I recalled a past discussion in which we concluded that translating punctuation marks appearing between a variable number of items within list expressions should be avoided... Thus, I'd like to propose to use an ACL-like notation, which doesn't need translation. ..| Mamber of | ..| pg_read_server_files=ais/horiguti,pg_execute_server_program=/postgres | It's very tempting to do so. But I don't like this approach. Showing membership options as an ACL-like column will be confusing. Even in your example, my first reaction is that pg_execute_server_program is available to public. (As for the general patterns, we can also consider combining three options into one column, like pg_class.reloptions.) So, yet another way to discuss: pg_read_all_stats(inherit,set/horiguti) pg_execute_server_program(empty/postgres) One more point. Grants without any option probably should be prohibited as useless. But this is for a new thread. -- Pavel Luzanov Postgres Professional: https://postgrespro.com
Re: psql: Add role's membership options to the \du+ command
After playing with the \du command, I found that we can't avoid translation.
All attributes are translatable. Also, two of nine attributes shows in
new line separated format (connection limit and password valid until).
$ LANGUAGE=fr psql -c "ALTER ROLE postgres CONNECTION LIMIT 3 VALID
UNTIL 'infinity'" -c '\du'
ALTER ROLE
Liste des rôles
Nom du rôle | Attributs | Membre de
-+-+---
postgres | Superutilisateur, Créer un rôle, Créer une base,
Réplication, Contournement RLS+| {}
| 3 connexions +|
| Mot de passe valide jusqu'à
infinity |
So I decided to keep the format suggested by David, but without
abbreviations and only for extended mode.
$ psql -c '\duS+'
List of roles
Role name | Attributes
| Member of | Description
-+---+---+-
pg_checkpoint | Cannot login
| |
pg_create_subscription | Cannot login
| |
pg_database_owner | Cannot login
| |
pg_execute_server_program | Cannot login
| |
pg_maintain | Cannot login
| |
pg_monitor | Cannot login |
pg_read_all_settings from postgres (inherit, set)+|
| |
pg_read_all_stats from postgres (inherit, set) +|
| |
pg_stat_scan_tables from postgres (inherit, set) |
pg_read_all_data | Cannot login
| |
pg_read_all_settings | Cannot login
| |
pg_read_all_stats | Cannot login
| |
pg_read_server_files | Cannot login
| |
pg_signal_backend | Cannot login
| |
pg_stat_scan_tables | Cannot login
| |
pg_use_reserved_connections | Cannot login
| |
pg_write_all_data | Cannot login
| |
pg_write_server_files | Cannot login
| |
postgres | Superuser
+| |
| Create role
+| |
| Create DB
+| |
| Replication
+| |
| Bypass RLS
+| |
| 3 connections
+| |
| Password valid until infinity
| |
Please look at new version. I understand that this is a compromise choice.
I am ready to change it if a better solution is offered.
P.S. If no objections I plan to add this patch to Open Items for v16
https://wiki.postgresql.org/wiki/PostgreSQL_16_Open_Items
On 05.04.2023 17:24, David G. Johnston wrote:
On Wed, Apr 5, 2023 at 6:58 AM Tom Lane wrote:
Pavel Luzanov writes:
> What if this long output will be available only for \du+, and
for \du
> just show distinct (without duplicates)
> roles in the current array format? For those, who don't care
about these
> new membership options, nothing will change.
> Those, who need details will use the + modifier.
> ?
I kind of like that. Would we change to newlines in the Attributes
field in both \du and \du+? (I'm +1 for that, but maybe others
aren't.)
If we don't change the \du "Member of" column display (aside from
removing duplicates) I'm disinclined to change the Attributes column.
I too am partial to only exposing this detail on the extended (+) display.
David J.
--
Pavel Luzanov
Postgres Professional:https://postgrespro.com
From 1f9433696e41a8f37cfd4c0514e136fedd50939e Mon Sep 17 00:00:00 2001
From: Pavel Luzanov
Date: Thu, 13 Apr
Re: pg_stat_bgwriter.buffers_backend is pretty meaningless (and more?)
On 05.04.2023 03:41, Melanie Plageman wrote: On Tue, Apr 4, 2023 at 4:35 PM Pavel Luzanov wrote: After a little thought... I'm not sure about the term 'bootstrap process'. I can't find this term in the documentation. There are various mentions of "bootstrap" peppered throughout the docs but no concise summary of what it is. For example, initdb docs mention the "bootstrap backend" [1]. Interestingly, 910cab820d0 added "Bootstrap superuser" in November. This doesn't really cover what bootstrapping is itself, but I wonder if that is useful? If so, you could propose a glossary entry for it? (preferably in a new thread) I'm not sure if this is the reason for adding a new entry in the glossary. Do I understand correctly that this is a postmaster? If so, then the postmaster process is not shown in pg_stat_activity. No, bootstrap process is for initializing the template database. You will not be able to see pg_stat_activity when it is running. Oh, it's clear to me now. Thank you for the explanation. You can query pg_stat_activity from single user mode, so it is relevant to pg_stat_activity also. I take your point that bootstrap mode isn't relevant for pg_stat_activity, but I am hesitant to add that distinction to the pg_stat_io docs since the reason you won't see it in pg_stat_activity is because it is ephemeral and before a user can access the database and not because stats are not tracked for it. Can you think of a way to convey this? See my attempt attached. I'm not sure about the wording. But I think we can avoid the term 'bootstrap process' by replacing it with "database cluster initialization", which should be clear to everyone. -- Pavel Luzanov Postgres Professional: https://postgrespro.com From ff76b81a9d52581f6fdaf9c1f3885e8272d2ae3c Mon Sep 17 00:00:00 2001 From: Pavel Luzanov Date: Mon, 10 Apr 2023 10:25:52 +0300 Subject: [PATCH v2] [PATCH v2] Document standalone backend type in pg_stat_activity Reported-by: Pavel Luzanov Discussion: https://www.postgresql.org/message-id/fcbe2851-f1fb-9863-54bc-a95dc7a0d946%40postgrespro.ru --- doc/src/sgml/monitoring.sgml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml index 3f33a1c56c..45e20efbfb 100644 --- a/doc/src/sgml/monitoring.sgml +++ b/doc/src/sgml/monitoring.sgml @@ -991,6 +991,9 @@ postgres 27093 0.0 0.0 30096 2752 ?Ss 11:34 0:00 postgres: ser archiver, startup, walreceiver, walsender and walwriter. + The special type standalone backend is used + when initializing a database cluster by + and when running in the . In addition, background workers registered by extensions may have additional types. -- 2.34.1
Re: psql: Add role's membership options to the \du+ command
On 04.04.2023 22:02, David G. Johnston wrote: On Tue, Apr 4, 2023 at 10:37 AM Tom Lane wrote: Robert Haas writes: > On Tue, Apr 4, 2023 at 1:12 PM Tom Lane wrote: >> I wonder if, while we're here, we should apply the idea of >> joining-with-newlines-not-commas to the attributes column too. > That would make the column narrower, which might be good, because it > seems to me that listing the memberships could require quite a lot of > space, both vertical and horizontal. Right, that's what I was thinking. So, by way of example: regress_du_role1 | cannot login | regress_du_role0 granted by regress_du_admin with admin, inherit, set | Description for regress_du_role1 Perhaps more closely to syntax? regress_du_role0 with admin, inherit, set granted by regress_du_admin instead of regress_du_role0 granted by regress_du_admin with admin, inherit, set No translations, all words are either identical to syntax or identifiers. I'm on board with newlines in the attributes field. +1 The specific member of column changes are: from -> granted by ( ) -> "with" ais -> admin, inherit, set I'm good with any or all of those selections, either as-is or in the more verbose form. From yesterday's discussion, I think two things are important: - it is advisable to avoid translation, - there is no sense in the abbreviation (a,i,s), if there are full names in the 'attributes' column. So I agree with such changes and plan to implement them. And one more question. (I think it's better to have it explicitly rejected than to keep silent.) What if this long output will be available only for \du+, and for \du just show distinct (without duplicates) roles in the current array format? For those, who don't care about these new membership options, nothing will change. Those, who need details will use the + modifier. ? -- Pavel Luzanov Postgres Professional:https://postgrespro.com
Re: psql: Add role's membership options to the \du+ command
On 04.04.2023 23:00, Robert Haas wrote: On Tue, Apr 4, 2023 at 3:02 PM David G. Johnston wrote: So, by way of example: regress_du_role1 | cannot login | regress_du_role0 granted by regress_du_admin with admin, inherit, set | Description for regress_du_role1 That seems wider than necessary. Why not have the third column be something like regress_du_role0 by regress_du_admin (admin, inherit, set)? 'granted by' can be left without translation, but just 'by' required translation, as I think. -- Pavel Luzanov Postgres Professional: https://postgrespro.com
Re: pg_stat_bgwriter.buffers_backend is pretty meaningless (and more?)
On 03.04.2023 23:50, Melanie Plageman wrote: Attached is a tiny patch to add standalone backend type to pg_stat_activity documentation (referenced by pg_stat_io). I mentioned both the bootstrap process and single user mode process in the docs, though I can't imagine that the bootstrap process is relevant for pg_stat_activity. After a little thought... I'm not sure about the term 'bootstrap process'. I can't find this term in the documentation. Do I understand correctly that this is a postmaster? If so, then the postmaster process is not shown in pg_stat_activity. Perhaps it may be worth adding a description of the standalone backend to pg_stat_io, not to pg_stat_activity. Something like: backend_type is all types from pg_stat_activity plus 'standalone backend', which is used for the postmaster process and in a single user mode. I also noticed that the pg_stat_activity docs call background workers "parallel workers" (though it also mentions that extensions could have other background workers registered), but this seems a bit weird because pg_stat_activity uses GetBackendTypeDesc() and this prints "background worker" for type B_BG_WORKER. Background workers doing parallelism tasks is what users will most often see in pg_stat_activity, but I feel like it is confusing to have it documented as something different than what would appear in the view. Unless I am misunderstanding something... 'parallel worker' appears in the pg_stat_activity for parallel queries. I think it's right here. -- Pavel Luzanov Postgres Professional: https://postgrespro.com
Re: pg_stat_bgwriter.buffers_backend is pretty meaningless (and more?)
Hello, I found that the 'standalone backend' backend type is not documented right now. Adding something like (from commit message) would be helpful: Both the bootstrap backend and single user mode backends will have backend_type STANDALONE_BACKEND. -- Pavel Luzanov Postgres Professional: https://postgrespro.com
Re: psql: Add role's membership options to the \du+ command
In the previous version, I didn't notice (unlike cfbot) the compiler
warning. Fixed in version 6.
-
Pavel Luzanov
From 1b8b5743df23637b70e8d4ad0df0e1f892c595f3 Mon Sep 17 00:00:00 2001
From: Pavel Luzanov
Date: Wed, 22 Mar 2023 20:54:41 +0300
Subject: [PATCH v6] psql: show membership options in the \du command
Format for the "Member of" column completely redesigned.
Shown within each row, in newline-separated format, are the memberships granted to
the role. The presentation includes both the name of the grantor
as well as the membership permissions (in an abbreviated format:
a for admin option, i for inherit option, s for set option.)
The word 'empty' is printed in the case that none of those permissions are granted.
---
doc/src/sgml/ref/psql-ref.sgml | 30 +++---
src/bin/psql/describe.c| 63 --
src/test/regress/expected/psql.out | 49 +++
src/test/regress/sql/psql.sql | 30 ++
4 files changed, 146 insertions(+), 26 deletions(-)
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 7b8ae9fac3..03e7da93de 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -1727,9 +1727,18 @@ INSERT INTO tbl1 VALUES ($1, $2) \bind 'first value' 'second value' \g
S modifier to include system roles.
If pattern is specified,
only those roles whose names match the pattern are listed.
-If the form \dg+ is used, additional information
-is shown about each role; currently this adds the comment for each
-role.
+
+
+Shown within each row, in newline-separated format, are the memberships granted to
+the role. The presentation includes both the name of the grantor
+as well as the membership permissions (in an abbreviated format:
+a for admin option, i for inherit option,
+s for set option.) The word empty is printed in
+the case that none of those permissions are granted.
+See the GRANT command for their meaning.
+
+
+If the form \dg+ is used the comment attached to the role is shown.
@@ -1969,9 +1978,18 @@ INSERT INTO tbl1 VALUES ($1, $2) \bind 'first value' 'second value' \g
S modifier to include system roles.
If pattern is specified,
only those roles whose names match the pattern are listed.
-If the form \du+ is used, additional information
-is shown about each role; currently this adds the comment for each
-role.
+
+
+Shown within each row, in newline-separated format, are the memberships granted to
+the role. The presentation includes both the name of the grantor
+as well as the membership permissions (in an abbreviated format:
+a for admin option, i for inherit option,
+s for set option.) The word empty is printed in
+the case that none of those permissions are granted.
+See the GRANT command for their meaning.
+
+
+If the form \du+ is used the comment attached to the role is shown.
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 99e28f607e..7f2b7c9363 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -3631,24 +3631,42 @@ describeRoles(const char *pattern, bool verbose, bool showSystem)
printfPQExpBuffer(,
"SELECT r.rolname, r.rolsuper, r.rolinherit,\n"
" r.rolcreaterole, r.rolcreatedb, r.rolcanlogin,\n"
- " r.rolconnlimit, r.rolvaliduntil,\n"
- " ARRAY(SELECT b.rolname\n"
- "FROM pg_catalog.pg_auth_members m\n"
- "JOIN pg_catalog.pg_roles b ON (m.roleid = b.oid)\n"
- "WHERE m.member = r.oid) as memberof");
+ " r.rolconnlimit, r.rolvaliduntil, r.rolreplication,\n");
- if (verbose)
- {
- appendPQExpBufferStr(, "\n, pg_catalog.shobj_description(r.oid, 'pg_authid') AS description");
- ncols++;
- }
- appendPQExpBufferStr(, "\n, r.rolreplication");
+ if (pset.sversion >= 16)
+ appendPQExpBufferStr(,
+ " (SELECT pg_catalog.string_agg(\n"
+ "pg_catalog.format('%I from %I (%s)',\n"
+ " b.rolname, m.grantor::pg_catalog.regrole::pg_catalog.text,\n"
+ " pg_catalog.regexp_replace(\n"
+ "pg_catalog.concat_ws(', ',\n"
+ " CASE WHEN m.admin_option THEN 'a' END,\n"
+ " CASE WHEN m.inherit_option THEN 'i' END,\n"
+ " CASE WHEN m.set_option THEN 's' END),\n"
+ " '^$', 'empty')\n"
+ "), E'\\n'\n"
+ "ORDER BY b.rolname, m.grantor::pg_catalog.regrole::p
Re: psql: Add role's membership options to the \du+ command
I would suggest tweaking the test output to include regress_du_admin ...
I found (with a help of cfbot) difficulty with this. The problem is the
bootstrap superuser name (oid=10).
This name depends on the OS username. In my case it's pal, but in most cases
it's postgres or something else.
And the output of \du regress_du_admin can't be predicted:
\du regress_du_admin
List of roles
Role name | Attributes | Member of
--+-+-
regress_du_admin | Create role | regress_du_role0 from pal (a, i, s)+
| | regress_du_role1 from pal (a, i, s)+
| | regress_du_role2 from pal (a, i, s)
So, I decided not to include regress_du_admin in the test output.
Please, see version 5 attached. Only tests changed.
-
Pavel Luzanov
From b8f35733126a843edd47a1f89da0d9f8babeec93 Mon Sep 17 00:00:00 2001
From: Pavel Luzanov
Date: Tue, 21 Mar 2023 05:58:31 +0300
Subject: [PATCH v5] psql: show membership options in the \du command
Format for the "Member of" column completely redesigned.
Shown within each row, in newline-separated format, are the memberships granted to
the role. The presentation includes both the name of the grantor
as well as the membership permissions (in an abbreviated format:
a for admin option, i for inherit option,
s for set option.) The word 'empty' is printed in
the case that none of those permissions are granted.
---
doc/src/sgml/ref/psql-ref.sgml | 30 ---
src/bin/psql/describe.c| 61 --
src/test/regress/expected/psql.out | 49
src/test/regress/sql/psql.sql | 30 +++
4 files changed, 144 insertions(+), 26 deletions(-)
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 7b8ae9fac3..03e7da93de 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -1727,9 +1727,18 @@ INSERT INTO tbl1 VALUES ($1, $2) \bind 'first value' 'second value' \g
S modifier to include system roles.
If pattern is specified,
only those roles whose names match the pattern are listed.
-If the form \dg+ is used, additional information
-is shown about each role; currently this adds the comment for each
-role.
+
+
+Shown within each row, in newline-separated format, are the memberships granted to
+the role. The presentation includes both the name of the grantor
+as well as the membership permissions (in an abbreviated format:
+a for admin option, i for inherit option,
+s for set option.) The word empty is printed in
+the case that none of those permissions are granted.
+See the GRANT command for their meaning.
+
+
+If the form \dg+ is used the comment attached to the role is shown.
@@ -1969,9 +1978,18 @@ INSERT INTO tbl1 VALUES ($1, $2) \bind 'first value' 'second value' \g
S modifier to include system roles.
If pattern is specified,
only those roles whose names match the pattern are listed.
-If the form \du+ is used, additional information
-is shown about each role; currently this adds the comment for each
-role.
+
+
+Shown within each row, in newline-separated format, are the memberships granted to
+the role. The presentation includes both the name of the grantor
+as well as the membership permissions (in an abbreviated format:
+a for admin option, i for inherit option,
+s for set option.) The word empty is printed in
+the case that none of those permissions are granted.
+See the GRANT command for their meaning.
+
+
+If the form \du+ is used the comment attached to the role is shown.
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 99e28f607e..9f7b7326e9 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -3631,24 +3631,42 @@ describeRoles(const char *pattern, bool verbose, bool showSystem)
printfPQExpBuffer(,
"SELECT r.rolname, r.rolsuper, r.rolinherit,\n"
" r.rolcreaterole, r.rolcreatedb, r.rolcanlogin,\n"
- " r.rolconnlimit, r.rolvaliduntil,\n"
- " ARRAY(SELECT b.rolname\n"
- "FROM pg_catalog.pg_auth_members m\n"
- "JOIN pg_catalog.pg_roles b ON (m.roleid = b.oid)\n"
- "WHERE m.member = r.oid) as memberof");
+ " r.rolconnlimit, r.rolvaliduntil, r.rolreplication,\n");
- if (verbose)
- {
- appendPQExpBufferStr(, "\n, pg_catalog.shobj_description(r.oid, 'pg_authid') AS description");
- ncols++;
- }
- appendPQExpBufferStr(, &
Re: psql: Add role's membership options to the \du+ command
On 10.03.2023 15:06, Pavel Luzanov wrote:
I missed the comment at the beginning of the file about version 9.2. I
will return the version check for rolbypassrls.
+
+ Shown within each row, in newline-separated format, are the
memberships granted to
+ the role. The presentation includes both the name of the
grantor
+ as well as the membership permissions (in an abbreviated format:
+ a for admin option, i
for inherit option,
+ s for set option.) The word
empty is printed in
+ the case that none of those permissions are granted.
+ See the linkend="sql-grant">GRANT command for their
meaning.
+
+
+ If the form \dg+ is used the comment
attached to the role is shown.
Thanks. I will replace the description with this one.
I would suggest tweaking the test output to include regress_du_admin
and also to make regress_du_admin a CREATEROLE role with LOGIN.
Ok.
Please review the attached version 4 with the changes discussed.
-
Pavel Luzanov
From b67cdf2299b6053ed247254c8be4edde472efdae Mon Sep 17 00:00:00 2001
From: Pavel Luzanov
Date: Mon, 20 Mar 2023 11:28:31 +0300
Subject: [PATCH v4] psql: show membership options in the \du command
---
doc/src/sgml/ref/psql-ref.sgml | 30 ---
src/bin/psql/describe.c| 61 --
src/test/regress/expected/psql.out | 55 +++
src/test/regress/sql/psql.sql | 30 +++
4 files changed, 150 insertions(+), 26 deletions(-)
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 7b8ae9fac3..03e7da93de 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -1727,9 +1727,18 @@ INSERT INTO tbl1 VALUES ($1, $2) \bind 'first value' 'second value' \g
S modifier to include system roles.
If pattern is specified,
only those roles whose names match the pattern are listed.
-If the form \dg+ is used, additional information
-is shown about each role; currently this adds the comment for each
-role.
+
+
+Shown within each row, in newline-separated format, are the memberships granted to
+the role. The presentation includes both the name of the grantor
+as well as the membership permissions (in an abbreviated format:
+a for admin option, i for inherit option,
+s for set option.) The word empty is printed in
+the case that none of those permissions are granted.
+See the GRANT command for their meaning.
+
+
+If the form \dg+ is used the comment attached to the role is shown.
@@ -1969,9 +1978,18 @@ INSERT INTO tbl1 VALUES ($1, $2) \bind 'first value' 'second value' \g
S modifier to include system roles.
If pattern is specified,
only those roles whose names match the pattern are listed.
-If the form \du+ is used, additional information
-is shown about each role; currently this adds the comment for each
-role.
+
+
+Shown within each row, in newline-separated format, are the memberships granted to
+the role. The presentation includes both the name of the grantor
+as well as the membership permissions (in an abbreviated format:
+a for admin option, i for inherit option,
+s for set option.) The word empty is printed in
+the case that none of those permissions are granted.
+See the GRANT command for their meaning.
+
+
+If the form \du+ is used the comment attached to the role is shown.
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 99e28f607e..9f7b7326e9 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -3631,24 +3631,42 @@ describeRoles(const char *pattern, bool verbose, bool showSystem)
printfPQExpBuffer(,
"SELECT r.rolname, r.rolsuper, r.rolinherit,\n"
" r.rolcreaterole, r.rolcreatedb, r.rolcanlogin,\n"
- " r.rolconnlimit, r.rolvaliduntil,\n"
- " ARRAY(SELECT b.rolname\n"
- "FROM pg_catalog.pg_auth_members m\n"
- "JOIN pg_catalog.pg_roles b ON (m.roleid = b.oid)\n"
- "WHERE m.member = r.oid) as memberof");
+ " r.rolconnlimit, r.rolvaliduntil, r.rolreplication,\n");
- if (verbose)
- {
- appendPQExpBufferStr(, "\n, pg_catalog.shobj_description(r.oid, 'pg_authid') AS description");
- ncols++;
- }
- appendPQExpBufferStr(, "\n, r.rolreplication");
+ if (pset.sversion >= 16)
+ appendPQExpBufferStr(,
+ " (SELECT pg_catalog.string_agg(\n"
+ "pg_catalog.format('%I from %I (%s)',\n"
+ " b.rolname, m.
Re: psql: Add role's membership options to the \du+ command
On 08.03.2023 00:02, David G. Johnston wrote: FWIW I've finally gotten to publishing my beta version of the Role Graph for PostgreSQL pseudo-extension I'd been talking about: https://github.com/polobo/RoleGraphForPostgreSQL Great. So far I've looked very briefly, but it's interesting. I handle EMPTY explicitly in the Role Graph but as I noted somewhere in my comments, it really shouldn't be possible to leave the database in that state. Do we need to bug Robert on this directly or do you plan to have a go at it? I don't plan to do that. Right now I don't have enough time and experience. This requires an experienced developer. -- Pavel Luzanov Postgres Professional:https://postgrespro.com
Re: psql: Add role's membership options to the \du+ command
On 08.03.2023 05:31, David G. Johnston wrote: Moving the goal posts for this meta-command to >= 9.5 seems like it should be done as a separate patch and thread. The documentation presently states we are targeting 9.2 and newer. I missed the comment at the beginning of the file about version 9.2. I will return the version check for rolbypassrls. My suggestion for the docs is below. + + Shown within each row, in newline-separated format, are the memberships granted to + the role. The presentation includes both the name of the grantor + as well as the membership permissions (in an abbreviated format: + a for admin option, i for inherit option, + s for set option.) The word empty is printed in + the case that none of those permissions are granted. + See the linkend="sql-grant">GRANT command for their meaning. + + + If the form \dg+ is used the comment attached to the role is shown. Thanks. I will replace the description with this one. I would suggest tweaking the test output to include regress_du_admin and also to make regress_du_admin a CREATEROLE role with LOGIN. Ok. Thank you for review. I will definitely work on the new version, but unfortunately and with a high probability it will happen after March 20. -- Pavel Luzanov Postgres Professional:https://postgrespro.com
Re: psql: Add role's membership options to the \du+ command
On 03.03.2023 19:21, David G. Johnston wrote: I'd be fine with "pg_can_admin_role" being a newly created function that provides this true/false answer but it seems indisputable that today there is no core-provided means to answer the question "can one role get ADMIN rights on another role". Modifying \du to show this seems out-of-scope but the pg_has_role function already provides that question for INHERIT and SET so it is at least plausible to extend it to include ADMIN, even if the phrase "has role" seems a bit of a misnomer. I do cover this aspect with the Role Graph pseudo-extension but given the presence and ease-of-use of a boolean-returning function this seems like a natural addition. We've also survived quite long without it - this isn't a new concept in v16, just a bit refined. I must admit that I am slowly coming to the same conclusions that you have already outlined in previous messages. Indeed, adding ADMIN to pg_has_role looks logical. The function will show whether one role can manage another directly or indirectly (via SET ROLE). Adding ADMIN will lead to the question of naming other values. It is more reasonable to have INHERIT instead of USAGE. And it is not very clear whether (except for backward compatibility) a separate MEMBER value is needed at all. I wouldn't bother starting yet another thread in this area right now, this one can absorb some related changes as well as the subject line item. I agree. -- Pavel Luzanov Postgres Professional:https://postgrespro.com
Re: psql: Add role's membership options to the \du+ command
Hello, On 22.02.2023 00:34, David G. Johnston wrote: I didn't even know this function existed. But I see that it was changed in 3d14e171 with updated documentation: https://www.postgresql.org/docs/devel/functions-info.html#FUNCTIONS-INFO-ACCESS Maybe that's enough. I think that should probably have ADMIN as one of the options as well. Also curious what it reports for an empty membership. I've been experimenting for a few days and I want to admit that this is a very difficult and not obvious topic. I'll try to summarize what I think. 1. About ADMIN value for pg_has_role. Implementation of ADMIN value will be different from USAGE and SET. To be True, USAGE value requires the full chain of memberships to have INHERIT option. Similar with SET: the full chain of memberships must have SET option. But for ADMIN, only last member in the chain must have ADMIN option and all previous members must have INHERIT (to administer directly) or SET option (to switch to role, last in the chain). Therefore, it is not obvious to me that the function needs the ADMIN value. 2. pg_has_role function description starts with: Does user have privilege for role? - This is not exact: function works not only with users, but with NOLOGIN roles too. - Term "privilege": this term used for ACL columns, such usage may be confusing, especially after adding INHERIT and SET in addition to ADMIN option. 3. It is possible to grant membership with all three options turned off: grant a to b with admin false, inherit false, set false; But such membership is completely useless (if i didn't miss something). May be such grants must be prohibited. At least this may be documented in the GRANT command. 4. Since v16 it is possible to grant membership from one role to another several times with different grantors. And only grantor can revoke membership. - This is not documented anywhere. - Current behavior of \du command with duplicated roles in "Member of" column strongly confusing. This is one of the goals of the discussion patch. I think to write about this to pgsql-docs additionally to this topic. -- Pavel Luzanov Postgres Professional:https://postgrespro.com
Re: psql: Add role's membership options to the \du+ command
Next version (v3) addresses complains from cfbot. Changed only tests.
--
Pavel Luzanov
Postgres Professional: https://postgrespro.com
From 6db62993d4b7afbcbce3e63ce3fbe3946ec50cff Mon Sep 17 00:00:00 2001
From: Pavel Luzanov
Date: Wed, 1 Mar 2023 13:29:10 +0300
Subject: [PATCH v3] psql: \du shows memberships options
---
doc/src/sgml/ref/psql-ref.sgml | 12
src/bin/psql/describe.c| 45 ++--
src/test/regress/expected/psql.out | 48 ++
src/test/regress/sql/psql.sql | 29 ++
4 files changed, 118 insertions(+), 16 deletions(-)
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index dc6528dc11..c94a2287f0 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -1724,6 +1724,12 @@ INSERT INTO tbl1 VALUES ($1, $2) \bind 'first value' 'second value' \g
S modifier to include system roles.
If pattern is specified,
only those roles whose names match the pattern are listed.
+For each membership in the role, the membership options and
+the role that granted the membership are displayed.
+Оne-letter abbreviations are used for membership options:
+a admin option, i inherit option,
+s set option and empty if no one is set.
+See GRANT command for their meaning.
If the form \dg+ is used, additional information
is shown about each role; currently this adds the comment for each
role.
@@ -1966,6 +1972,12 @@ INSERT INTO tbl1 VALUES ($1, $2) \bind 'first value' 'second value' \g
S modifier to include system roles.
If pattern is specified,
only those roles whose names match the pattern are listed.
+For each membership in the role, the membership options and
+the role that granted the membership are displayed.
+Оne-letter abbreviations are used for membership options:
+a admin option, i inherit option,
+s set option and empty if no one is set.
+See GRANT command for their meaning.
If the form \du+ is used, additional information
is shown about each role; currently this adds the comment for each
role.
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index c8a0bb7b3a..27a8680ddf 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -3623,22 +3623,36 @@ describeRoles(const char *pattern, bool verbose, bool showSystem)
"SELECT r.rolname, r.rolsuper, r.rolinherit,\n"
" r.rolcreaterole, r.rolcreatedb, r.rolcanlogin,\n"
" r.rolconnlimit, r.rolvaliduntil,\n"
- " ARRAY(SELECT b.rolname\n"
- "FROM pg_catalog.pg_auth_members m\n"
- "JOIN pg_catalog.pg_roles b ON (m.roleid = b.oid)\n"
- "WHERE m.member = r.oid) as memberof");
+ " r.rolreplication, r.rolbypassrls,\n");
+
+ if (pset.sversion >= 16)
+ appendPQExpBufferStr(,
+ " (SELECT pg_catalog.string_agg(\n"
+ "pg_catalog.format('%I from %I (%s)',\n"
+ " b.rolname, m.grantor::pg_catalog.regrole::pg_catalog.text,\n"
+ " pg_catalog.regexp_replace(\n"
+ "pg_catalog.concat_ws(', ',\n"
+ " CASE WHEN m.admin_option THEN 'a' END,\n"
+ " CASE WHEN m.inherit_option THEN 'i' END,\n"
+ " CASE WHEN m.set_option THEN 's' END),\n"
+ " '^$', 'empty')\n"
+ "), E'\\n'\n"
+ "ORDER BY b.rolname, m.grantor::pg_catalog.regrole::pg_catalog.text)\n"
+ " FROM pg_catalog.pg_auth_members m\n"
+ " JOIN pg_catalog.pg_roles b ON (m.roleid = b.oid)\n"
+ " WHERE m.member = r.oid) as memberof");
+ else
+ appendPQExpBufferStr(,
+ " ARRAY(SELECT b.rolname\n"
+ "FROM pg_catalog.pg_auth_members m\n"
+ "JOIN pg_catalog.pg_roles b ON (m.roleid = b.oid)\n"
+ "WHERE m.member = r.oid) as memberof");
if (verbose)
{
appendPQExpBufferStr(, "\n, pg_catalog.shobj_description(r.oid, 'pg_authid') AS description");
ncols++;
}
- appendPQExpBufferStr(, "\n, r.rolreplication");
-
- if (pset.sversion >= 90500)
- {
- appendPQExpBufferStr(, "\n, r.rolbypassrls");
- }
appendPQExpBufferStr(, "\nFROM pg_catalog.pg_roles r\n");
@@ -3692,12 +3706,11 @@ describeRoles(const char *pattern, bool verbose, bool showSystem)
if (strcmp(PQgetvalue(res, i, 5), "t") != 0)
add_role_attribute(, _("Cannot login"));
- if (strcmp(PQgetvalue(res, i, (verbose ? 10 : 9)), "t") == 0)
+ if (strcmp(PQgetvalue(res,
Re: psql: Add role's membership options to the \du+ command
On 22.02.2023 00:34, David G. Johnston wrote:
This is the format I've gone for (more-or-less) in my RoleGraph view
(I'll be sharing it publicly in the near future).
bob from grantor (a, s, i) \n
adam from postgres (a, s, i) \n
emily from postgres (empty)
I think this is a good compromise.
Based upon prior comments going for something like the following is
undesirable: bob=asi/grantor
Agree. Membership options are not the ACL (although they have
similarities). Therefore, showing them as a ACL-like column will be
confusing.
So, please find attached the second version of the patch. It implements
suggested display format and small refactoring of existing code for \du
command.
As a non-native writer, I have doubts about the documentation part.
--
Pavel Luzanov
Postgres Professional:https://postgrespro.com
From 70489a605687a325287bad109e2741dd7d08cea3 Mon Sep 17 00:00:00 2001
From: Pavel Luzanov
Date: Mon, 27 Feb 2023 22:35:29 +0300
Subject: [PATCH v2] psql: \du shows membership options
---
doc/src/sgml/ref/psql-ref.sgml | 12
src/bin/psql/describe.c| 45 +++---
src/test/regress/expected/psql.out | 45 ++
src/test/regress/sql/psql.sql | 25 +
4 files changed, 111 insertions(+), 16 deletions(-)
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index dc6528dc11..c94a2287f0 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -1724,6 +1724,12 @@ INSERT INTO tbl1 VALUES ($1, $2) \bind 'first value' 'second value' \g
S modifier to include system roles.
If pattern is specified,
only those roles whose names match the pattern are listed.
+For each membership in the role, the membership options and
+the role that granted the membership are displayed.
+Оne-letter abbreviations are used for membership options:
+a admin option, i inherit option,
+s set option and empty if no one is set.
+See GRANT command for their meaning.
If the form \dg+ is used, additional information
is shown about each role; currently this adds the comment for each
role.
@@ -1966,6 +1972,12 @@ INSERT INTO tbl1 VALUES ($1, $2) \bind 'first value' 'second value' \g
S modifier to include system roles.
If pattern is specified,
only those roles whose names match the pattern are listed.
+For each membership in the role, the membership options and
+the role that granted the membership are displayed.
+Оne-letter abbreviations are used for membership options:
+a admin option, i inherit option,
+s set option and empty if no one is set.
+See GRANT command for their meaning.
If the form \du+ is used, additional information
is shown about each role; currently this adds the comment for each
role.
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index c8a0bb7b3a..27a8680ddf 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -3623,22 +3623,36 @@ describeRoles(const char *pattern, bool verbose, bool showSystem)
"SELECT r.rolname, r.rolsuper, r.rolinherit,\n"
" r.rolcreaterole, r.rolcreatedb, r.rolcanlogin,\n"
" r.rolconnlimit, r.rolvaliduntil,\n"
- " ARRAY(SELECT b.rolname\n"
- "FROM pg_catalog.pg_auth_members m\n"
- "JOIN pg_catalog.pg_roles b ON (m.roleid = b.oid)\n"
- "WHERE m.member = r.oid) as memberof");
+ " r.rolreplication, r.rolbypassrls,\n");
+
+ if (pset.sversion >= 16)
+ appendPQExpBufferStr(,
+ " (SELECT pg_catalog.string_agg(\n"
+ "pg_catalog.format('%I from %I (%s)',\n"
+ " b.rolname, m.grantor::pg_catalog.regrole::pg_catalog.text,\n"
+ " pg_catalog.regexp_replace(\n"
+ "pg_catalog.concat_ws(', ',\n"
+ " CASE WHEN m.admin_option THEN 'a' END,\n"
+ " CASE WHEN m.inherit_option THEN 'i' END,\n"
+ " CASE WHEN m.set_option THEN 's' END),\n"
+ " '^$', 'empty')\n"
+ "), E'\\n'\n"
+ "ORDER BY b.rolname, m.grantor::pg_catalog.regrole::pg_catalog.text)\n"
+ " FROM pg_catalog.pg_auth_members m\n"
+ " JOIN pg_catalog.pg_roles b ON (m.roleid = b.oid)\n"
+ " WHERE m.member = r.oid) as memberof");
+ else
+ appendPQExpBufferStr(,
+ " ARRAY(SELECT b.rolname\n"
+ "FROM pg_catalog.pg_auth_members m\n"
+ "JOIN pg_catalog.pg_roles b ON (m.roleid = b.oid)\n"
+ "WHERE m.member = r.oid) as memb
Re: psql: Add role's membership options to the \du+ command
On 17.02.2023 19:53, David G. Johnston wrote:
On Fri, Feb 17, 2023 at 4:02 AM Pavel Luzanov
wrote:
List of roles
Role name | Attributes | Member of
---++---
admin | Create
role | {bob,bob}
bob | | {}
postgres | Superuser, Create role, Create DB, Replication,
Bypass RLS | {}
First 'grant bob to admin' command issued immediately after
creating role bob by superuser(grantor=10). Second command issues
by admin role and set membership options SET and INHERIT.
If we don't ready to display membership options with \du+ may be
at least we must group records in 'Member of' column for \du command?
I agree that these views should GROUP BY roleid and use
bool_or(*_option) to produce their result.
Ok, I'll try in the next few days. But what presentation format to use?
1. bob(admin_option=t inherit_option=t set_option=f) -- it seems very long
2. bob(ai) -- short, but will it be clear?
3. something else?
Their purpose is to communicate the current effective state to the
user, not facilitate full inspection of the configuration, possibly to
aid in issuing GRANT and REVOKE commands.
This can help in issuing GRANT command, but not REVOKE. Revoking a
role's membership is now very similar to revoking privileges. Only the
role that granted membership can revoke that membership. So for REVOKE
you need to know who granted membership, but this information will not
be available after grouping.
One thing I found, and I plan to bring this up independently once I've
collected my thoughts, is that pg_has_role() uses the terminology
"USAGE" and "MEMBER" for "INHERIT" and "SET" respectively.
It's annoying that "member" has been overloaded here. And the choice
of USAGE just seems arbitrary (though I haven't researched it) given
the related syntax.
https://www.postgresql.org/docs/15/functions-info.html
I didn't even know this function existed. But I see that it was changed
in 3d14e171 with updated documentation:
https://www.postgresql.org/docs/devel/functions-info.html#FUNCTIONS-INFO-ACCESS
Maybe that's enough.
--
Pavel Luzanov
Postgres Professional:https://postgrespro.com
Re: psql: Add role's membership options to the \du+ command
Hello,
On the one hand, it would be nice to see the membership options with
the psql command.
After playing with cf5eb37c and e5b8a4c0 I think something must be made
with \du command.
postgres@demo(16.0)=# CREATE ROLE admin LOGIN CREATEROLE;
CREATE ROLE
postgres@demo(16.0)=# \c - admin
You are now connected to database "demo" as user "admin".
admin@demo(16.0)=> SET createrole_self_grant = 'SET, INHERIT';
SET
admin@demo(16.0)=> CREATE ROLE bob LOGIN;
CREATE ROLE
admin@demo(16.0)=> \du
List of roles
Role name | Attributes | Member of
---++---
admin | Create role
| {bob,bob}
bob | | {}
postgres | Superuser, Create role, Create DB, Replication, Bypass RLS
| {}
We see two bob roles in the 'Member of' column.Strange? But this is correct.
admin@demo(16.0)=> select roleid::regrole, member::regrole, * from
pg_auth_members where roleid = 'bob'::regrole;
roleid | member | oid | roleid | member | grantor | admin_option |
inherit_option | set_option
++---+++-+--++
bob | admin | 16713 | 16712 | 16711 | 10 | t |
f | f
bob | admin | 16714 | 16712 | 16711 | 16711 | f |
t | t
(2 rows)
First 'grant bob to admin' command issued immediately after creating
role bob by superuser(grantor=10). Second command issues by admin role
and set membership options SET and INHERIT.
If we don't ready to display membership options with \du+ may be at
least we must group records in 'Member of' column for \du command?
-
Pavel Luzanov
Re: psql: Add role's membership options to the \du+ command
On 16.02.2023 00:37, David G. Johnston wrote: I mean, either you accept the change in how this meta-command presents its information or you don't. Yes, that's the main issue of this patch. On the one hand, it would be nice to see the membership options with the psql command. On the other hand, I don't have an exact understanding of how best to do it. That's why I proposed a variant for discussion. It is quite possible that if there is no consensus, it would be better to leave it as is, and get information by queries to the system catalog. - Pavel Luzanov
Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf
On 15.02.2023 10:11, Michael Paquier wrote: Which comes down to blame me for both of them. My only intention was to make postgres better.I'm sorry you took it that way. So, please find attached a patch to close the gap the sample files, for both things, with descriptions of all the field values they can use. A short and precise description. Nothing to add.Next time I will try to offer a patch at once. - Pavel Luzanov
Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf
Hello, Playing with this patch, I did not see descriptive comments in pg_ident.conf. Does it make sense to reflect changes to the PG-USERNAME field in the pg_ident.conf.sample file? The same relates to the regexp supportin the DATABASE and USER fieldsof the pg_hba.conf.sample file(8fea8683). - Pavel Luzanov
Re: psql: Add role's membership options to the \du+ command
On 24.01.2023 20:16, David G. Johnston wrote: Yeah, I noticed the lack too, then went a bit too far afield with trying to compose a graph of the roles. I'm still working on that but at this point it probably won't be something I try to get committed to psql. Something more limited like this does need to be included. Glad to hear that you're working on it. I'm not too keen on the idea of converting the existing array into a newline separated string. I would try hard to make the modification here purely additional. I agree with all of your arguments. A couple of months I tried to find an acceptable variant in the background. But apparently tried not very hard )) In the end, the variant proposed in the patch seemed to me worthy to show and start a discussion. But I'm not sure that this is the best choice. Another thing I did with the graph was have both "member" and "memberof" columns in the output. In short, every grant row in pg_auth_members appears twice, once in each column, so the role being granted membership and the role into which membership is granted both have visibility when you filter on them. For the role graph I took this idea and extended out to an entire chain of roles (and also broke out user and group separately) but I think doing the direct-grant only here would still be a big improvement. It will be interesting to see the result. -- Pavel Luzanov
Re: v16 GRANT role TO role needs a multi-option setting capability
On 23.01.2023 23:09, David G. Johnston wrote:
GRANT role_name [, ...] TO role_specification [, ...]
[ WITH { ADMIN | INHERIT | SET } { OPTION | TRUE | FALSE } ]
[ GRANTED BY role_specification ]
It would be really nice to complete this new feature of INHERIT/SET
FALSE/TRUE with a multi-specification capability.
If I understand properly, the multi-specification capability is
supported in the form:
GRANT admin1, admin2 TO usr1, usr2
WITH ADMIN OPTION, SET FALSE, INHERIT TRUE;
But this doesn't seem to be reflected correctly in the documentation.
If I'm not mistaken, the current spec should be like this:
GRANT role_name [, ...] TO role_specification [, ...]
[ WITH [ { ADMIN | INHERIT | SET } { OPTION | TRUE | FALSE } ] [,
...] ]
[ GRANTED BY role_specification ]
By the way, there is suggestion to add role's membership options to the
\du+ command.[1]
[1]https://www.postgresql.org/message-id/flat/b9be2d0e-a9bc-0a30-492f-a4f68e4f7...@postgrespro.ru
--
Pavel Luzanov
Re: psql: Add role's membership options to the \du+ command
Added the patch to the open commitfest: https://commitfest.postgresql.org/42/4116/ Feel free to reject if it's not interesting. -- Pavel Luzanov
psql: Add role's membership options to the \du+ command
When you include one role in another, you can specify three options:
ADMIN, INHERIT (added in e3ce2de0) and SET (3d14e171).
For example.
CREATE ROLE alice LOGIN;
GRANT pg_read_all_settings TO alice WITH ADMIN TRUE, INHERIT TRUE, SET TRUE;
GRANT pg_stat_scan_tables TO alice WITH ADMIN FALSE, INHERIT FALSE, SET
FALSE;
GRANT pg_read_all_stats TO alice WITH ADMIN FALSE, INHERIT TRUE, SET FALSE;
For information about the options, you need to look in the pg_auth_members:
SELECT roleid::regrole, admin_option, inherit_option, set_option
FROM pg_auth_members
WHERE member = 'alice'::regrole;
roleid | admin_option | inherit_option | set_option
--+--++
pg_read_all_settings | t | t | t
pg_stat_scan_tables | f | f | f
pg_read_all_stats | f | t | f
(3 rows)
I think it would be useful to be able to get this information with a
psql command
like \du (and \dg). With proposed patch the \du command still only lists
the roles of which alice is a member:
\du alice
List of roles
Role name | Attributes | Member of
---++--
alice | |
{pg_read_all_settings,pg_read_all_stats,pg_stat_scan_tables}
But the \du+ command adds information about the selected ADMIN, INHERIT
and SET options:
\du+ alice
List of roles
Role name | Attributes | Member of
| Description
---++---+-
alice | | pg_read_all_settings WITH ADMIN, INHERIT, SET+|
| | pg_read_all_stats WITH INHERIT +|
| | pg_stat_scan_tables |
One more change. The roles in the "Member of" column are sorted for both
\du+ and \du for consistent output.
Any comments are welcome.
--
Pavel Luzanov
Postgres Professional: https://postgrespro.com
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 8a5285da9a..ef3e87fa32 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -1724,9 +1724,8 @@ INSERT INTO tbl1 VALUES ($1, $2) \bind 'first value' 'second value' \g
S modifier to include system roles.
If pattern is specified,
only those roles whose names match the pattern are listed.
-If the form \dg+ is used, additional information
-is shown about each role; currently this adds the comment for each
-role.
+If the form \dg+ is used, each role is listed
+with its associated description and options for each role's membership.
@@ -1964,9 +1963,8 @@ INSERT INTO tbl1 VALUES ($1, $2) \bind 'first value' 'second value' \g
S modifier to include system roles.
If pattern is specified,
only those roles whose names match the pattern are listed.
-If the form \du+ is used, additional information
-is shown about each role; currently this adds the comment for each
-role.
+If the form \du+ is used, each role is listed
+with its associated description and options for each role's membership.
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index df166365e8..c7b10f96f3 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -3624,11 +3624,29 @@ describeRoles(const char *pattern, bool verbose, bool showSystem)
printfPQExpBuffer(,
"SELECT r.rolname, r.rolsuper, r.rolinherit,\n"
" r.rolcreaterole, r.rolcreatedb, r.rolcanlogin,\n"
- " r.rolconnlimit, r.rolvaliduntil,\n"
- " ARRAY(SELECT b.rolname\n"
- "FROM pg_catalog.pg_auth_members m\n"
- "JOIN pg_catalog.pg_roles b ON (m.roleid = b.oid)\n"
- "WHERE m.member = r.oid) as memberof");
+ " r.rolconnlimit, r.rolvaliduntil,\n");
+
+ if (verbose)
+ {
+ appendPQExpBufferStr(,
+ " (SELECT string_agg (quote_ident(b.rolname) || regexp_replace(\n"
+ " CASE WHEN m.admin_option THEN ', ADMIN' ELSE '' END ||\n"
+ " CASE WHEN m.inherit_option THEN ', INHERIT' ELSE '' END ||\n"
+ " CASE WHEN m.set_option THEN ', SET' ELSE '' END, \n"
+ " '^,', ' WITH', 1), E'\\n' ORDER BY b.rolname)\n"
+ " FROM pg_catalog.pg_auth_members m\n"
+ " JOIN pg_catalog.pg_roles b ON (m.roleid = b.oid)\n"
+ " WHERE m.member = r.oid) as memberof");
+ }
+ else
+ {
+ appendPQExpBufferStr(,
+ " ARRAY(SELECT b.rolname\n"
+ &qu
Re: allow granting CLUSTER, REFRESH MATERIALIZED VIEW, and REINDEX
On 15.12.2022 03:27, Nathan Bossart wrote: Another option I'm looking at is skipping the privilege checks when VACUUM recurses to a TOAST table. This won't allow you to VACUUM the TOAST table directly, but it would at least address the originally-reported issue This approach can be implemented for partitioned tables too. Skipping the privilege checks when VACUUM/ANALYZE recurses to partitions. I don't know if this is good enough. At least it's better than before. It seems like ideally you should be able to VACUUM a TOAST table directly if you have MAINTAIN on its main relation. I agree, that would be ideally. -- Pavel Luzanov Postgres Professional: https://postgrespro.com
Re: allow granting CLUSTER, REFRESH MATERIALIZED VIEW, and REINDEX
On 15.12.2022 03:18, Jeff Davis wrote: Right, that's what I had in mind: a user is only granted operations on the partitioned table, not the partitions. It's all clear now. There's definitely a problem with this patch and partitioning, because REINDEX affects the partitions, CLUSTER is a no-op, and VACUUM/ANALYZE skip them. I think the approach that Nathan implemented [1] for TOAST tables in the latest version can be used for partitioned tables as well. Skipping the privilege check for partitions while working with a partitioned table. In that case we would get exactly the same behavior as for INSERT, SELECT, etc privileges - the MAINTAIN privilege would work for the whole partitioned table, but not for individual partitions. [1] https://www.postgresql.org/message-id/20221215002705.GA889413%40nathanxps13 -- Pavel Luzanov Postgres Professional: https://postgrespro.com
Re: allow granting CLUSTER, REFRESH MATERIALIZED VIEW, and REINDEX
On 14.12.2022 22:46, Jeff Davis wrote: The behavior is that MAINTAIN privileges on the partitioned table does not imply MAINTAIN privileges on the partitions. I believe that's fine and it's consistent with other privileges on partitioned tables, such as SELECT and INSERT. Sorry, I may have missed something, but here's what I see: postgres@postgres(16.0)=# create table p (id int) partition by list (id); postgres@postgres(16.0)=# create table p1 partition of p for values in (1); postgres@postgres(16.0)=# create table p2 partition of p for values in (2); postgres@postgres(16.0)=# grant select, insert, maintain on p to alice ; postgres@postgres(16.0)=# \c - alice You are now connected to database "postgres" as user "alice". alice@postgres(16.0)=> insert into p values (1); INSERT 0 1 alice@postgres(16.0)=> select * from p; id 1 (1 row) alice@postgres(16.0)=> vacuum p; WARNING: permission denied to vacuum "p1", skipping it WARNING: permission denied to vacuum "p2", skipping it VACUUM -- Pavel Luzanov Postgres Professional: https://postgrespro.com
Re: allow granting CLUSTER, REFRESH MATERIALIZED VIEW, and REINDEX
After a fresh install, including the patch for \dpS [1], I found that granting MAINTAIN privilege does not allow the TOAST table to be vacuumed. postgres@postgres(16.0)=# GRANT MAINTAIN ON pg_type TO alice; GRANT postgres@postgres(16.0)=# \c - alice You are now connected to database "postgres" as user "alice". alice@postgres(16.0)=> \dpS pg_type Access privileges Schema | Name | Type | Access privileges | Column privileges | Policies +-+---++---+-- pg_catalog | pg_type | table | postgres=arwdDxtm/postgres+| | | | | =r/postgres +| | | | | alice=m/postgres | | (1 row) So, the patch for \dpS works as expected and can be committed. alice@postgres(16.0)=> VACUUM pg_type; WARNING: permission denied to vacuum "pg_toast_1247", skipping it VACUUM [1] https://www.postgresql.org/message-id/20221206193606.GB3078082%40nathanxps13 -- Pavel Luzanov Postgres Professional: https://postgrespro.com
Re: fix and document CLUSTER privileges
On 08.12.2022 01:39, Nathan Bossart wrote: It was also noted elsewhere [1] that the privilege requirements for CLUSTER are not documented. The attached patch adds such documentation. [1] https://postgr.es/m/661148f4-c7f1-dec1-2bc8-29f3bd58e242%40postgrespro.ru Thanks for the patch. It correctly states the existing behavior. But perhaps we should wait for the decision in discussion [1] (link above), since this decision may affect the documentation on the CLUSTER command. -- Pavel Luzanov Postgres Professional: https://postgrespro.com
Re: add \dpS to psql
On 08.12.2022 07:48, Isaac Morland wrote: If we implement MAINTAIN to control access to VACUUM, ANALYZE, REFRESH, CLUSTER, and REINDEX, we will cover everything that I can find that has seriously discussed on this list I like this approach with MAINTAIN privilege. I'm trying to find any disadvantages ... and I can't. For the complete picture, I tried to see what other actions with the table could *potentially* be considered as maintenance. Here is the list: - create|alter|drop on extended statistics objects - alter table|index alter column set statistics - alter table|index [re]set (storage_parameters) - alter table|index set tablespace - alter table alter column set storage|compression - any actions with the TOAST table that can be performed separately from the main table I have to admit that the discussion has moved away from the $subject. -- Pavel Luzanov Postgres Professional: https://postgrespro.com
Re: add \dpS to psql
On 06.12.2022 22:36, Nathan Bossart wrote: As discussed elsewhere [0], \dp doesn't show privileges on system objects, and this behavior is not mentioned in the docs. I've attached a small patch that adds support for the S modifier (i.e., \dpS) and the adjusts the docs. Thoughts? [0] https://postgr.es/m/a2382acd-e465-85b2-9d8e-f9ed1a5a66e9%40postgrespro.ru A few words in support of this patch, since I was the initiator of the discussion. Before VACUUM, ANALYZE privileges, there was no such question. Why check privileges on system catalog objects? But now it doesn't. It is now possible to grant privileges on system tables, so it should be possible to see privileges with psql commands. However, the \dp command does not support the S modifier, which is inconsistent. Furthermore. The VACUUM privilege allows you to also execute VACUUM FULL. VACUUM and VACUUM FULL are commands with similar names, but work completely differently. It may be worth clarifying on this page: https://www.postgresql.org/docs/devel/ddl-priv.html Something like: Allows VACUUM on a relation, including VACUUM FULL. But that's not all. There is a very similar command to VACUUM FULL with a different name - CLUSTER. The VACUUM privilege does not apply to the CLUSTER command. This is probably correct. However, the documentation for the CLUSTER command does not say who can perform this command. I think it would be correct to add a sentence to the Notes section (https://www.postgresql.org/docs/devel/sql-cluster.html) similar to the one in the VACUUM documentation: "To cluster a table, one must ordinarily be the table's owner or a superuser." Ready to participate, if it seems reasonable. -- Pavel Luzanov Postgres Professional: https://postgrespro.com
Re: predefined role(s) for VACUUM and ANALYZE
On 06.12.2022 03:04, Nathan Bossart wrote: I wonder why \dpS wasn't added. I wrote up a patch to add it and the corresponding documentation that other meta-commands already have. Yes, \dpS command and clarification in the documentation is exactly what is needed. -- Pavel Luzanov Postgres Professional: https://postgrespro.com
Re: predefined role(s) for VACUUM and ANALYZE
Hello,
While looking into the new feature, I found the following situation with
the \dp command displaying privileges on the system tables:
GRANT VACUUM, ANALYZE ON TABLE pg_type TO alice;
SELECT relacl FROM pg_class WHERE oid = 'pg_type'::regclass;
relacl
-
{=r/postgres,postgres=arwdDxtvz/postgres,alice=vz/postgres}
(1 row)
But the \dp command does not show the granted privileges:
\dp pg_type
Access privileges
Schema | Name | Type | Access privileges | Column privileges | Policies
+--+--+---+---+--
(0 rows)
The comment in src/bin/psql/describe.c explains the situation:
/*
* Unless a schema pattern is specified, we suppress system and temp
* tables, since they normally aren't very interesting from a
permissions
* point of view. You can see 'em by explicit request though, eg
with \z
* pg_catalog.*
*/
So to see the privileges you have to explicitly specify the schema name:
\dp pg_catalog.pg_type
Access privileges
Schema | Name | Type | Access privileges | Column
privileges | Policies
+-+---+-+---+--
pg_catalog | pg_type | table | =r/postgres +| |
| | |
postgres=arwdDxtvz/postgres+| |
| | | alice=vz/postgres | |
(1 row)
But perhaps this behavior should be reviewed or at least documented?
-
Pavel Luzanov
Re: Add proper planner support for ORDER BY / DISTINCT aggregates
On 08.11.2022 04:31, David Rowley wrote: I've been playing around with the attached patch which does: 1. Adjusts add_paths_to_grouping_rel so that we don't add a Sort path when we can add an Incremental Sort path instead. This removes quite a few redundant lines of code. 2. Removes the * 1.5 fuzz-factor in cost_incremental_sort() 3. Does various other code tidy stuff in cost_incremental_sort(). 4. Removes the test from incremental_sort.sql that was ensuring the inferior Sort -> Sort plan was being used instead of the superior Sort -> Incremental Sort plan. I can confirm that with this patch, the plan with incremental sorting beats the others. Here are the test results with my previous example. Script: create table t (a text, b text, c text); insert into t (a,b,c) select x,y,x from generate_series(1,100) as x, generate_series(1,1) y; create index on t (a); vacuum analyze t; reset all; explain (settings, analyze) select a, array_agg(c order by c) from t group by a; \echo set enable_incremental_sort=off; set enable_incremental_sort=off; explain (settings, analyze) select a, array_agg(c order by c) from t group by a; \echo set enable_seqscan=off; set enable_seqscan=off; explain (settings, analyze) select a, array_agg(c order by c) from t group by a; Script output: CREATE TABLE INSERT 0 100 CREATE INDEX VACUUM RESET QUERY PLAN - GroupAggregate (cost=957.60..113221.24 rows=100 width=34) (actual time=6.088..381.777 rows=100 loops=1) Group Key: a -> Incremental Sort (cost=957.60..108219.99 rows=100 width=4) (actual time=2.387..272.332 rows=100 loops=1) Sort Key: a, c Presorted Key: a Full-sort Groups: 100 Sort Method: quicksort Average Memory: 27kB Peak Memory: 27kB Pre-sorted Groups: 100 Sort Method: quicksort Average Memory: 697kB Peak Memory: 697kB -> Index Scan using t_a_idx on t (cost=0.42..29279.42 rows=100 width=4) (actual time=0.024..128.083 rows=100 loops=1) Planning Time: 0.070 ms Execution Time: 381.815 ms (10 rows) set enable_incremental_sort=off; SET QUERY PLAN GroupAggregate (cost=128728.34..136229.59 rows=100 width=34) (actual time=234.044..495.537 rows=100 loops=1) Group Key: a -> Sort (cost=128728.34..131228.34 rows=100 width=4) (actual time=231.172..383.393 rows=100 loops=1) Sort Key: a, c Sort Method: external merge Disk: 15600kB -> Seq Scan on t (cost=0.00..15396.00 rows=100 width=4) (actual time=0.005..78.189 rows=100 loops=1) Settings: enable_incremental_sort = 'off' Planning Time: 0.041 ms Execution Time: 497.230 ms (9 rows) set enable_seqscan=off; SET QUERY PLAN - GroupAggregate (cost=142611.77..150113.02 rows=100 width=34) (actual time=262.250..527.260 rows=100 loops=1) Group Key: a -> Sort (cost=142611.77..145111.77 rows=100 width=4) (actual time=259.551..417.154 rows=100 loops=1) Sort Key: a, c Sort Method: external merge Disk: 15560kB -> Index Scan using t_a_idx on t (cost=0.42..29279.42 rows=100 width=4) (actual time=0.012..121.995 rows=100 loops=1) Settings: enable_incremental_sort = 'off', enable_seqscan = 'off' Planning Time: 0.041 ms Execution Time: 528.950 ms (9 rows) -- Pavel Luzanov Postgres Professional: https://postgrespro.com The Russian Postgres Company
Re: Add proper planner support for ORDER BY / DISTINCT aggregates
On 07.11.2022 20:30, Ronan Dunklau wrote: What I meant here is that disabling seqscans, the planner still chooses a full sort over a partial sort. The underlying index is the same, it is just a matter of choosing a Sort node over an IncrementalSort node. This, I think, is wrong: I can't see how it could be worse to use an incrementalsort in that case. I finally get your point. And I agree with you. Maybe the original costing code for incremental sort was a bit too pessimistic. In this query, incremental sorting lost just a little bit in cost: 164468.95 vs 162504.23. QUERY PLAN --- GroupAggregate (cost=155002.98..162504.23 rows=100 width=34) (actual time=296.591..568.270 rows=100 loops=1) Group Key: a -> Sort (cost=155002.98..157502.98 rows=100 width=4) (actual time=293.810..454.170 rows=100 loops=1) Sort Key: a, c Sort Method: external merge Disk: 15560kB -> Index Scan using t_a_b_idx on t (cost=0.42..41670.64 rows=100 width=4) (actual time=0.021..156.441 rows=100 loops=1) Settings: enable_seqscan = 'off' Planning Time: 0.074 ms Execution Time: 569.957 ms (9 rows) set enable_sort=off; SET QUERY PLAN --- GroupAggregate (cost=1457.58..164468.95 rows=100 width=34) (actual time=6.623..408.833 rows=100 loops=1) Group Key: a -> Incremental Sort (cost=1457.58..159467.70 rows=100 width=4) (actual time=2.652..298.530 rows=100 loops=1) Sort Key: a, c Presorted Key: a Full-sort Groups: 100 Sort Method: quicksort Average Memory: 27kB Peak Memory: 27kB Pre-sorted Groups: 100 Sort Method: quicksort Average Memory: 697kB Peak Memory: 697kB -> Index Scan using t_a_b_idx on t (cost=0.42..41670.64 rows=100 width=4) (actual time=0.011..155.260 rows=100 loops=1) Settings: enable_seqscan = 'off', enable_sort = 'off' Planning Time: 0.044 ms Execution Time: 408.867 ms -- Pavel Luzanov Postgres Professional: https://postgrespro.com The Russian Postgres Company
Re: Add proper planner support for ORDER BY / DISTINCT aggregates
Hi, On 07.11.2022 17:53, Ronan Dunklau wrote: In your exact use case, the combo incremental-sort + Index scan is evaluated to cost more than doing a full sort + seqscan. I think we can trace that back to incremental sort being pessimistic about it's performance. If you try the same query, but with set enable_seqscan = off, you will get a full sort over an index scan: QUERY PLAN - GroupAggregate (cost=154944.94..162446.19 rows=100 width=34) Group Key: a -> Sort (cost=154944.94..157444.94 rows=100 width=4) Sort Key: a, c -> Index Scan using t_a_b_idx on t (cost=0.42..41612.60 rows=100 width=4) (5 rows) You are right. By disabling seq scan, we can get this plan. But compare it with the plan in v15: postgres@db(15.0)=# explain select a, array_agg(c order by c) from t group by a; QUERY PLAN --- GroupAggregate (cost=0.42..46667.56 rows=100 width=34) Group Key: a -> Index Scan using t_a_b_idx on t (cost=0.42..41666.31 rows=100 width=4) (3 rows) The total plan cost in v16 is ~4 times higher, while the index scan cost remains the same. I can't see why an incrementalsort could be more expensive than a full sort, using the same presorted path. The only reason I can see is the number of buffers to read. In the plan with incremental sort we read the whole index, ~19 buffers. And the plan with seq scan only required ~5000 (I think due to buffer ring optimization). Perhaps this behavior is preferable. Especially when many concurrent queries are running. The less buffer cache is busy, the better. But in single-user mode this query is slower. -- Pavel Luzanov Postgres Professional: https://postgrespro.com The Russian Postgres Company
Re: Add proper planner support for ORDER BY / DISTINCT aggregates
Hello, While playing with the patch I found a situation where the performance may be degraded compared to previous versions. The test case below. If you create a proper index for the query (a,c), version 16 wins. On my notebook, the query runs ~50% faster. But if there is no index (a,c), but only (a,b), in previous versions the planner uses it, but with this patch a full table scan is selected. create table t (a text, b text, c text); insert into t (a,b,c) select x,y,x from generate_series(1,100) as x, generate_series(1,1) y; create index on t (a,b); vacuum analyze t; explain (analyze, buffers) select a, array_agg(c order by c) from t group by a; v 14.5 QUERY PLAN - GroupAggregate (cost=0.42..46587.76 rows=100 width=34) (actual time=3.077..351.526 rows=100 loops=1) Group Key: a Buffers: shared hit=193387 read=2745 -> Index Scan using t_a_b_idx on t (cost=0.42..41586.51 rows=100 width=4) (actual time=0.014..155.095 rows=100 loops=1) Buffers: shared hit=193387 read=2745 Planning: Buffers: shared hit=9 Planning Time: 0.059 ms Execution Time: 351.581 ms (9 rows) v 16 QUERY PLAN GroupAggregate (cost=128728.34..136229.59 rows=100 width=34) (actual time=262.930..572.915 rows=100 loops=1) Group Key: a Buffers: shared hit=5396, temp read=1950 written=1964 -> Sort (cost=128728.34..131228.34 rows=100 width=4) (actual time=259.423..434.105 rows=100 loops=1) Sort Key: a, c Sort Method: external merge Disk: 15600kB Buffers: shared hit=5396, temp read=1950 written=1964 -> Seq Scan on t (cost=0.00..15396.00 rows=100 width=4) (actual time=0.005..84.104 rows=100 loops=1) Buffers: shared hit=5396 Planning: Buffers: shared hit=9 Planning Time: 0.055 ms Execution Time: 575.146 ms (13 rows) -- Pavel Luzanov Postgres Professional: https://postgrespro.com The Russian Postgres Company
Re: replacing role-level NOINHERIT with a grant-level option
Hello,
Thanks for reviewing. Committed.
Let me return to this topic.
After looking at the changes in this patch, I have a suggestion.
The inheritance option for role memberships is important information to
know if the role privileges will be available automatically or if a
switch with a SET ROLE command is required. However, this information
cannot be obtained with psql commands, specifically \du or \dg.
Previously, you could see the inherit attribute of the role (its absence
is shown with \du). Now you have to look in the pg_auth_members system
catalog.
My suggestion is to add information about pg_auth_members.inherit_option
to the output of \du (\dg).
If so, we can also add information about pg_auth_members.admin_option.
Right now this information is not available in psql command output either.
I thought about how exactly to represent these options in the output
\du, but did not find a single solution. Considered the following choices:
1.
Add \du+ command and for each membership in the role add values of two
options. I haven't done a patch yet, but you can imagine the changes
like this:
CREATE ROLE alice LOGIN IN ROLE pg_read_all_data;
\du+ alice
List of roles
Role name | Attributes | Member of
---++
alice | | {pg_read_all_data(admin=f inherit=t)}
It looks long, but for \du+ it's not a problem.
2.
I assume that the default values for these options will rarely change.
In that case, we can do without \du+ and output only the changed values
directly in the \du command.
GRANT pg_read_all_data TO alice WITH INHERIT FALSE;
2a.
\du alice
List of roles
Role name | Attributes | Member of
---++
alice | | {pg_read_all_data(inherit=f)}
2b.
Similar to GRANT OPTION, we can use symbols instead of long text
(inherit=f) for options. For example, for the ADMIN OPTION we can use
"*" (again similar to GRANT OPTION), and for the missing INHERIT option
something else, such as "-":
GRANT pg_read_all_data TO alice WITH ADMIN TRUE;
GRANT pg_write_all_data TO alice WITH INHERIT FALSE;
\du alice
List of roles
Role name | Attributes | Member of
---++
alice | | {pg_read_all_data*-,pg_write_all_data-}
But I think choices 2a and 2b are too complicated to understand.
Especially because the two options have different default values. And
even more. The default value for the INHERIT option depends on the value
of the INHERIT attribute for the role.
So I like the first choice with \du+ better.
But perhaps there are other choices as well.
If it's interesting, I'm ready to open a new thread (the commitfest
entry for this topic is now closed) and prepare a patch.
--
Pavel Luzanov
Re: How about a psql backslash command to show GUCs?
On 06.04.2022 20:48, Tom Lane wrote: However, I very often find myself resorting to the much more tedious select * from pg_settings where name like '%foo%'; In the discussion about adding privileges for GUCs [1], there was a proposal to add a new psql backslash command to show GUCs, which could reduce this problem to something like \dcp *foo* +1, great idea. Right now I use the psql :show variable in my .psqlrc for this purpose: =# \echo :show SELECT name, current_setting(name) AS value, context FROM pg_settings\g (format=wrapped columns=100) | grep =# :show autovacuum autovacuum | on | sighup autovacuum_analyze_scale_factor | 0.1 | sighup autovacuum_analyze_threshold | 50 | sighup autovacuum_freeze_max_age | 2 | postmaster autovacuum_max_workers | 3 | postmaster autovacuum_multixact_freeze_max_age | 4 | postmaster autovacuum_naptime | 1min | sighup autovacuum_vacuum_cost_delay | 2ms | sighup autovacuum_vacuum_cost_limit | -1 | sighup autovacuum_vacuum_scale_factor | 0.2 | sighup autovacuum_vacuum_threshold | 50 | sighup autovacuum_work_mem | -1 | sighup log_autovacuum_min_duration | -1 | sighup As for the name, I can't think of a better candidate. Any of the previously suggested list of \dconf, \dguc, \dG, \dcp is fine. -- Pavel Luzanov Postgres Professional: https://postgrespro.com The Russian Postgres Company
Re: psql: \dl+ to list large objects privileges
On 06.01.2022 21:13, Tom Lane wrote: I made the same changes to two files in the 'expected' directory: largeobject.out and largeobject_1.out. Although I must say that I still can't understand why more than one file with expected result is used for some tests. Because the output sometimes varies across platforms. IIRC, the case where largeobject_1.out is needed is for Windows, where the file that gets inserted into one of the LOs might contain CR/LF not just LF newlines, so the LO contents look different. So simple. Thanks for the explanation. Pushed with some minor editorialization. Thanks! -- Pavel Luzanov Postgres Professional: https://postgrespro.com The Russian Postgres Company
