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 <p.luza...@postgrespro.ru>
Date: Thu, 6 Jun 2024 23:48:32 +0300
Subject: [PATCH v8] psql: Rethinking of \du command
Cnanges in the \du command
- "Login", "Connection limit" and "Valid until" attributes are placed
in separate columns.
to understand that there is no limit.
- The "Attributes" column includes only the enabled logical attributes.
- The attribute names correspond to the keywords of the CREATE ROLE command.
- The attributes are listed in the same order as in the documentation.
- Value -1 for "Connection limit" replaced by NULL to make it easier
- General refactoring of describeRoles function in describe.c.
---
src/bin/psql/describe.c | 146 ++++++++---------------------
src/test/regress/expected/psql.out | 40 +++++---
src/test/regress/sql/psql.sql | 12 ++-
3 files changed, 72 insertions(+), 126 deletions(-)
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index f67bf0b892..31cc40b38f 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -36,7 +36,6 @@ static bool describeOneTableDetails(const char *schemaname,
bool verbose);
static void add_tablespace_footer(printTableContent *const cont, char relkind,
Oid tablespace, const bool newline);
-static void add_role_attribute(PQExpBuffer buf, const char *const str);
static bool listTSParsersVerbose(const char *pattern);
static bool describeOneTSParser(const char *oid, const char *nspname,
const char *prsname);
@@ -3615,34 +3614,47 @@ describeRoles(const char *pattern, bool verbose, bool showSystem)
{
PQExpBufferData buf;
PGresult *res;
- printTableContent cont;
- printTableOpt myopt = pset.popt.topt;
- int ncols = 2;
- int nrows = 0;
- int i;
- int conns;
- const char align = 'l';
- char **attr;
-
- myopt.default_footer = false;
+ printQueryOpt myopt = pset.popt;
initPQExpBuffer(&buf);
-
printfPQExpBuffer(&buf,
- "SELECT r.rolname, r.rolsuper, r.rolinherit,\n"
- " r.rolcreaterole, r.rolcreatedb, r.rolcanlogin,\n"
- " r.rolconnlimit, r.rolvaliduntil");
-
- if (verbose)
- {
- appendPQExpBufferStr(&buf, "\n, pg_catalog.shobj_description(r.oid, 'pg_authid') AS description");
- ncols++;
- }
- appendPQExpBufferStr(&buf, "\n, r.rolreplication");
+ "SELECT r.rolname AS \"%s\",\n"
+ " CASE WHEN r.rolcanlogin THEN '%s' ELSE '%s' END \"%s\",\n"
+ " pg_catalog.concat_ws(E'\\n',\n"
+ " CASE WHEN r.rolsuper THEN '%s' END,\n"
+ " CASE WHEN r.rolcreatedb THEN '%s' END,\n"
+ " CASE WHEN r.rolcreaterole THEN '%s' END,\n"
+ " CASE WHEN r.rolinherit THEN '%s' END,\n"
+ " CASE WHEN r.rolreplication THEN '%s' END",
+ gettext_noop("Role name"),
+ gettext_noop("yes"), gettext_noop("no"),
+ gettext_noop("Login"),
+ gettext_noop("Superuser"),
+ gettext_noop("Create DB"),
+ gettext_noop("Create role"),
+ gettext_noop("Inherit"),
+ gettext_noop("Replication"));
if (pset.sversion >= 90500)
+ appendPQExpBuffer(&buf,
+ ",\n CASE WHEN r.rolbypassrls THEN '%s' END",
+ gettext_noop("Bypass RLS"));
+
+ appendPQExpBuffer(&buf, "\n ) AS \"%s\"", gettext_noop("Attributes"));
+
+ appendPQExpBuffer(&buf,
+ ",\n r.rolvaliduntil AS \"%s\",\n"
+ " CASE WHEN r.rolconnlimit = -1 THEN NULL\n"
+ " ELSE r.rolconnlimit\n"
+ " END \"%s\"",
+ gettext_noop("Valid until"),
+ gettext_noop("Connection limit"));
+
+ if (verbose)
{
- appendPQExpBufferStr(&buf, "\n, r.rolbypassrls");
+ appendPQExpBuffer(&buf,
+ ",\n pg_catalog.shobj_description(r.oid, 'pg_authid') AS \"%s\"",
+ gettext_noop("Description"));
}
appendPQExpBufferStr(&buf, "\nFROM pg_catalog.pg_roles r\n");
@@ -3661,99 +3673,19 @@ describeRoles(const char *pattern, bool verbose, bool showSystem)
appendPQExpBufferStr(&buf, "ORDER BY 1;");
res = PSQLexec(buf.data);
+ termPQExpBuffer(&buf);
if (!res)
return false;
- nrows = PQntuples(res);
- attr = pg_malloc0((nrows + 1) * sizeof(*attr));
-
- printTableInit(&cont, &myopt, _("List of roles"), ncols, nrows);
-
- printTableAddHeader(&cont, gettext_noop("Role name"), true, align);
- printTableAddHeader(&cont, gettext_noop("Attributes"), true, align);
-
- if (verbose)
- printTableAddHeader(&cont, gettext_noop("Description"), true, align);
-
- for (i = 0; i < nrows; i++)
- {
- printTableAddCell(&cont, PQgetvalue(res, i, 0), false, false);
-
- resetPQExpBuffer(&buf);
- if (strcmp(PQgetvalue(res, i, 1), "t") == 0)
- add_role_attribute(&buf, _("Superuser"));
-
- if (strcmp(PQgetvalue(res, i, 2), "t") != 0)
- add_role_attribute(&buf, _("No inheritance"));
-
- if (strcmp(PQgetvalue(res, i, 3), "t") == 0)
- add_role_attribute(&buf, _("Create role"));
-
- if (strcmp(PQgetvalue(res, i, 4), "t") == 0)
- add_role_attribute(&buf, _("Create DB"));
-
- if (strcmp(PQgetvalue(res, i, 5), "t") != 0)
- add_role_attribute(&buf, _("Cannot login"));
-
- if (strcmp(PQgetvalue(res, i, (verbose ? 9 : 8)), "t") == 0)
- add_role_attribute(&buf, _("Replication"));
-
- if (pset.sversion >= 90500)
- if (strcmp(PQgetvalue(res, i, (verbose ? 10 : 9)), "t") == 0)
- add_role_attribute(&buf, _("Bypass RLS"));
-
- conns = atoi(PQgetvalue(res, i, 6));
- if (conns >= 0)
- {
- if (buf.len > 0)
- appendPQExpBufferChar(&buf, '\n');
-
- if (conns == 0)
- appendPQExpBufferStr(&buf, _("No connections"));
- else
- appendPQExpBuffer(&buf, ngettext("%d connection",
- "%d connections",
- conns),
- conns);
- }
-
- if (strcmp(PQgetvalue(res, i, 7), "") != 0)
- {
- if (buf.len > 0)
- appendPQExpBufferChar(&buf, '\n');
- appendPQExpBufferStr(&buf, _("Password valid until "));
- appendPQExpBufferStr(&buf, PQgetvalue(res, i, 7));
- }
-
- attr[i] = pg_strdup(buf.data);
-
- printTableAddCell(&cont, attr[i], false, false);
-
- if (verbose)
- printTableAddCell(&cont, PQgetvalue(res, i, 8), false, false);
- }
- termPQExpBuffer(&buf);
-
- printTable(&cont, pset.queryFout, false, pset.logfile);
- printTableCleanup(&cont);
+ myopt.title = _("List of roles");
+ myopt.translate_header = true;
- for (i = 0; i < nrows; i++)
- free(attr[i]);
- free(attr);
+ printQuery(res, &myopt, pset.queryFout, false, pset.logfile);
PQclear(res);
return true;
}
-static void
-add_role_attribute(PQExpBuffer buf, const char *const str)
-{
- if (buf->len > 0)
- appendPQExpBufferStr(buf, ", ");
-
- appendPQExpBufferStr(buf, str);
-}
-
/*
* \drds
*/
diff --git a/src/test/regress/expected/psql.out b/src/test/regress/expected/psql.out
index 3bbe4c5f97..f5a3308d10 100644
--- a/src/test/regress/expected/psql.out
+++ b/src/test/regress/expected/psql.out
@@ -6201,9 +6201,10 @@ List of text search templates
(0 rows)
\dg "no.such.role"
- List of roles
- Role name | Attributes
------------+------------
+ List of roles
+ Role name | Login | Attributes | Valid until | Connection limit
+-----------+-------+------------+-------------+------------------
+(0 rows)
\dL "no.such.language"
List of languages
@@ -6633,10 +6634,11 @@ cross-database references are not implemented: "no.such.database"."no.such.schem
\dX "no.such.database"."no.such.schema"."no.such.extended.statistics"
cross-database references are not implemented: "no.such.database"."no.such.schema"."no.such.extended.statistics"
-- check \drg and \du
-CREATE ROLE regress_du_role0;
-CREATE ROLE regress_du_role1;
-CREATE ROLE regress_du_role2;
-CREATE ROLE regress_du_admin;
+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';
GRANT regress_du_role0 TO regress_du_admin WITH ADMIN TRUE;
GRANT regress_du_role1 TO regress_du_admin WITH ADMIN TRUE;
GRANT regress_du_role2 TO regress_du_admin WITH ADMIN TRUE;
@@ -6660,13 +6662,23 @@ GRANT regress_du_role0 TO regress_du_role2 WITH ADMIN FALSE, INHERIT FALSE, SET
regress_du_role2 | regress_du_role1 | ADMIN, SET | regress_du_admin
(7 rows)
-\du regress_du_role*
- List of roles
- Role name | Attributes
-------------------+--------------
- regress_du_role0 | Cannot login
- regress_du_role1 | Cannot login
- regress_du_role2 | Cannot login
+\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)
DROP ROLE regress_du_role0;
DROP ROLE regress_du_role1;
diff --git a/src/test/regress/sql/psql.sql b/src/test/regress/sql/psql.sql
index 3b3c6f6e29..10e510eaf7 100644
--- a/src/test/regress/sql/psql.sql
+++ b/src/test/regress/sql/psql.sql
@@ -1834,10 +1834,12 @@ DROP FUNCTION psql_error;
\dX "no.such.database"."no.such.schema"."no.such.extended.statistics"
-- check \drg and \du
-CREATE ROLE regress_du_role0;
-CREATE ROLE regress_du_role1;
-CREATE ROLE regress_du_role2;
-CREATE ROLE regress_du_admin;
+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';
GRANT regress_du_role0 TO regress_du_admin WITH ADMIN TRUE;
GRANT regress_du_role1 TO regress_du_admin WITH ADMIN TRUE;
@@ -1852,7 +1854,7 @@ GRANT regress_du_role0 TO regress_du_role1 WITH ADMIN FALSE, INHERIT FALSE, SET
GRANT regress_du_role0 TO regress_du_role2 WITH ADMIN FALSE, INHERIT FALSE, SET FALSE GRANTED BY regress_du_role2;
\drg regress_du_role*
-\du regress_du_role*
+\du+ regress_du*
DROP ROLE regress_du_role0;
DROP ROLE regress_du_role1;
--
2.34.1
