Improve default and empty privilege outputs in psql.

Default privileges are represented as NULL::aclitem[] in catalog ACL
columns, while revoking all privileges leaves an empty aclitem[].
These two cases used to produce identical output in psql meta-commands
like \dp.  Using something like "\pset null '(default)'" as a
workaround for spotting the difference did not work, because null
values were always displayed as empty strings by describe.c's
meta-commands.

This patch improves that with two changes:

1. Print "(none)" for empty privileges so that the user is able to
   distinguish them from default privileges, even without special
   workarounds.

2. Remove the special handling of null values in describe.c,
   so that "\pset null" is honored like everywhere else.
   (This affects all output from these commands, not only ACLs.)

The privileges shown by \dconfig+ and \ddp as well as the column
privileges shown by \dp are not affected by change #1, because the
respective aclitem[] is reset to NULL or deleted from the catalog
instead of leaving an empty array.

Erik Wienhold and Laurenz Albe

Discussion: https://postgr.es/m/1966228777.127452.1694979110595@office.mailbox.org

diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index 075ff329912628c6edeccb32ee6664f5cb0ed5c4..4490e82aa52f3c0e2039047ab57194933d7c0275 100644 (file)
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -1737,6 +1737,11 @@ ALTER TABLE products RENAME TO items;
    <primary>ACL</primary>
   </indexterm>
 
+  <indexterm zone="ddl-priv-default">
+   <primary>privilege</primary>
+   <secondary>default</secondary>
+  </indexterm>
+
   <para>
    When an object is created, it is assigned an owner. The
    owner is normally the role that executed the creation statement.
@@ -2049,7 +2054,7 @@ REVOKE ALL ON accounts FROM PUBLIC;
    reference page of the respective command.
   </para>
 
-  <para>
+  <para id="ddl-priv-default">
    PostgreSQL grants privileges on some types of objects to
    <literal>PUBLIC</literal> by default when the objects are created.
    No privileges are granted to <literal>PUBLIC</literal> by default on
@@ -2375,6 +2380,15 @@ GRANT SELECT (col1), UPDATE (col1) ON mytable TO miriam_rw;
    access privileges display.  A <literal>*</literal> will appear only when
    grant options have been explicitly granted to someone.
   </para>
+
+  <para>
+   The <quote>Access privileges</quote> column
+   shows <literal>(none)</literal> when the object's privileges entry is
+   non-null but empty.  This means that no privileges are granted at all,
+   even to the object's owner &mdash; a rare situation.  (The owner still
+   has implicit grant options in this case, and so could re-grant her own
+   privileges; but she has none at the moment.)
+  </para>
  </sect1>
 
  <sect1 id="ddl-rowsecurity">

diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index bac94a338cfbc497200f0cf960cbabce2dadaa33..5077e7b3581b48eea21ee78f92a0426dba78957d 100644 (file)
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -124,7 +124,6 @@ describeAggregates(const char *pattern, bool verbose, bool showSystem)
    if (!res)
        return false;
 
-   myopt.nullPrint = NULL;
    myopt.title = _("List of aggregate functions");
    myopt.translate_header = true;
 
@@ -197,7 +196,6 @@ describeAccessMethods(const char *pattern, bool verbose)
    if (!res)
        return false;
 
-   myopt.nullPrint = NULL;
    myopt.title = _("List of access methods");
    myopt.translate_header = true;
    myopt.translate_columns = translate_columns;
@@ -262,7 +260,6 @@ describeTablespaces(const char *pattern, bool verbose)
    if (!res)
        return false;
 
-   myopt.nullPrint = NULL;
    myopt.title = _("List of tablespaces");
    myopt.translate_header = true;
 
@@ -585,7 +582,6 @@ describeFunctions(const char *functypes, const char *func_pattern,
    if (!res)
        return false;
 
-   myopt.nullPrint = NULL;
    myopt.title = _("List of functions");
    myopt.translate_header = true;
    if (pset.sversion >= 90600)
@@ -702,7 +698,6 @@ describeTypes(const char *pattern, bool verbose, bool showSystem)
    if (!res)
        return false;
 
-   myopt.nullPrint = NULL;
    myopt.title = _("List of data types");
    myopt.translate_header = true;
 
@@ -893,7 +888,6 @@ describeOperators(const char *oper_pattern,
    if (!res)
        return false;
 
-   myopt.nullPrint = NULL;
    myopt.title = _("List of operators");
    myopt.translate_header = true;
 
@@ -995,7 +989,6 @@ listAllDbs(const char *pattern, bool verbose)
    if (!res)
        return false;
 
-   myopt.nullPrint = NULL;
    myopt.title = _("List of databases");
    myopt.translate_header = true;
 
@@ -1047,6 +1040,11 @@ permissionsList(const char *pattern, bool showSystem)
 
    printACLColumn(&buf, "c.relacl");
 
+   /*
+    * The formatting of attacl should match printACLColumn().  However, we
+    * need no special case for an empty attacl, because the backend always
+    * optimizes that back to NULL.
+    */
    appendPQExpBuffer(&buf,
                      ",\n  pg_catalog.array_to_string(ARRAY(\n"
                      "    SELECT attname || E':\\n  ' || pg_catalog.array_to_string(attacl, E'\\n  ')\n"
@@ -1146,7 +1144,6 @@ permissionsList(const char *pattern, bool showSystem)
    if (!res)
        goto error_return;
 
-   myopt.nullPrint = NULL;
    printfPQExpBuffer(&buf, _("Access privileges"));
    myopt.title = buf.data;
    myopt.translate_header = true;
@@ -1218,7 +1215,6 @@ listDefaultACLs(const char *pattern)
    if (!res)
        goto error_return;
 
-   myopt.nullPrint = NULL;
    printfPQExpBuffer(&buf, _("Default access privileges"));
    myopt.title = buf.data;
    myopt.translate_header = true;
@@ -1417,7 +1413,6 @@ objectDescription(const char *pattern, bool showSystem)
    if (!res)
        return false;
 
-   myopt.nullPrint = NULL;
    myopt.title = _("Object descriptions");
    myopt.translate_header = true;
    myopt.translate_columns = translate_columns;
@@ -3852,7 +3847,6 @@ listDbRoleSettings(const char *pattern, const char *pattern2)
    }
    else
    {
-       myopt.nullPrint = NULL;
        myopt.title = _("List of settings");
        myopt.translate_header = true;
 
@@ -3926,7 +3920,6 @@ describeRoleGrants(const char *pattern, bool showSystem)
    if (!res)
        return false;
 
-   myopt.nullPrint = NULL;
    myopt.title = _("List of role grants");
    myopt.translate_header = true;
 
@@ -4122,7 +4115,6 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
    }
    else
    {
-       myopt.nullPrint = NULL;
        myopt.title = _("List of relations");
        myopt.translate_header = true;
        myopt.translate_columns = translate_columns;
@@ -4332,7 +4324,6 @@ listPartitionedTables(const char *reltypes, const char *pattern, bool verbose)
    initPQExpBuffer(&title);
    appendPQExpBufferStr(&title, tabletitle);
 
-   myopt.nullPrint = NULL;
    myopt.title = title.data;
    myopt.translate_header = true;
    myopt.translate_columns = translate_columns;
@@ -4412,7 +4403,6 @@ listLanguages(const char *pattern, bool verbose, bool showSystem)
    if (!res)
        return false;
 
-   myopt.nullPrint = NULL;
    myopt.title = _("List of languages");
    myopt.translate_header = true;
 
@@ -4497,7 +4487,6 @@ listDomains(const char *pattern, bool verbose, bool showSystem)
    if (!res)
        return false;
 
-   myopt.nullPrint = NULL;
    myopt.title = _("List of domains");
    myopt.translate_header = true;
 
@@ -4576,7 +4565,6 @@ listConversions(const char *pattern, bool verbose, bool showSystem)
    if (!res)
        return false;
 
-   myopt.nullPrint = NULL;
    myopt.title = _("List of conversions");
    myopt.translate_header = true;
    myopt.translate_columns = translate_columns;
@@ -4644,7 +4632,6 @@ describeConfigurationParameters(const char *pattern, bool verbose,
    if (!res)
        return false;
 
-   myopt.nullPrint = NULL;
    if (pattern)
        myopt.title = _("List of configuration parameters");
    else
@@ -4726,7 +4713,6 @@ listEventTriggers(const char *pattern, bool verbose)
    if (!res)
        return false;
 
-   myopt.nullPrint = NULL;
    myopt.title = _("List of event triggers");
    myopt.translate_header = true;
    myopt.translate_columns = translate_columns;
@@ -4825,7 +4811,6 @@ listExtendedStats(const char *pattern)
    if (!res)
        return false;
 
-   myopt.nullPrint = NULL;
    myopt.title = _("List of extended statistics");
    myopt.translate_header = true;
 
@@ -4938,7 +4923,6 @@ listCasts(const char *pattern, bool verbose)
    if (!res)
        return false;
 
-   myopt.nullPrint = NULL;
    myopt.title = _("List of casts");
    myopt.translate_header = true;
    myopt.translate_columns = translate_columns;
@@ -5057,7 +5041,6 @@ listCollations(const char *pattern, bool verbose, bool showSystem)
    if (!res)
        return false;
 
-   myopt.nullPrint = NULL;
    myopt.title = _("List of collations");
    myopt.translate_header = true;
    myopt.translate_columns = translate_columns;
@@ -5119,7 +5102,6 @@ listSchemas(const char *pattern, bool verbose, bool showSystem)
    if (!res)
        goto error_return;
 
-   myopt.nullPrint = NULL;
    myopt.title = _("List of schemas");
    myopt.translate_header = true;
 
@@ -5236,7 +5218,6 @@ listTSParsers(const char *pattern, bool verbose)
    if (!res)
        return false;
 
-   myopt.nullPrint = NULL;
    myopt.title = _("List of text search parsers");
    myopt.translate_header = true;
 
@@ -5384,7 +5365,6 @@ describeOneTSParser(const char *oid, const char *nspname, const char *prsname)
    if (!res)
        return false;
 
-   myopt.nullPrint = NULL;
    initPQExpBuffer(&title);
    if (nspname)
        printfPQExpBuffer(&title, _("Text search parser \"%s.%s\""),
@@ -5421,7 +5401,6 @@ describeOneTSParser(const char *oid, const char *nspname, const char *prsname)
        return false;
    }
 
-   myopt.nullPrint = NULL;
    if (nspname)
        printfPQExpBuffer(&title, _("Token types for parser \"%s.%s\""),
                          nspname, prsname);
@@ -5497,7 +5476,6 @@ listTSDictionaries(const char *pattern, bool verbose)
    if (!res)
        return false;
 
-   myopt.nullPrint = NULL;
    myopt.title = _("List of text search dictionaries");
    myopt.translate_header = true;
 
@@ -5563,7 +5541,6 @@ listTSTemplates(const char *pattern, bool verbose)
    if (!res)
        return false;
 
-   myopt.nullPrint = NULL;
    myopt.title = _("List of text search templates");
    myopt.translate_header = true;
 
@@ -5618,7 +5595,6 @@ listTSConfigs(const char *pattern, bool verbose)
    if (!res)
        return false;
 
-   myopt.nullPrint = NULL;
    myopt.title = _("List of text search configurations");
    myopt.translate_header = true;
 
@@ -5764,7 +5740,6 @@ describeOneTSConfig(const char *oid, const char *nspname, const char *cfgname,
        appendPQExpBuffer(&title, _("\nParser: \"%s\""),
                          prsname);
 
-   myopt.nullPrint = NULL;
    myopt.title = title.data;
    myopt.footers = NULL;
    myopt.topt.default_footer = false;
@@ -5841,7 +5816,6 @@ listForeignDataWrappers(const char *pattern, bool verbose)
    if (!res)
        return false;
 
-   myopt.nullPrint = NULL;
    myopt.title = _("List of foreign-data wrappers");
    myopt.translate_header = true;
 
@@ -5918,7 +5892,6 @@ listForeignServers(const char *pattern, bool verbose)
    if (!res)
        return false;
 
-   myopt.nullPrint = NULL;
    myopt.title = _("List of foreign servers");
    myopt.translate_header = true;
 
@@ -5974,7 +5947,6 @@ listUserMappings(const char *pattern, bool verbose)
    if (!res)
        return false;
 
-   myopt.nullPrint = NULL;
    myopt.title = _("List of user mappings");
    myopt.translate_header = true;
 
@@ -6047,7 +6019,6 @@ listForeignTables(const char *pattern, bool verbose)
    if (!res)
        return false;
 
-   myopt.nullPrint = NULL;
    myopt.title = _("List of foreign tables");
    myopt.translate_header = true;
 
@@ -6099,7 +6070,6 @@ listExtensions(const char *pattern)
    if (!res)
        return false;
 
-   myopt.nullPrint = NULL;
    myopt.title = _("List of installed extensions");
    myopt.translate_header = true;
 
@@ -6203,7 +6173,6 @@ listOneExtensionContents(const char *extname, const char *oid)
    if (!res)
        return false;
 
-   myopt.nullPrint = NULL;
    initPQExpBuffer(&title);
    printfPQExpBuffer(&title, _("Objects in extension \"%s\""), extname);
    myopt.title = title.data;
@@ -6340,7 +6309,6 @@ listPublications(const char *pattern)
    if (!res)
        return false;
 
-   myopt.nullPrint = NULL;
    myopt.title = _("List of publications");
    myopt.translate_header = true;
    myopt.translate_columns = translate_columns;
@@ -6695,7 +6663,6 @@ describeSubscriptions(const char *pattern, bool verbose)
    if (!res)
        return false;
 
-   myopt.nullPrint = NULL;
    myopt.title = _("List of subscriptions");
    myopt.translate_header = true;
    myopt.translate_columns = translate_columns;
@@ -6713,12 +6680,19 @@ describeSubscriptions(const char *pattern, bool verbose)
  * Helper function for consistently formatting ACL (privilege) columns.
  * The proper targetlist entry is appended to buf.  Note lack of any
  * whitespace or comma decoration.
+ *
+ * If you change this, see also the handling of attacl in permissionsList(),
+ * which can't conveniently use this code.
  */
 static void
 printACLColumn(PQExpBuffer buf, const char *colname)
 {
    appendPQExpBuffer(buf,
-                     "pg_catalog.array_to_string(%s, E'\\n') AS \"%s\"",
+                     "CASE"
+                     " WHEN pg_catalog.cardinality(%s) = 0 THEN '%s'"
+                     " ELSE pg_catalog.array_to_string(%s, E'\\n')"
+                     " END AS \"%s\"",
+                     colname, gettext_noop("(none)"),
                      colname, gettext_noop("Access privileges"));
 }
 
@@ -6808,7 +6782,6 @@ listOperatorClasses(const char *access_method_pattern,
    if (!res)
        return false;
 
-   myopt.nullPrint = NULL;
    myopt.title = _("List of operator classes");
    myopt.translate_header = true;
    myopt.translate_columns = translate_columns;
@@ -6897,7 +6870,6 @@ listOperatorFamilies(const char *access_method_pattern,
    if (!res)
        return false;
 
-   myopt.nullPrint = NULL;
    myopt.title = _("List of operator families");
    myopt.translate_header = true;
    myopt.translate_columns = translate_columns;
@@ -6996,7 +6968,6 @@ listOpFamilyOperators(const char *access_method_pattern,
    if (!res)
        return false;
 
-   myopt.nullPrint = NULL;
    myopt.title = _("List of operators of operator families");
    myopt.translate_header = true;
    myopt.translate_columns = translate_columns;
@@ -7089,7 +7060,6 @@ listOpFamilyFunctions(const char *access_method_pattern,
    if (!res)
        return false;
 
-   myopt.nullPrint = NULL;
    myopt.title = _("List of support functions of operator families");
    myopt.translate_header = true;
    myopt.translate_columns = translate_columns;
@@ -7141,7 +7111,6 @@ listLargeObjects(bool verbose)
    if (!res)
        return false;
 
-   myopt.nullPrint = NULL;
    myopt.title = _("Large objects");
    myopt.translate_header = true;
 

diff --git a/src/test/regress/expected/psql.out b/src/test/regress/expected/psql.out
index c70205b98a42e611c9dd8ba24066ce35bfde845d..13e4f6db7ba90d9d8fd33855598f1ae7e57da189 100644 (file)
--- a/src/test/regress/expected/psql.out
+++ b/src/test/regress/expected/psql.out
@@ -5812,6 +5812,7 @@ SELECT * FROM bla ORDER BY 1;
  Susie
 (5 rows)
 
+COMMIT;
 -- reset all
 \set AUTOCOMMIT on
 \set ON_ERROR_ROLLBACK off
@@ -6663,3 +6664,57 @@ DROP ROLE regress_du_role0;
 DROP ROLE regress_du_role1;
 DROP ROLE regress_du_role2;
 DROP ROLE regress_du_admin;
+-- Test display of empty privileges.
+BEGIN;
+-- Create an owner for tested objects because output contains owner name.
+CREATE ROLE regress_zeropriv_owner;
+SET LOCAL ROLE regress_zeropriv_owner;
+CREATE DOMAIN regress_zeropriv_domain AS int;
+REVOKE ALL ON DOMAIN regress_zeropriv_domain FROM CURRENT_USER, PUBLIC;
+\dD+ regress_zeropriv_domain
+                                                    List of domains
+ Schema |          Name           |  Type   | Collation | Nullable | Default | Check | Access privileges | Description 
+--------+-------------------------+---------+-----------+----------+---------+-------+-------------------+-------------
+ public | regress_zeropriv_domain | integer |           |          |         |       | (none)            | 
+(1 row)
+
+CREATE PROCEDURE regress_zeropriv_proc() LANGUAGE sql AS '';
+REVOKE ALL ON PROCEDURE regress_zeropriv_proc() FROM CURRENT_USER, PUBLIC;
+\df+ regress_zeropriv_proc
+                                                                                            List of functions
+ Schema |         Name          | Result data type | Argument data types | Type | Volatility | Parallel |         Owner          | Security | Access privileges | Language | Internal name | Description 
+--------+-----------------------+------------------+---------------------+------+------------+----------+------------------------+----------+-------------------+----------+---------------+-------------
+ public | regress_zeropriv_proc |                  |                     | proc | volatile   | unsafe   | regress_zeropriv_owner | invoker  | (none)            | sql      |               | 
+(1 row)
+
+CREATE TABLE regress_zeropriv_tbl (a int);
+REVOKE ALL ON TABLE regress_zeropriv_tbl FROM CURRENT_USER;
+\dp regress_zeropriv_tbl
+                                    Access privileges
+ Schema |         Name         | Type  | Access privileges | Column privileges | Policies 
+--------+----------------------+-------+-------------------+-------------------+----------
+ public | regress_zeropriv_tbl | table | (none)            |                   | 
+(1 row)
+
+CREATE TYPE regress_zeropriv_type AS (a int);
+REVOKE ALL ON TYPE regress_zeropriv_type FROM CURRENT_USER, PUBLIC;
+\dT+ regress_zeropriv_type
+                                                          List of data types
+ Schema |         Name          |     Internal name     | Size  | Elements |         Owner          | Access privileges | Description 
+--------+-----------------------+-----------------------+-------+----------+------------------------+-------------------+-------------
+ public | regress_zeropriv_type | regress_zeropriv_type | tuple |          | regress_zeropriv_owner | (none)            | 
+(1 row)
+
+ROLLBACK;
+-- Test display of default privileges with \pset null.
+CREATE TABLE defprivs (a int);
+\pset null '(default)'
+\z defprivs
+                              Access privileges
+ Schema |   Name   | Type  | Access privileges | Column privileges | Policies 
+--------+----------+-------+-------------------+-------------------+----------
+ public | defprivs | table | (default)         |                   | 
+(1 row)
+
+\pset null ''
+DROP TABLE defprivs;

diff --git a/src/test/regress/sql/psql.sql b/src/test/regress/sql/psql.sql
index 66ff64a160fd504aa35378e1f9142fa24c3c9c90..695c72d8668cb1401782e35909d8a3898142ccbb 100644 (file)
--- a/src/test/regress/sql/psql.sql
+++ b/src/test/regress/sql/psql.sql
@@ -1578,6 +1578,7 @@ COMMIT;
 SELECT COUNT(*) AS "#mum"
 FROM bla WHERE s = 'Mum' \;               -- no mum here
 SELECT * FROM bla ORDER BY 1;
+COMMIT;
 
 -- reset all
 \set AUTOCOMMIT on
@@ -1853,3 +1854,34 @@ DROP ROLE regress_du_role0;
 DROP ROLE regress_du_role1;
 DROP ROLE regress_du_role2;
 DROP ROLE regress_du_admin;
+
+-- Test display of empty privileges.
+BEGIN;
+-- Create an owner for tested objects because output contains owner name.
+CREATE ROLE regress_zeropriv_owner;
+SET LOCAL ROLE regress_zeropriv_owner;
+
+CREATE DOMAIN regress_zeropriv_domain AS int;
+REVOKE ALL ON DOMAIN regress_zeropriv_domain FROM CURRENT_USER, PUBLIC;
+\dD+ regress_zeropriv_domain
+
+CREATE PROCEDURE regress_zeropriv_proc() LANGUAGE sql AS '';
+REVOKE ALL ON PROCEDURE regress_zeropriv_proc() FROM CURRENT_USER, PUBLIC;
+\df+ regress_zeropriv_proc
+
+CREATE TABLE regress_zeropriv_tbl (a int);
+REVOKE ALL ON TABLE regress_zeropriv_tbl FROM CURRENT_USER;
+\dp regress_zeropriv_tbl
+
+CREATE TYPE regress_zeropriv_type AS (a int);
+REVOKE ALL ON TYPE regress_zeropriv_type FROM CURRENT_USER, PUBLIC;
+\dT+ regress_zeropriv_type
+
+ROLLBACK;
+
+-- Test display of default privileges with \pset null.
+CREATE TABLE defprivs (a int);
+\pset null '(default)'
+\z defprivs
+\pset null ''
+DROP TABLE defprivs;