Subject: Re: Restoring default privileges on objects

From: Erik Wienhold <[email-masked]>
Date: Tue, 29 Aug 2023 13:22:31 +0200 (CEST)

> On 29/08/2023 03:23 CEST Stuart McGraw <[email-masked]> wrote:
>
> If I've done a GRANT or REVOKE on some of the tables, how do I restore
> the default privileges so that the “Access privileges” appears empty
> again?  I re-granted what I think are the default privileges but the
> "Access privileges" column for that table contains "user1=arwdDxt/user1"
> rather than being blank.  This is Postgresql-14.

Yes, "user1=arwdDxt/user1" matches the default privileges if user1 is the table
owner.  Function acldefault('r', 'user1'::regrole) [1] gives you the default
privileges for tables.

You could set pg_class.relacl to NULL to restore the default privileges, but
messing with pg_catalog is at your own risk.  Besides that I don't know of any
way to restore the default privileges other than revoking all privileges before
granting whatever acldefault gives you.  Changing the table owner will then
also change the grantee and grantor in pg_class.relacl to the new owner.

[1] https://www.postgresql.org/docs/14/functions-info.html#FUNCTIONS-ACLITEM-FN-TABLE

--
Erik

---

From: Tom Lane <[email-masked]>
Date: Tue, 29 Aug 2023 10:14:45 -0400

> owner.

Right.  There is no (supported) way to cause the ACL entry to go back
to null.  It starts life that way as an ancient hack to save a step
during object creation.  But the moment you do anything to the object's
privileges, the NULL is replaced by an explicit representation of the
default privileges, which is then modified per whatever command you
are giving.  After that the privileges will always be explicit.

There's been occasional discussion of changing this behavior, but
it'd take work and it'd likely add about as much surprise as it
removes.  People have been used to this quirk for a long time.

			regards, tom lane

---

From: Stuart McGraw <[email-masked]>
Date: Tue, 29 Aug 2023 10:43:45 -0600

> removes.  People have been used to this quirk for a long time.

Thank you Erik and Tom for the explanations.  I guess it's a it-is-
what-it-is situation :-).  But while trying to figure it out myself
I found the following:

   test=# CREATE ROLE user1;
   test=# SET ROLE user1;
   test=> CREATE TABLE t1(x int);
   test=> \dp
                               Access privileges
    Schema | Name | Type  | Access privileges | Column privileges | Policies
   --------+------+-------+-------------------+-------------------+----------
    public | t1   | table |                   |                   |

   test=> SELECT FROM t1;
   (0 rows)

   test=> SET ROLE postgres;
   test=# REVOKE ALL ON t1 FROM user1;
   test=# SET ROLE user1;
   test=> \dp
    Schema | Name | Type  | Access privileges | Column privileges | Policies
   --------+------+-------+-------------------+-------------------+----------
    public | t1   | table |                   |                   |

   test=> SELECT FROM t1;
   ERROR:  permission denied for table t1

How does one distinguish between (blank)=(default privileges)
and (blank)=(no privileges)?

Shouldn't psql put *something* (like "(default)" or "-") in the
"Access privileges" column to indicate that?  Or conversely,
something (like "(none)"?) in the revoked case?

It doesn't seem like a good idea to use the same visual
representation for two nearly opposite conditions.  It confused
the heck out of me anyway... :-)

---

From: Erik Wienhold <[email-masked]>
Date: Tue, 29 Aug 2023 21:04:53 +0200 (CEST)

> On 29/08/2023 18:43 CEST Stuart McGraw <[email-masked]> wrote:
>

Indeed, that's confusing.  Command \dp always prints null as empty string [1].
So \pset null '(null)' has no effect.

The docs don't mention that edge case [2] (the second to last paragraph):

	"If the “Access privileges” 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)."

[1] https://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/bin/psql/describe.c;h=bac94a338cfbc497200f0cf960cbabce2dadaa33;hb=9b581c53418666205938311ef86047aa3c6b741f#l1149
[2] https://www.postgresql.org/docs/14/ddl-priv.html

--
Erik

---

From: Tom Lane <[email-masked]>
Date: Tue, 29 Aug 2023 15:27:18 -0400

> So \pset null '(null)' has no effect.

Yeah, perhaps.  The reason it so seldom comes up is that a state of
zero privileges is extremely rare (because it's useless in practice).

That being the case, if we were to do something about this, I'd vote
for changing the display of zero-privileges to "(none)" or something
along that line, rather than changing the display of NULL, which
people are accustomed to.

Fixing \dp to honor "\pset null" for this might be a reasonable
thing to do too.  I'm actually a bit surprised that that doesn't
work already.

			regards, tom lane

---

From: Erik Wienhold <[email-masked]>
Date: Tue, 29 Aug 2023 21:50:30 +0200 (CEST)

> On 29/08/2023 21:27 CEST Tom Lane <[email-masked]> wrote:
>

+1

> thing to do too.  I'm actually a bit surprised that that doesn't

Looks like all commands in src/bin/psql/describe.c set nullPrint = NULL.  Has
been that way since at least 1999.

--
Erik

---

From: Stuart McGraw <[email-masked]>
Date: Tue, 29 Aug 2023 14:44:48 -0600

> thing to do too.  I'm actually a bit surprised that that doesn't

That change would still require someone using \dp to realize that
the "Access privileges" value could be either '' or NULL (I guess
that could be pointed out more obviously in the psql doc), and then
do a '\pset null' before doing \dp?  That seems a little inconvenient.

As a possible alternative, in the query that \dp sends, what about
replacing the line:

   select ...,
     pg_catalog.array_to_string(c.relacl, E'\n') as "Access privileges"
     ...

with something like:

     CASE array_length(c.relacl,1) WHEN 0 THEN '(none)' ELSE pg_catalog.array_to_string(c.relacl, E'\n') END as "Access privileges"

I realize that removes the ability to control with pset what is
displayed, but maybe a little more foolproof for naive users like
myself?

---

From: "Peter J. Holzer" <[email-masked]>
Date: Wed, 30 Aug 2023 12:00:30 +0200

> do a '\pset null' before doing \dp?  That seems a little inconvenient.

Or just always do a \pset null. For me printing NULL the same as an
empty string is just as confusing in normal tables, so that's the first
line in my ~/.psqlrc. YMMV, of course.

But I guess the point is that people who do \pset null expect to be able
to distinguish '' and NULL visually and might be surprised if that
doesn't work everywhere, while people who don't \pset null know that ''
and NULL are visually indistinguishable and that they may need some
other way to distinguish them if the difference matters.

So +1 for me fixing \dp to honor "\pset null".

        hp

-- 
   _  | Peter J. Holzer    | Story must make more sense than reality.
|_|_) |                    |
| |   | [email-masked]         |    -- Charles Stross, "Creative writing
__/   | http://www.hjp.at/ |       challenge!"

---

From: Laurenz Albe <[email-masked]>
Date: Fri, 06 Oct 2023 22:16:28 +0200

+1

Here is a patch that does away with the special handling of NULL values
in psql backslash commands.

Yours,
Laurenz Albe

---

From: Laurenz Albe <[email-masked]>
Date: Fri, 06 Oct 2023 22:18:20 +0200

> in psql backslash commands.

Erm, I forgot to attach the patch.

Yours,
Laurenz Albe
From 6c67f15f011ddf1e309cb7e84580b266d674a1e2 Mon Sep 17 00:00:00 2001
From: Laurenz Albe <[email-masked]>
Date: Fri, 6 Oct 2023 22:12:33 +0200
Subject: [PATCH] psql: honor "\pset null" in backslash commands

In the output of backslash commands, NULL was always displayed
as empty string, rather than honoring the values set with
"\pset null".

This was surprising, and in some cases it was downright confusing:
for example, default privileges (stored as NULL) were displayed
just like an empty aclitem[], making these cases undistinguishable
in the output.

Discussion: https://postgr.es/m/96d6885a-5e25-9ae8-4a1a-d7e557a5fe9c%40mtneva.com
---
 src/bin/psql/describe.c | 43 -----------------------------------------
 1 file changed, 43 deletions(-)

diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index bac94a338c..224aa22575 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;
@@ -6808,7 +6770,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 +6858,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 +6956,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 +7048,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 +7099,6 @@ listLargeObjects(bool verbose)
 	if (!res)
 		return false;
 
-	myopt.nullPrint = NULL;
 	myopt.title = _("Large objects");
 	myopt.translate_header = true;
 
-- 
2.41.0

---

From: Laurenz Albe <[email-masked]>
Date: Fri, 06 Oct 2023 22:28:20 +0200

> Erm, I forgot to attach the patch.

I just realize that there is a conflicting proposal.  I'll reply to that thread.
Sorry for the noise.

Yours,
Laurenz Albe

---

From: "David G. Johnston" <[email-masked]>
Date: Tue, 17 Oct 2023 09:19:01 -0700

> I just realize that there is a conflicting proposal.  I'll reply to that
This thread seems officially closed and the discussion moved to [1].

Over there, after reading both threads, I am seeing enough agreement that
changing these queries to always print "(none)" (translating the word none)
where today they output null, and thus plan to move forward with the v1
patch on that thread proposing to do just that.  Please chime in over there
on this specific option - whether you wish to support or reject it.  Should
it be rejected the plan is to have these queries respect the user's
preference in \pset null.  Please make it clear if you would rather
maintain the status quo against either of those two options.

Thanks!

David J.

[1]
https://www.postgresql.org/message-id/ab67c99bfb5dea7bae18c77f96442820d19b5448.camel%40cybertec.at