On 2023-10-20 22:35 +0200, David G. Johnston wrote:
> Ok, I found my mis-understanding and better understand where you are all
> coming from now; I apparently had the usage of NULL flip-flopped.
>
> Taking pg_tablespace as an example. Its "spcacl" column produces NULL for
> default privileges and '{}'::text[] for empty privileges.
>
> Thus, today:
> empty: array_to_string('{}'::text[], '\n') produces an empty string
> default: array_to_string(null, '\n') produces null which then was printed
> as a hard-coded empty string via forcibly changing \pset null
>
> I was thinking the two cases were reversed.
>
> My proposal for changing printACLColumn is thus:
>
> case when spcacl is null then ''
> when cardinality(spcacl) = 0 then '(none)'
> else array_to_string(spcacl, E'\\n')
> end as "Access privileges"
>
> In short, I don't want default privileges to start to obey \pset null when
> it never has before and is documented as displaying the empty string. I do
> want the empty string produced by empty privileges to change to (none) so
> that it no longer is indistinguishable from our choice of presentation for
> the default privilege case.
>
> Mechanically, we remove the existing \pset null for these metacommands and
> move it into the query via the added CASE expression in the ‎printACLColumn
> function.
>
> This gets rid of NULL as an output value for this column and makes the
> patch regarding \pset null discussion unnecessary. And it leaves the
> existing well-established empty string for default privileges alone (and
> changing this is what I believe Tom is against and I agree on that point).
I haven't thought off this yet. The attached v3 of my initial patch
does that. It also includes Laurenz' fix to no longer ignore \pset null
(minus the doc changes that suggest using \pset null to distinguish
between default and empty privileges because that's no longer needed).
--
Erik
>From 5e3f1840a5a6f49ccfa7f61c040b24638daad421 Mon Sep 17 00:00:00 2001
From: Erik Wienhold <e...@ewie.name>
Date: Sun, 17 Sep 2023 20:54:48 +0200
Subject: [PATCH v3] Fix output of empty privileges in psql
Print "(none)" for empty privileges instead of nothing so that the user
is able to distinguish them from default privileges. \pset null was
ignored to always print default privileges as empty strings. This
kludge is now removed by explicitly returning the empty string for
default privileges with the nice side effect that all describe commands
now honor \pset null.
Meta commands affected by empty privileges:
\db+ \dD+ \des+ \dew+ \df+ \dL+ \dl+ \dn+ \dp \dT+ \l
Default privileges start as NULL::aclitem[] in various catalog columns
but revoking the default privileges leaves an empty aclitem[]. Using
\pset null '(default)' as a workaround for spotting default privileges
did not work because the meta commands ignored this setting.
The privileges shown by \dconfig+ and \ddp as well as the column
privileges shown by \dp are not affected by this change because the
respective aclitem[] is reset to NULL or deleted from the catalog
instead of leaving empty arrays.
Commands \des+ and \dew+ are not covered in src/test/regress because no
foreign data wrapper is available at this point to create a foreign
server.
Handling of empty privileges by Erik Wienhold. Fixing \pset null by
Laurenz Albe.
---
doc/src/sgml/ddl.sgml | 4 +-
src/bin/psql/describe.c | 51 +++-------------
src/test/regress/expected/psql.out | 94 ++++++++++++++++++++++++++++++
src/test/regress/sql/psql.sql | 45 ++++++++++++++
4 files changed, 149 insertions(+), 45 deletions(-)
diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index 075ff32991..1c17c4a967 100644
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -2353,7 +2353,9 @@ GRANT SELECT (col1), UPDATE (col1) ON mytable TO
miriam_rw;
<para>
If the <quote>Access privileges</quote> column is empty for a given
object, it means the object has default privileges (that is, its
- privileges entry in the relevant system catalog is null). Default
+ privileges entry in the relevant system catalog is null). The column shows
+ <literal>(none)</literal> for empty privileges (that is, no privileges at
+ all, even for the object owner — a rare occurrence). Default
privileges always include all privileges for the owner, and can include
some privileges for <literal>PUBLIC</literal> depending on the object
type, as explained above. The first <command>GRANT</command>
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index bac94a338c..0d7f86d80f 100644
--- 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;
@@ -1146,7 +1139,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 +1210,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 +1408,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 +3842,6 @@ listDbRoleSettings(const char *pattern, const char
*pattern2)
}
else
{
- myopt.nullPrint = NULL;
myopt.title = _("List of settings");
myopt.translate_header = true;
@@ -3926,7 +3915,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 +4110,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 +4319,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 +4398,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 +4482,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 +4560,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 +4627,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 +4708,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 +4806,6 @@ listExtendedStats(const char *pattern)
if (!res)
return false;
- myopt.nullPrint = NULL;
myopt.title = _("List of extended statistics");
myopt.translate_header = true;
@@ -4938,7 +4918,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 +5036,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 +5097,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 +5213,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 +5360,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 +5396,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 +5471,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 +5536,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 +5590,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 +5735,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 +5811,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 +5887,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 +5942,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 +6014,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 +6065,6 @@ listExtensions(const char *pattern)
if (!res)
return false;
- myopt.nullPrint = NULL;
myopt.title = _("List of installed extensions");
myopt.translate_header = true;
@@ -6203,7 +6168,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 +6304,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 +6658,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;
@@ -6718,7 +6680,13 @@ static void
printACLColumn(PQExpBuffer buf, const char *colname)
{
appendPQExpBuffer(buf,
- "pg_catalog.array_to_string(%s,
E'\\n') AS \"%s\"",
+ "CASE\n"
+ " WHEN %s IS NULL THEN ''\n"
+ " WHEN pg_catalog.cardinality(%s) =
0 THEN '%s'\n"
+ " ELSE
pg_catalog.array_to_string(%s, E'\\n')\n"
+ "END AS \"%s\"",
+ colname,
+ colname, gettext_noop("(none)"),
colname, gettext_noop("Access
privileges"));
}
@@ -6808,7 +6776,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 +6864,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 +6962,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 +7054,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 +7105,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 c70205b98a..08f854d0a8 100644
--- a/src/test/regress/expected/psql.out
+++ b/src/test/regress/expected/psql.out
@@ -6663,3 +6663,97 @@ DROP ROLE regress_du_role0;
DROP ROLE regress_du_role1;
DROP ROLE regress_du_role2;
DROP ROLE regress_du_admin;
+-- Test empty privileges.
+BEGIN;
+WARNING: there is already a transaction in progress
+-- Create an owner for tested objects because output contains owner info.
+-- Must be superuser to be owner of tablespace.
+CREATE ROLE regress_zeropriv_owner SUPERUSER;
+SET LOCAL ROLE regress_zeropriv_owner;
+ALTER TABLESPACE regress_tblspace OWNER TO CURRENT_USER;
+REVOKE ALL ON TABLESPACE regress_tblspace FROM CURRENT_USER;
+\db+ regress_tblspace
+ List of tablespaces
+ Name | Owner | Location | Access
privileges | Options | Size | Description
+------------------+------------------------+-----------------+-------------------+---------+---------+-------------
+ regress_tblspace | regress_zeropriv_owner | pg_tblspc/16385 | (none)
| | 0 bytes |
+(1 row)
+
+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)
+
+ALTER LANGUAGE plpgsql OWNER TO CURRENT_USER;
+REVOKE ALL ON LANGUAGE plpgsql FROM CURRENT_USER, PUBLIC;
+\dL+ plpgsql
+
List of languages
+ Name | Owner | Trusted | Internal language | Call
handler | Validator | Inline handler |
Access privileges | Description
+---------+------------------------+---------+-------------------+------------------------+------------------------+----------------------------------+-------------------+------------------------------
+ plpgsql | regress_zeropriv_owner | t | f |
plpgsql_call_handler() | plpgsql_validator(oid) |
plpgsql_inline_handler(internal) | (none) | PL/pgSQL procedural
language
+(1 row)
+
+SELECT lo_create(3001);
+ lo_create
+-----------
+ 3001
+(1 row)
+
+REVOKE ALL ON LARGE OBJECT 3001 FROM CURRENT_USER;
+\dl+
+ Large objects
+ ID | Owner | Access privileges | Description
+------+------------------------+-------------------+-------------
+ 3001 | regress_zeropriv_owner | (none) |
+(1 row)
+
+CREATE SCHEMA regress_zeropriv_schema;
+REVOKE ALL ON SCHEMA regress_zeropriv_schema FROM CURRENT_USER;
+\dn+ regress_zeropriv_schema
+ List of schemas
+ Name | Owner | Access privileges |
Description
+-------------------------+------------------------+-------------------+-------------
+ regress_zeropriv_schema | regress_zeropriv_owner | (none) |
+(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)
+
+ALTER DATABASE :"DBNAME" OWNER TO CURRENT_USER;
+REVOKE ALL ON DATABASE :"DBNAME" FROM CURRENT_USER, PUBLIC;
+\l :"DBNAME"
+ List of databases
+ Name | Owner | Encoding | Locale Provider | Collate |
Ctype | ICU Locale | ICU Rules | Access privileges
+------------+------------------------+-----------+-----------------+---------+-------+------------+-----------+-------------------
+ regression | regress_zeropriv_owner | SQL_ASCII | libc | C |
C | | | (none)
+(1 row)
+
+ROLLBACK;
diff --git a/src/test/regress/sql/psql.sql b/src/test/regress/sql/psql.sql
index 66ff64a160..a8fcb898ba 100644
--- a/src/test/regress/sql/psql.sql
+++ b/src/test/regress/sql/psql.sql
@@ -1853,3 +1853,48 @@ DROP ROLE regress_du_role0;
DROP ROLE regress_du_role1;
DROP ROLE regress_du_role2;
DROP ROLE regress_du_admin;
+
+-- Test empty privileges.
+BEGIN;
+-- Create an owner for tested objects because output contains owner info.
+-- Must be superuser to be owner of tablespace.
+CREATE ROLE regress_zeropriv_owner SUPERUSER;
+SET LOCAL ROLE regress_zeropriv_owner;
+
+ALTER TABLESPACE regress_tblspace OWNER TO CURRENT_USER;
+REVOKE ALL ON TABLESPACE regress_tblspace FROM CURRENT_USER;
+\db+ regress_tblspace
+
+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
+
+ALTER LANGUAGE plpgsql OWNER TO CURRENT_USER;
+REVOKE ALL ON LANGUAGE plpgsql FROM CURRENT_USER, PUBLIC;
+\dL+ plpgsql
+
+SELECT lo_create(3001);
+REVOKE ALL ON LARGE OBJECT 3001 FROM CURRENT_USER;
+\dl+
+
+CREATE SCHEMA regress_zeropriv_schema;
+REVOKE ALL ON SCHEMA regress_zeropriv_schema FROM CURRENT_USER;
+\dn+ regress_zeropriv_schema
+
+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
+
+ALTER DATABASE :"DBNAME" OWNER TO CURRENT_USER;
+REVOKE ALL ON DATABASE :"DBNAME" FROM CURRENT_USER, PUBLIC;
+\l :"DBNAME"
+
+ROLLBACK;
--
2.42.0
