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 <p.luza...@postgrespro.ru>
Date: Sun, 21 Jan 2024 23:44:33 +0300
Subject: [PATCH v3] psql: Rethinking of \du command

The Attributes column includes only the enabled logical attributes.
The attribute names correspond to the keywords of the CREATE ROLE
command. The attributes are listed in the same order as in
the documentation. The "Connection limit" and "Valid until" attributes
are placed in separate columns. The "Password?" column has been added.
---
 src/backend/catalog/system_views.sql |   6 +-
 src/bin/psql/describe.c              | 149 ++++++++-------------------
 2 files changed, 45 insertions(+), 110 deletions(-)

diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index e43e36f5ac..5f67e0f112 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 37f9516320..f6c70f0636 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);
@@ -3654,34 +3653,50 @@ describeRoles(const char *pattern, bool verbose, bool showSystem)
 {
 	PQExpBufferData buf;
 	PGresult   *res;
-	printTableContent cont;
-	printTableOpt myopt = pset.popt.topt;
-	int			ncols = 2;
-	int			nrows = 0;
-	int			i;
-	int			conns;
-	const char	align = 'l';
-	char	  **attr;
-
-	myopt.default_footer = false;
+	printQueryOpt myopt = pset.popt;
 
 	initPQExpBuffer(&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"
+					  "  pg_catalog.concat_ws(' ',\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.rolcanlogin THEN '%s' END,\n"
+					  "    CASE WHEN r.rolreplication THEN '%s' END",
+					  gettext_noop("Role name"),
+					  gettext_noop("SUPERUSER"),
+					  gettext_noop("CREATEDB"),
+					  gettext_noop("CREATEROLE"),
+					  gettext_noop("INHERIT"),
+					  gettext_noop("LOGIN"),
+					  gettext_noop("REPLICATION"));
 
 	if (pset.sversion >= 90500)
+		appendPQExpBuffer(&buf,
+						  ",\n    CASE WHEN r.rolbypassrls THEN '%s' END",
+						  gettext_noop("BYPASSRLS"));
+
+	appendPQExpBuffer(&buf, "\n  ) AS \"%s\"", gettext_noop("Attributes"));
+
+	if (pset.sversion >= 170000)
+		appendPQExpBuffer(&buf,
+						  ",\n  CASE WHEN r.rolpassword IS NULL THEN '%s' ELSE '%s' END AS \"%s\"",
+					  gettext_noop("no"), gettext_noop("yes"),
+					  gettext_noop("Password?"));
+
+	appendPQExpBuffer(&buf,
+					  ",\n  r.rolvaliduntil AS \"%s\",\n"
+					  "  r.rolconnlimit AS \"%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");
@@ -3700,99 +3715,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
  */
-- 
2.34.1

Reply via email to