Re: Fix output of zero privileges in psql
On 2023-11-13 21:49 +0100, Tom Lane wrote: > Patch pushed with minor adjustments, mainly rewriting some comments. Thanks a lot! -- Erik
Re: Fix output of zero privileges in psql
On Mon, 2023-11-13 at 15:49 -0500, Tom Lane wrote: > Patch pushed with minor adjustments, mainly rewriting some comments. Thank you! Yours, Laurenz Albe
Re: Fix output of zero privileges in psql
"David G. Johnston" writes: > On Mon, Nov 13, 2023 at 12:36 PM Laurenz Albe > wrote: >> On Mon, 2023-11-13 at 11:27 +0100, Erik Wienhold wrote: >>> On 2023-11-09 20:19 +0100, Tom Lane wrote: So, just to clarify, we're settling on your v4 from [1]? >>> Yes from my side. >> +1 > +0.5 for the reasons already stated; but I get and accept the argument for > NULL. Patch pushed with minor adjustments, mainly rewriting some comments. One notable change is that I dropped the newline whitespace printed by printACLColumn. That was contrary to the policy expressed in the function's comment, and IMO it made -E output look worse not better. The problem is that the calling code determines the indentation that this targetlist item should have, and we don't want to outdent from that. I think it's better to make it one line, even though that will run a bit over 80 columns. I also got rid of the use of a created superuser in the test case. The test seems pretty duplicative to me anyway, so let's just not test the object types that need superuser. > I will reiterate my preference for writing an explicit IS NULL branch in > the case expression instead of relying upon the strict-ness of > array_to_string. Meh. We were relying on that already, and it wasn't a problem. I might have done it, except that it'd have made the one line even longer and harder to read (and slower to execute, probably). regards, tom lane
Re: Fix output of zero privileges in psql
On Mon, Nov 13, 2023 at 12:36 PM Laurenz Albe wrote: > On Mon, 2023-11-13 at 11:27 +0100, Erik Wienhold wrote: > > On 2023-11-09 20:19 +0100, Tom Lane wrote: > > > Laurenz Albe writes: > > > > Thanks for the feedback. I'll set the patch to "ready for > committer" then. > > > > > > So, just to clarify, we're settling on your v4 from [1]? > > > > > > [1] > https://www.postgresql.org/message-id/d799f996f422231a99655f1223667d6d887e4c95.ca...@cybertec.at > > > > Yes from my side. > > +1 > > +0.5 for the reasons already stated; but I get and accept the argument for NULL. I will reiterate my preference for writing an explicit IS NULL branch in the case expression instead of relying upon the strict-ness of array_to_string. + "CASE\n" WHEN %s IS NULL THEN NULL + " WHEN pg_catalog.cardinality(%s) = 0 THEN '%s'\n" + " ELSE pg_catalog.array_to_string(%s, E'\\n')\n" + "END AS \"%s\"", David J.
Re: Fix output of zero privileges in psql
On Mon, 2023-11-13 at 11:27 +0100, Erik Wienhold wrote: > On 2023-11-09 20:19 +0100, Tom Lane wrote: > > Laurenz Albe writes: > > > Thanks for the feedback. I'll set the patch to "ready for committer" > > > then. > > > > So, just to clarify, we're settling on your v4 from [1]? > > > > [1] > > https://www.postgresql.org/message-id/d799f996f422231a99655f1223667d6d887e4c95.ca...@cybertec.at > > Yes from my side. +1 Yours, Laurenz Albe
Re: Fix output of zero privileges in psql
On 2023-11-09 20:19 +0100, Tom Lane wrote: > Laurenz Albe writes: > > Thanks for the feedback. I'll set the patch to "ready for committer" then. > > So, just to clarify, we're settling on your v4 from [1]? > > [1] > https://www.postgresql.org/message-id/d799f996f422231a99655f1223667d6d887e4c95.ca...@cybertec.at Yes from my side. -- Erik
Re: Fix output of zero privileges in psql
Laurenz Albe writes: > Thanks for the feedback. I'll set the patch to "ready for committer" then. So, just to clarify, we're settling on your v4 from [1]? regards, tom lane [1] https://www.postgresql.org/message-id/d799f996f422231a99655f1223667d6d887e4c95.ca...@cybertec.at
Re: Fix output of zero privileges in psql
On Thu, 2023-11-09 at 03:40 +0100, Erik Wienhold wrote: > On 2023-11-08 13:23 +0100, Laurenz Albe wrote: > > I wonder how to proceed with this patch. The main disagreement is > > whether default privileges should be displayed as NULL (less invasive, > > but more confusing for beginners) or "(default)" (more invasive, > > but nicer for beginners). > > Are there any reports from beginners being confused about default > privileges being NULL or being displayed as a blank string in psql? > This is usually resolved with a pointer to the docs if it comes up in > discussions or the user makes the mental leap and checks the docs > himself. Both patches add some details to the docs to explain psql's > output. Right. > > David is for "(default)", Tom and me are for NULL, and I guess Erik > > would also prefer "(default)", since that was how his original > > patch did it, IIRC. I think I could live with both solutions. > > > > Kind of a stalemate. Who wants to tip the scales? > > Yes I had a slight preference for my patch but I'd go with yours (\pset > null) now. I followed the discussion after my last mail but had nothing > more to add that wasn't already said. Tom then wrote that NULL is the > catalog's representation for the default privileges and obscuring that > fact in psql is not doing any service to the users. This convinced me > because users may have to deal with aclitem[] being NULL anyway at some > point if they need to check privileges in more detail. So it makes > absolutely sense that psql is transparent about that. Thanks for the feedback. I'll set the patch to "ready for committer" then. Yours, Laurenz Albe
Re: Fix output of zero privileges in psql
On 2023-11-08 13:23 +0100, Laurenz Albe wrote: > I wonder how to proceed with this patch. The main disagreement is > whether default privileges should be displayed as NULL (less invasive, > but more confusing for beginners) or "(default)" (more invasive, > but nicer for beginners). Are there any reports from beginners being confused about default privileges being NULL or being displayed as a blank string in psql? This is usually resolved with a pointer to the docs if it comes up in discussions or the user makes the mental leap and checks the docs himself. Both patches add some details to the docs to explain psql's output. > David is for "(default)", Tom and me are for NULL, and I guess Erik > would also prefer "(default)", since that was how his original > patch did it, IIRC. I think I could live with both solutions. > > Kind of a stalemate. Who wants to tip the scales? Yes I had a slight preference for my patch but I'd go with yours (\pset null) now. I followed the discussion after my last mail but had nothing more to add that wasn't already said. Tom then wrote that NULL is the catalog's representation for the default privileges and obscuring that fact in psql is not doing any service to the users. This convinced me because users may have to deal with aclitem[] being NULL anyway at some point if they need to check privileges in more detail. So it makes absolutely sense that psql is transparent about that. -- Erik
Re: Fix output of zero privileges in psql
On Wed, 2023-11-08 at 10:56 +0530, Shubham Khanna wrote: > I tested the Patch for the modified changes and it is working fine. Thanks for the review! I wonder how to proceed with this patch. The main disagreement is whether default privileges should be displayed as NULL (less invasive, but more confusing for beginners) or "(default)" (more invasive, but nicer for beginners). David is for "(default)", Tom and me are for NULL, and I guess Erik would also prefer "(default)", since that was how his original patch did it, IIRC. I think I could live with both solutions. Kind of a stalemate. Who wants to tip the scales? Yours, Laurenz Albe
Re: Fix output of zero privileges in psql
On Wed, Nov 8, 2023 at 10:46 AM Laurenz Albe wrote:
>
> On Sat, 2023-10-21 at 04:29 +0200, Erik Wienhold wrote:
> > 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).
>
> Thanks!
>
> I went over the patch, fixed some problems and added some more stuff from
> my patch.
>
> In particular:
>
> --- 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;
>
> 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). Default
> + privileges entry in the relevant system catalog is null). The column
> shows
> + (none) 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 PUBLIC depending on the object
> type, as explained above. The first GRANT
>
> This description of empty privileges is smack in the middle of describing
> default privileges. I thought that was confusing and moved it to its
> own paragraph.
>
> --- a/src/bin/psql/describe.c
> +++ b/src/bin/psql/describe.c
> @@ -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"));
>}
>
> This erroneously displays NULL as empty string and subverts my changes.
> I have removed the first branch of the CASE expression.
>
> --- 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
>
> This warning is caused by a pre-existing error in the regression test, which
> forgot to close the transaction. I have added a COMMIT at the appropriate
> place.
>
> +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)
>
> This test is not stable, since it contains the OID of the tablespace, which
> is different every time.
>
> +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)
>
> This test is also not stable, since it depends on the locale definition
> of the regression test database. If you use "make installcheck", that could
> be a different locale.
>
> I think that these tests are not absolutely necessary, and the other tests
> are sufficient. Consequently, I took the simple road of removing them.
>
> I also tried to improve the commit message.
>
> Patch attached.
I tested the Patch for the modified changes and it is working fine.
Thanks and regards,
Shubham Khanna.
Re: Fix output of zero privileges in psql
On Mon, 2023-10-23 at 22:43 -0400, Tom Lane wrote: > Laurenz Albe writes: > > On Mon, 2023-10-23 at 11:37 -0700, David G. Johnston wrote: > > > I do believe that we should be against exposing, like in this case, any > > > internal > > > implementation detail that encodes something (e.g., default privileges) > > > as NULL > > > in the catalogs, to the user of the psql meta-commands. > > > Sure, it would be best to hide this implementation detail from the user. > > The correct way to do that would be to fake an ACL entry like > > "laurenz=arwdDxt/laurenz" > > if there is a NULL in the catalog, but that would add a ton of special-case > > code to psql, which does not look appealing at all. > > For better or worse, that *is* the backend's catalog representation, > and I don't think that psql would be doing our users a service by > trying to obscure the fact. They'd run into it anyway the moment > they look at the catalogs with anything but a \d-something command. ... for example with a client like pgAdmin, which is a frequent choice of many PostgreSQL beginners (they display empty privileges). Yes, it is "(default)" or NULL. The former is friendlier for beginners, the latter incurs less backward incompatibility. I could live with either solution, but I am still leaning towards NULL. I ran the regression tests with a patch that displays "(default)", and I counted 22 failures, excluding the one added by my patch. The tests can of course be fixed, but perhaps that serves as a measure of the backward incompatibility. Yours, Laurenz Albe
Re: Fix output of zero privileges in psql
On Monday, October 23, 2023, Tom Lane wrote: > Laurenz Albe writes: > > On Mon, 2023-10-23 at 11:37 -0700, David G. Johnston wrote: > >> I do believe that we should be against exposing, like in this case, any > internal > >> implementation detail that encodes something (e.g., default privileges) > as NULL > >> in the catalogs, to the user of the psql meta-commands. > > > Sure, it would be best to hide this implementation detail from the user. > > The correct way to do that would be to fake an ACL entry like > "laurenz=arwdDxt/laurenz" > > if there is a NULL in the catalog, but that would add a ton of > special-case > > code to psql, which does not look appealing at all. > > For better or worse, that *is* the backend's catalog representation, > and I don't think that psql would be doing our users a service by > trying to obscure the fact. They'd run into it anyway the moment > they look at the catalogs with anything but a \d-something command. > Which many may never do, and those few that do will see immediately that the catalog uses null where they expected to see “(default)” and realize we made a presentational choice in the interests of readability and their query will need to make a choice regarding the null and empty arrays as well. David J.
Re: Fix output of zero privileges in psql
On Monday, October 23, 2023, Laurenz Albe wrote:
> On Mon, 2023-10-23 at 11:37 -0700, David G. Johnston wrote:
> > > I didn't understand this completely. You want default privileges
> displayed as
> > > "(default)", but are you for or against "\pset null" to have its
> normal effect on
> > > the output of backslash commands in all other cases?
> >
> > I haven’t inspected other cases but to my knowledge we don’t typically
> represent
> > non-unknown things using NULL so I’m not expecting other places to have
> this
> > representation problem.
>
> The first example that comes to my mind is the "ICU Locale" and the "ICU
> Rules"
> in the output of \l. There are many others.
Both of those fall into “this null means there is no value for these
(because we aren’t using icu)”. I have no qualms with leaving true nulls
represented as themselves. Clean slate maybe I print “(not using icu)”
there instead of null but it isn’t worth the effort to change.
>
> > I won’t argue that exposing such NULLS is wrong, just it would
> preferable IME
> > to avoid doing so. NULL means unknown or not applicable and default
> privileges
> > are neither of those things. I get why our catalogs choose such an
> encoding and
> > agree with it, and users that find the need to consult the catalogs will
> need to
> > learn such details. But we should strive for them to be able to survive
> with
> > psql meta-commands.
>
> Sure, it would be best to hide this implementation detail from the user.
> The correct way to do that would be to fake an ACL entry like
> "laurenz=arwdDxt/laurenz"
> if there is a NULL in the catalog, but that would add a ton of special-case
> code to psql, which does not look appealing at all.
More generically it would be “[PUBLIC=]/???/postgres” and
{OWNER}=???/postgres
It would ideally be a function call for psql and a system info function
usable for anyone.
David J.
Re: Fix output of zero privileges in psql
Laurenz Albe writes: > On Mon, 2023-10-23 at 11:37 -0700, David G. Johnston wrote: >> I do believe that we should be against exposing, like in this case, any >> internal >> implementation detail that encodes something (e.g., default privileges) as >> NULL >> in the catalogs, to the user of the psql meta-commands. > Sure, it would be best to hide this implementation detail from the user. > The correct way to do that would be to fake an ACL entry like > "laurenz=arwdDxt/laurenz" > if there is a NULL in the catalog, but that would add a ton of special-case > code to psql, which does not look appealing at all. For better or worse, that *is* the backend's catalog representation, and I don't think that psql would be doing our users a service by trying to obscure the fact. They'd run into it anyway the moment they look at the catalogs with anything but a \d-something command. regards, tom lane
Re: Fix output of zero privileges in psql
On Mon, 2023-10-23 at 11:37 -0700, David G. Johnston wrote: > > I didn't understand this completely. You want default privileges displayed > > as > > "(default)", but are you for or against "\pset null" to have its normal > > effect on > > the output of backslash commands in all other cases? > > I haven’t inspected other cases but to my knowledge we don’t typically > represent > non-unknown things using NULL so I’m not expecting other places to have this > representation problem. The first example that comes to my mind is the "ICU Locale" and the "ICU Rules" in the output of \l. There are many others. > I don’t think any of our meta-command outputs should modify pset null. > Left join cases should be considered unknown, represented as NULL, and obey > the > user’s setting. That's what I think too. psql output should respect "\pset null". So it looks like we agree on that. > I do believe that we should be against exposing, like in this case, any > internal > implementation detail that encodes something (e.g., default privileges) as > NULL > in the catalogs, to the user of the psql meta-commands. > > I won’t argue that exposing such NULLS is wrong, just it would preferable IME > to avoid doing so. NULL means unknown or not applicable and default > privileges > are neither of those things. I get why our catalogs choose such an encoding > and > agree with it, and users that find the need to consult the catalogs will need > to > learn such details. But we should strive for them to be able to survive with > psql meta-commands. Sure, it would be best to hide this implementation detail from the user. The correct way to do that would be to fake an ACL entry like "laurenz=arwdDxt/laurenz" if there is a NULL in the catalog, but that would add a ton of special-case code to psql, which does not look appealing at all. So we cannot completely hide the implementation, but perhaps "(default)" would be less confusing than a NULL value. If everybody agrees, I can modify the patch to do that. Yours, Laurenz Albe
Re: Fix output of zero privileges in psql
On Monday, October 23, 2023, Laurenz Albe wrote: > On Mon, 2023-10-23 at 08:35 -0700, David G. Johnston wrote: > > > along with not translating (none) and (default) and thus making the data > contents > > of these views environment independent. But minimizing the variance of > these command's > > output across systems doesn't seem like a design goal that is likely to > gain consensus > > and is excessive when viewed within the framework of these being only > for human consumption. > > I didn't understand this completely. You want default privileges > displayed as > "(default)", but are you for or against "\pset null" to have its normal > effect on > the output of backslash commands in all other cases? I haven’t inspected other cases but to my knowledge we don’t typically represent non-unknown things using NULL so I’m not expecting other places to have this representation problem. I don’t think any of our meta-command outputs should modify pset null. Left join cases should be considered unknown, represented as NULL, and obey the user’s setting. I do believe that we should be against exposing, like in this case, any internal implementation detail that encodes something (e.g., default privileges) as NULL in the catalogs, to the user of the psql meta-commands. I won’t argue that exposing such NULLS is wrong, just it would preferable IME to avoid doing so. NULL means unknown or not applicable and default privileges are neither of those things. I get why our catalogs choose such an encoding and agree with it, and users that find the need to consult the catalogs will need to learn such details. But we should strive for them to be able to survive with psql meta-commands. David J.
Re: Fix output of zero privileges in psql
On Mon, 2023-10-23 at 08:35 -0700, David G. Johnston wrote: > I tend to prefer the argument that these views are for human consumption and > should > be designed with that in mind. True, although given the shape of ACLs, it takes a somewhat trained human to consume the string representation. But we won't be able to hide the fact that default ACLs are NULL in the catalogs. We can leave them empty, we can show them as "(default)" or we can let the user choose with "\pset null". > I would suggest that we make the expected presence of NULL as an input > explicit: > I would offer up: > > when spcacl is null then '(default)' Noted. > along with not translating (none) and (default) and thus making the data > contents > of these views environment independent. But minimizing the variance of these > command's > output across systems doesn't seem like a design goal that is likely to gain > consensus > and is excessive when viewed within the framework of these being only for > human consumption. I didn't understand this completely. You want default privileges displayed as "(default)", but are you for or against "\pset null" to have its normal effect on the output of backslash commands in all other cases? Speaking of consensus, it seems to me that Tom, Erik and me are in consensus. Yours, Laurenz Albe
Re: Fix output of zero privileges in psql
On Mon, Oct 23, 2023 at 7:57 AM Tom Lane wrote: > > IOW, the current definition is "NULL privileges print as an empty > string no matter what", and I don't think that serves to reduce > confusion about whether an ACL is NULL or not. We ought to be doing > what we can to make clear that such an ACL *is* NULL, because > otherwise people won't understand how it differs from an empty ACL. > > I tend to prefer the argument that these views are for human consumption and should be designed with that in mind. Allowing the user to decide whether they prefer NULL to print as the empty string or something else works within that framework. And the change of behavior for everyone with a non-default \pset gets accepted under that framework as well. I would suggest that we make the expected presence of NULL as an input explicit: case when spcacl is null then null when cardinality(spcacl) = 0 then '(none)' -- so as not to confuse it with null being printed also as an empty string else array_to_string(spcacl, E'\\n') end as "Access privileges" I would offer up: when spcacl is null then '(default)' along with not translating (none) and (default) and thus making the data contents of these views environment independent. But minimizing the variance of these command's output across systems doesn't seem like a design goal that is likely to gain consensus and is excessive when viewed within the framework of these being only for human consumption. David J.
Re: Fix output of zero privileges in psql
"David G. Johnston" writes: > We document default privileges print as an empty string - I do not think we > should change the definition to "default privileges print null which can be > controlled via \pset null", and regardless of preference doing so is not a > bug. Well, "if it's documented it's not a bug" is a defensible argument for calling something not a bug, but it doesn't address the question of whether changing it would be an improvement. I think it would be, and I object to your claim that we have a consensus to not do that. The core of the problem here, IMO, is exactly that there is confusion between whether a visibly empty string represents NULL (default privileges) or an empty ACL (no privileges). I believe we have agreed that printing "(none)" or some other clearly-not-an-ACL-entry string for the second case is an improvement, even though that's a change in behavior. That doesn't mean that changing the other case can't also be an improvement. I think it'd be less confusing all around if this instance of NULL prints like other instances of NULL. IOW, the current definition is "NULL privileges print as an empty string no matter what", and I don't think that serves to reduce confusion about whether an ACL is NULL or not. We ought to be doing what we can to make clear that such an ACL *is* NULL, because otherwise people won't understand how it differs from an empty ACL. regards, tom lane
Re: Fix output of zero privileges in psql
On Monday, October 23, 2023, Laurenz Albe wrote:
> On Mon, 2023-10-23 at 07:03 -0700, David G. Johnston wrote:
> > On Monday, October 23, 2023, Laurenz Albe
> wrote:
> > >
> > > --- a/src/bin/psql/describe.c
> > > +++ b/src/bin/psql/describe.c
> > > @@ -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"));
> > >}
> > >
> > > This erroneously displays NULL as empty string and subverts my changes.
> > > I have removed the first branch of the CASE expression.
> >
> > There is no error here, the current consensus which this patch
> implements is to
> > not change the documented “default privileges display as blank”. We are
> solving
> > the empty privileges are not distinguishable complaint by printing
> (none) for them.
>
> Erik's latest patch included my changes to display NULL as NULL in psql,
> so that "\pset null" works as expected.
>
No, per the commit message, issuing an explicit \pset null is a kludge and
it gets rid of the hack in favor of making the query itself produce an
empty string. i.e., we choose a poor implementation to get the documented
behavior and we are cleaning that up as an aside to the main fix.
Changing the behavior so that default privileges print in correspondence to
the setting of \pset null is, IME, off the table for this patch. Its one
and only goal is to reliably distinguish empty and default privileges.
That is our extant bug.
We document default privileges print as an empty string - I do not think we
should change the definition to "default privileges print null which can be
controlled via \pset null", and regardless of preference doing so is not a
bug.
David J.
Re: Fix output of zero privileges in psql
On Mon, 2023-10-23 at 07:03 -0700, David G. Johnston wrote:
> On Monday, October 23, 2023, Laurenz Albe wrote:
> >
> > --- a/src/bin/psql/describe.c
> > +++ b/src/bin/psql/describe.c
> > @@ -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"));
> > }
> >
> > This erroneously displays NULL as empty string and subverts my changes.
> > I have removed the first branch of the CASE expression.
>
> There is no error here, the current consensus which this patch implements is
> to
> not change the documented “default privileges display as blank”. We are
> solving
> the empty privileges are not distinguishable complaint by printing (none) for
> them.
Erik's latest patch included my changes to display NULL as NULL in psql,
so that "\pset null" works as expected.
But he left the above hunk from his original patch, and that hunk replaces
NULL with an empty string, so "\pset null" wouldn't work for the display
of default provoleges.
He didn't notice it because he didn't have a regression test that displays
default privileges with non-empty "\pset null".
Yours,
Laurenz Albe
Re: Fix output of zero privileges in psql
On Monday, October 23, 2023, Laurenz Albe wrote:
>
> --- a/src/bin/psql/describe.c
> +++ b/src/bin/psql/describe.c
> @@ -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"));
>}
>
> This erroneously displays NULL as empty string and subverts my changes.
> I have removed the first branch of the CASE expression.
>
There is no error here, the current consensus which this patch implements
is to not change the documented “default privileges display as blank”. We
are solving the empty privileges are not distinguishable complaint by
printing (none) for them.
David J.
Re: Fix output of zero privileges in psql
On Sat, 2023-10-21 at 04:29 +0200, Erik Wienhold wrote:
> 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).
Thanks!
I went over the patch, fixed some problems and added some more stuff from
my patch.
In particular:
--- 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;
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). Default
+ privileges entry in the relevant system catalog is null). The column
shows
+ (none) 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 PUBLIC depending on the object
type, as explained above. The first GRANT
This description of empty privileges is smack in the middle of describing
default privileges. I thought that was confusing and moved it to its
own paragraph.
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -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"));
}
This erroneously displays NULL as empty string and subverts my changes.
I have removed the first branch of the CASE expression.
--- 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
This warning is caused by a pre-existing error in the regression test, which
forgot to close the transaction. I have added a COMMIT at the appropriate
place.
+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)
This test is not stable, since it contains the OID of the tablespace, which
is different every time.
+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)
This test is also not stable, since it depends on the locale definition
of the regression test database. If you use "make installcheck", that could
be a different locale.
I think that these tests are not absolutely necessary, and the other tests
are sufficient. Consequently, I took the simple road of removing them.
I also tried to improve the commit message.
Patch attached.
Yours,
Laurenz Albe
From df5137f537cc0bef68c126a1e20bda831689b8aa Mon Sep 17 00:00:00 2001
From: Laurenz Albe
Date: Mon, 23 Oct 2023 11:24:01 +0200
Subject: [PATCH] Fix default and empty privilege output in psql
Default privileges start as NULL::aclitem[] in various catalog columns,
but revoking all privileges leaves an empty aclitem[]. These two
cases used to produce the same output with psql meta-commands like \dp.
Using "\pset null '(default)'" as a workaround for spotting default
privileges did not work, because null values were always displayed
as empty strings by psql meta-commands.
This patch improves that with two changes:
1. Print
Re: Fix output of zero privileges in psql
On Fri, Oct 20, 2023 at 7:29 PM Erik Wienhold wrote: > On 2023-10-20 22:35 +0200, David G. Johnston wrote: > > 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. > > 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). > > Thank you. It looks good to me as-is, with one possible nit. I wonder if it would be clearer to say: "If the Access privileges column is *blank* for a given object..." instead of "empty" to avoid having both "empty [string]" and "empty privileges" present in the same paragraph and the empty string not pertaining to the empty privileges. David J.
Re: Fix output of zero privileges in psql
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
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;
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). Default
+ privileges entry in the relevant system catalog is null). The column shows
+ (none) 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 PUBLIC depending on the object
type, as explained above. The first GRANT
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
Re: Fix output of zero privileges in psql
On 2023-10-20 08:42 +0200, Laurenz Albe wrote:
> If you want to proceed with your patch, you could send a new version.
v2 attached.
> I think it could do with an added line of documentation to the
> "Privileges" chapter, and I'd say that the regression tests should be
> in "psql.sql" (not that it is very important).
I added some docs. There will be merge conflicts when combining with
your v5! Also moved the regression tests to psql.sql which is the right
place for testing meta commands.
--
Erik
>From 170ca82fa7b74cc9102582cdf48042131d5ae016 Mon Sep 17 00:00:00 2001
From: Erik Wienhold
Date: Sun, 17 Sep 2023 20:54:48 +0200
Subject: [PATCH v2] Fix output of empty privileges in psql
Print "(none)" for empty privileges instead of nothing so that the user
is able to distinguish empty from default privileges. This affects the
following meta commands:
\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 array.
Using \pset null '(null)' as a workaround to spot default privileges
does not work because the meta commands ignore 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.
---
doc/src/sgml/ddl.sgml | 4 +-
src/bin/psql/describe.c| 6 +-
src/test/regress/expected/psql.out | 94 ++
src/test/regress/sql/psql.sql | 45 ++
4 files changed, 147 insertions(+), 2 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;
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). Default
+ privileges entry in the relevant system catalog is null). The column shows
+ (none) 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 PUBLIC depending on the object
type, as explained above. The first GRANT
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index bac94a338c..154d244d97 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -6718,7 +6718,11 @@ static void
printACLColumn(PQExpBuffer buf, const char *colname)
{
appendPQExpBuffer(buf,
- "pg_catalog.array_to_string(%s,
E'\\n') AS \"%s\"",
+ "CASE pg_catalog.cardinality(%s)\n"
+ " WHEN 0 THEN '%s'\n"
+ " ELSE
pg_catalog.array_to_string(%s, E'\\n')\n"
+ "END AS \"%s\"",
+ colname, gettext_noop("(none)"),
colname, gettext_noop("Access
privileges"));
}
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 |
Re: Fix output of zero privileges in psql
On Fri, Oct 20, 2023 at 12:57 PM Tom Lane wrote:
> "David G. Johnston" writes:
> > On Fri, Oct 20, 2023 at 12:34 PM Tom Lane wrote:
> >> As near as I can tell, doing both things (the \pset null fix and
> >> substituting "(none)" for empty privileges) would be an acceptable
> >> answer to everyone who has commented. Let's proceed with both
> >> patches, or combine them into one if there are merge conflicts.
>
> > I'm under the impression that removing the null representation of empty
> > privileges by making them (none) removes all known \d commands that
> output
> > nulls and don't obey \pset null.
>
> How so? IIUC the proposal is to substitute "(none)" for empty-string
> ACLs, not null ACLs. The \pset change should be addressing an
> independent case.
>
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).
David J.
Re: Fix output of zero privileges in psql
"David G. Johnston" writes: > On Fri, Oct 20, 2023 at 12:34 PM Tom Lane wrote: >> As near as I can tell, doing both things (the \pset null fix and >> substituting "(none)" for empty privileges) would be an acceptable >> answer to everyone who has commented. Let's proceed with both >> patches, or combine them into one if there are merge conflicts. > I'm under the impression that removing the null representation of empty > privileges by making them (none) removes all known \d commands that output > nulls and don't obey \pset null. How so? IIUC the proposal is to substitute "(none)" for empty-string ACLs, not null ACLs. The \pset change should be addressing an independent case. regards, tom lane
Re: Fix output of zero privileges in psql
On Fri, Oct 20, 2023 at 12:34 PM Tom Lane wrote: > Laurenz Albe writes: > > I am not sure how to proceed. Perhaps it would indeed be better to have > > two competing commitfest entries. Both could be "ready for committer", > > and the committers can decide what they prefer. > > As near as I can tell, doing both things (the \pset null fix and > substituting "(none)" for empty privileges) would be an acceptable > answer to everyone who has commented. Let's proceed with both > patches, or combine them into one if there are merge conflicts. > > I'm under the impression that removing the null representation of empty privileges by making them (none) removes all known \d commands that output nulls and don't obey \pset null. At least, the existing \pset null patch doesn't purport to fix anything that would become not applicable if the (none) patch goes in. I.e., at present they are mutually exclusive. David J.
Re: Fix output of zero privileges in psql
Laurenz Albe writes: > I am not sure how to proceed. Perhaps it would indeed be better to have > two competing commitfest entries. Both could be "ready for committer", > and the committers can decide what they prefer. As near as I can tell, doing both things (the \pset null fix and substituting "(none)" for empty privileges) would be an acceptable answer to everyone who has commented. Let's proceed with both patches, or combine them into one if there are merge conflicts. regards, tom lane
Re: Fix output of zero privileges in psql
On Fri, 2023-10-20 at 04:13 +0200, Erik Wienhold wrote: > On 2023-10-17 04:05 +0200, David G. Johnston wrote: > > Erik seems to favors (none) > > Yes, with a slight favor for "(none)" because it's the least disruptive > to users who change \pset null to a non-blank string. The output of \dp > etc. would still look the same for default privileges. > > But I'm also okay with respecting \pset null because it is so simple. > We will no longer hide the already documented null representation of > default privileges by allowing the user to display the ACL as it is. > And with Laurenz' patch we will also document the special case of zero > privileges and how to distinguish it. If you want to proceed with your patch, you could send a new version. I think it could do with an added line of documentation to the "Privileges" chapter, and I'd say that the regression tests should be in "psql.sql" (not that it is very important). I am not sure how to proceed. Perhaps it would indeed be better to have two competing commitfest entries. Both could be "ready for committer", and the committers can decide what they prefer. Yours, Laurenz Albe
Re: Fix output of zero privileges in psql
On 2023-10-17 04:05 +0200, David G. Johnston wrote: > Erik seems to favors (none) Yes, with a slight favor for "(none)" because it's the least disruptive to users who change \pset null to a non-blank string. The output of \dp etc. would still look the same for default privileges. But I'm also okay with respecting \pset null because it is so simple. We will no longer hide the already documented null representation of default privileges by allowing the user to display the ACL as it is. And with Laurenz' patch we will also document the special case of zero privileges and how to distinguish it. -- Erik
Re: Fix output of zero privileges in psql
Laurenz Albe writes: > On Mon, 2023-10-16 at 19:05 -0700, David G. Johnston wrote: >> Reading both threads I'm not seeing any specific rejection of the >> solution that we simply represent empty privileges as "(none)". > Thanks for that summary. I prefer my version (simply display NULLs > as NULLs), but I am not wedded to it. I had the impression that Tom > would prefer that too, but is woried if the incompatibility introduced > would outweigh the small benefit (of either patch). > So it is clear that we don't have a consensus. FWIW, my druthers are to make the describe.c queries honor \pset null (not only for privileges, but anywhere else they fail to) and do nothing beyond that. I think that'll generally reduce the surprise factor, while anything else we might opt to do will increase it. If that fails to garner a consensus, my second choice would be to do that plus translate empty-but-not-null ACLs to "(none)". regards, tom lane
Re: Fix output of zero privileges in psql
On Mon, 2023-10-16 at 19:05 -0700, David G. Johnston wrote: > Reading both threads I'm not seeing any specific rejection of the > solution that we simply represent empty privileges as "(none)". > > I see an apparent consensus that if we do continue to represent it > as NULL that the printout should respect \pset null. > > Tom commented in favor of (none) on the other thread with some > commentary regarding how extremely rare it is; then turns around > and is "fairly concerned" about changing the current blank presentation > of its value which is going to happen for some people regardless > of which approach is chosen. > > Stuart, the original user complainant, seems to favor (none) > > Erik seems to favors (none) > > I favor (none) > > It's unclear to me whether you Laurenz are against (none) or just > trying to go with the group consensus that doesn't actually seem to > be against (none). > > I'm in favor of iterating on v1 on this thread (haven't read it yet) > and presenting it as the proposed solution. If that ends up getting > shot down we can revive v5 (still need to review as well). Thanks for that summary. I prefer my version (simply display NULLs as NULLs), but I am not wedded to it. I had the impression that Tom would prefer that too, but is woried if the incompatibility introduced would outweigh the small benefit (of either patch). So it is clear that we don't have a consensus. I don't think I want to go ahead with my version of the patch unless there is more support for it. I can review Erik's original code, if that design meets with more favor. > We should probably post on that thread that this one exists and post a link > to it. Perhaps a good idea, yes. Yours, Laurenz Albe
Re: Fix output of zero privileges in psql
On Mon, Oct 16, 2023 at 6:19 PM Laurenz Albe wrote: > On Mon, 2023-10-16 at 23:51 +0200, Erik Wienhold wrote: > > What's the process for the CommitFest now since we settled on your > > patch? This is my first time being involved in this, so still learning. > > I'see that you've withdrawn your initial patch [1] but this thread is > > still attached to my patch [2]. Should I edit my CF entry (or withdraw > > it) and you reactivate yours? > > I don't think it makes sense to have two competing commitfest entries, > and we should leave it on this thread. If you are concerned about > authorship, we could both be mentioned as co-authors, if the patch ever > gets committed. > > I'd still like to wait for feedback from David before I change anything. > > Reading both threads I'm not seeing any specific rejection of the solution that we simply represent empty privileges as "(none)". I see an apparent consensus that if we do continue to represent it as NULL that the printout should respect \pset null. Tom commented in favor of (none) on the other thread with some commentary regarding how extremely rare it is; then turns around and is "fairly concerned" about changing the current blank presentation of its value which is going to happen for some people regardless of which approach is chosen. (idk...maybe in favor of (none)) Peter's comment doesn't strike me as recognizing that (none) is even an option on the table...(n/a) Stuart, the original user complainant, seems to favor (none) Erik seems to favors (none) I favor (none) It's unclear to me whether you Laurenz are against (none) or just trying to go with the group consensus that doesn't actually seem to be against (none). I'm in favor of iterating on v1 on this thread (haven't read it yet) and presenting it as the proposed solution. If that ends up getting shot down we can revive v5 (still need to review as well). We should probably post on that thread that this one exists and post a link to it. David J.
Re: Fix output of zero privileges in psql
On Mon, 2023-10-16 at 23:51 +0200, Erik Wienhold wrote: > What's the process for the CommitFest now since we settled on your > patch? This is my first time being involved in this, so still learning. > I'see that you've withdrawn your initial patch [1] but this thread is > still attached to my patch [2]. Should I edit my CF entry (or withdraw > it) and you reactivate yours? I don't think it makes sense to have two competing commitfest entries, and we should leave it on this thread. If you are concerned about authorship, we could both be mentioned as co-authors, if the patch ever gets committed. I'd still like to wait for feedback from David before I change anything. Yours, Laurenz Albe
Re: Fix output of zero privileges in psql
On 2023-10-16 17:56 +0200, Laurenz Albe write: > David, how do you feel about this? I am wondering whether to set this patch > "ready for committer" or not. > > There is Tom wondering upthread whether changing psql's behavior like that > is too much of a compatibility break or not, but I guess it is alright to > leave that final verdict to a committer. What's the process for the CommitFest now since we settled on your patch? This is my first time being involved in this, so still learning. I'see that you've withdrawn your initial patch [1] but this thread is still attached to my patch [2]. Should I edit my CF entry (or withdraw it) and you reactivate yours? [1] https://commitfest.postgresql.org/45/4603/ [2] https://commitfest.postgresql.org/45/4593/ -- Erik
Re: Fix output of zero privileges in psql
On Sat, 2023-10-14 at 02:45 +0200, Erik Wienhold wrote: > On 2023-10-09 09:54 +0200, Laurenz Albe write: > > > > I tinkered a bit with your documentation. For example, the suggestion to > > "\pset null" seemed to be in an inappropriate place. Tell me what you > > think. > > +1 You're right to put that sentence right after the explanation of > empty privileges. Thanks for looking. David, how do you feel about this? I am wondering whether to set this patch "ready for committer" or not. There is Tom wondering upthread whether changing psql's behavior like that is too much of a compatibility break or not, but I guess it is alright to leave that final verdict to a committer. Yours, Laurenz Albe
Re: Fix output of zero privileges in psql
On 2023-10-09 22:34 +0200, David G. Johnston write: > On Mon, Oct 9, 2023 at 12:13 PM Tom Lane wrote: > > Yeah. There is a lot of attraction in having \pset null affect these > > displays just like all other ones. The fact that they act differently > > now is a wart, not something we should replace with a different special > > case behavior. > > > > Also, I'm fairly concerned about not changing the default behavior here. > > The fact that this behavior has stood for a couple dozen years without > > many complaints indicates that there's not all that big a problem to be > > solved. I doubt that a new default behavior will be well received, > > even if it's arguably better. > > > > My position is that the default behavior should be changed such that the > distinction these reports need to make between empty privileges and default > privileges is impossible for the user to remove. I suppose the best direct > solution given that goal is for the query to simply do away with any > reliance on NULL being an indicator. Output an i18n'd "no permissions > present" line in the rare empty permissions situation and leave the empty > string for built-in default. My initial patch does exactly that. > If the consensus is to not actually fix these views to make them > environment invariant in their accuracy then so be it. Having them obey > \pset null seems like a half-measure but it at least is an improvement over > the status quo such that users are capable of altering their system to make > the reports distinguish the two cases if they realize the need. I agree. -- Erik
Re: Fix output of zero privileges in psql
On 2023-10-09 10:29 +0200, Laurenz Albe write: > On Sun, 2023-10-08 at 19:58 -0700, David G. Johnston wrote: > > We probably should add the two terms to the glossary: > > > > empty privileges: all privileges explicitly revoked from the owner and > > PUBLIC > > (where applicable), and none otherwise granted. > > > > built-in default privileges: owner having all privileges and no privileges > > granted or removed via ALTER DEFAULT PRIVILEGES > > "Empty privileges" are too unimportant to warrant an index entry. > > I can see the value of an index entry > > > privilege > default > > > Done in the attached v5 of the patch, even though this is not really > the business of this patch. +1 -- Erik
Re: Fix output of zero privileges in psql
On 2023-10-09 09:54 +0200, Laurenz Albe write: > > I tinkered a bit with your documentation. For example, the suggestion to > "\pset null" seemed to be in an inappropriate place. Tell me what you think. +1 You're right to put that sentence right after the explanation of empty privileges. -- Erik
Re: Fix output of zero privileges in psql
On Mon, 2023-10-09 at 15:13 -0400, Tom Lane wrote: > Laurenz Albe writes: > > The whole point of this patch is to make psql behave consistently with > > respect to > > NULLs in meta-commands. > > Yeah. There is a lot of attraction in having \pset null affect these > displays just like all other ones. The fact that they act differently > now is a wart, not something we should replace with a different special > case behavior. > > Also, I'm fairly concerned about not changing the default behavior here. > The fact that this behavior has stood for a couple dozen years without > many complaints indicates that there's not all that big a problem to be > solved. I doubt that a new default behavior will be well received, > even if it's arguably better. I understand your concern. People who have "\pset null" in their .psqlrc might be surprised if suddenly "" starts appearing in the outout of \l. But then the people who have "\pset null" in their .psqlrc are typically already somewhat experienced and might have less trouble dealing with that change (but they are more likely to bleat on the mailing list, granted). Users with little experience won't notice any difference, so they won't be confused by the change. Yours, Laurenz Albe
Re: Fix output of zero privileges in psql
On Mon, Oct 9, 2023 at 12:13 PM Tom Lane wrote: > Laurenz Albe writes: > > On Mon, 2023-10-09 at 09:30 -0700, David G. Johnston wrote: > >> My point with the second paragraph is that we could, instead of > documenting the > >> caveat about null printing as empty strings is to instead issue an > implicit > >> "\pset null ''" as part of these commands, and a "\pset null" > afterward, > >> conditioned upon it not already being set to a non-empty value. IOW, > the > >> special-casing we do today but actually do it in a way that > distinguishes the > >> two cases instead of forcing them to be indistinguishable. > > > -1 > > > The whole point of this patch is to make psql behave consistently with > respect to > > NULLs in meta-commands. Your suggestion would subvert that idea. > > Yeah. There is a lot of attraction in having \pset null affect these > displays just like all other ones. The fact that they act differently > now is a wart, not something we should replace with a different special > case behavior. > > Also, I'm fairly concerned about not changing the default behavior here. > The fact that this behavior has stood for a couple dozen years without > many complaints indicates that there's not all that big a problem to be > solved. I doubt that a new default behavior will be well received, > even if it's arguably better. > My position is that the default behavior should be changed such that the distinction these reports need to make between empty privileges and default privileges is impossible for the user to remove. I suppose the best direct solution given that goal is for the query to simply do away with any reliance on NULL being an indicator. Output an i18n'd "no permissions present" line in the rare empty permissions situation and leave the empty string for built-in default. If the consensus is to not actually fix these views to make them environment invariant in their accuracy then so be it. Having them obey \pset null seems like a half-measure but it at least is an improvement over the status quo such that users are capable of altering their system to make the reports distinguish the two cases if they realize the need. I do agree that my suggestion regarding the implicit \pset null, while plausible, leaves the wart that NULL is being used to mean something specific. Better is to just use a label for that specific thing. David J.
Re: Fix output of zero privileges in psql
Laurenz Albe writes: > On Mon, 2023-10-09 at 09:30 -0700, David G. Johnston wrote: >> My point with the second paragraph is that we could, instead of documenting >> the >> caveat about null printing as empty strings is to instead issue an implicit >> "\pset null ''" as part of these commands, and a "\pset null" >> afterward, >> conditioned upon it not already being set to a non-empty value. IOW, the >> special-casing we do today but actually do it in a way that distinguishes the >> two cases instead of forcing them to be indistinguishable. > -1 > The whole point of this patch is to make psql behave consistently with > respect to > NULLs in meta-commands. Your suggestion would subvert that idea. Yeah. There is a lot of attraction in having \pset null affect these displays just like all other ones. The fact that they act differently now is a wart, not something we should replace with a different special case behavior. Also, I'm fairly concerned about not changing the default behavior here. The fact that this behavior has stood for a couple dozen years without many complaints indicates that there's not all that big a problem to be solved. I doubt that a new default behavior will be well received, even if it's arguably better. regards, tom lane
Re: Fix output of zero privileges in psql
On Mon, 2023-10-09 at 09:30 -0700, David G. Johnston wrote: > On Mon, Oct 9, 2023 at 1:29 AM Laurenz Albe wrote: > > On Sun, 2023-10-08 at 19:58 -0700, David G. Johnston wrote: > > > > > The built-in default privileges are only in effect if the object has not > > > been > > > the target of a GRANT or REVOKE and also has not had its default > > > privileges > > > modified using ALTER DEFAULT PRIVILEGES. (???: if it is possible to revert > > > back to the state of built-in privileges that would need to be described > > > here.) > > > > I don't think that we need to mention ALTER DEFAULT PRIVILEGES there. If > > the default privileges have been altered, the ACL will not be stored as > > NULL in the catalogs. > > It's already mentioned there, I just felt bringing the mention of ADP and > grant/revoke both invalidating the built-in default privileges closer together > would be an improvement. Ah, sorry, I misread your comment. You are right. But then, the effects of ALTER DEFAULT PRIVILEGES are discussed later in the paragraph anyway, and we don't have to repeat that here. > > > > To me, mentioning the default privileges are stored as NULLs in the catalogs > > is not an invitation to view the privileges with catalog queries, but > > information about implementation details that explains why default > > privileges > > are displayed the way they are. > > Calling it an implementation detail leads me to conclude the point does not > belong in the user-facing administration documentation. Again, you have a point there. However, I find that detail useful, as it explains the user-facing behavior. Anyway, I don't think it is the job of this patch to remove that pre-existing detail. > > Are you advocating for adding a mention of "\pset null" to every backslash > > command > > that displays privileges? That is excessive, in my opinion. > > Yes, I am. I suppose the argument that any user of those commands is expected > to have read the ddl/privileges chapter would suffice for me though. Thanks. Then let's leave it like that. > My point with the second paragraph is that we could, instead of documenting > the > caveat about null printing as empty strings is to instead issue an implicit > "\pset null ''" as part of these commands, and a "\pset null" afterward, > conditioned upon it not already being set to a non-empty value. IOW, the > special-casing we do today but actually do it in a way that distinguishes the > two cases instead of forcing them to be indistinguishable. -1 The whole point of this patch is to make psql behave consistently with respect to NULLs in meta-commands. Your suggestion would subvert that idea. Yours, Laurenz Albe
Re: Fix output of zero privileges in psql
On Mon, Oct 9, 2023 at 1:29 AM Laurenz Albe wrote: > On Sun, 2023-10-08 at 19:58 -0700, David G. Johnston wrote: > > > The built-in default privileges are only in effect if the object has not > been > > the target of a GRANT or REVOKE and also has not had its default > privileges > > modified using ALTER DEFAULT PRIVILEGES. (???: if it is possible to > revert > > back to the state of built-in privileges that would need to be described > here.) > > I don't think that we need to mention ALTER DEFAULT PRIVILEGES there. If > the default privileges have been altered, the ACL will not be stored as > NULL in the catalogs. > It's already mentioned there, I just felt bringing the mention of ADP and grant/revoke both invalidating the built-in default privileges closer together would be an improvement. > > > The above removes the parenthetical regarding null in the catalogs, this > is > > intentional as it seems that the goal here is to use psql instead of the > > catalogs and adding its use of null being printed as the empty string > just > > seems likely to add confusion. > > To me, mentioning the default privileges are stored as NULLs in the > catalogs > is not an invitation to view the privileges with catalog queries, but > information about implementation details that explains why default > privileges > are displayed the way they are. > Calling it an implementation detail leads me to conclude the point does not belong in the user-facing administration documentation. > > > > > Perhaps it would also be good to mention this in the psql > documentation. > > > > We've chosen a poor default and I'd rather inform the user of specific > meta-commands > > to be wary of this poor default and change it at the point they will be > learning > > about the meta-commands adversely affected. > > > > That said, I'd be willing to document that these commands, because they > are affected > > by empty string versus null, require a non-empty-string value for \pset > null and will > > choose the string '' for the duration of the meta-command's > execution if the > > user's setting is incompatible. > > I am not certain I understood you correctly. > Are you advocating for adding a mention of "\pset null" to every backslash > command > that displays privileges? That is excessive, in my opinion. > Yes, I am. I suppose the argument that any user of those commands is expected to have read the ddl/privileges chapter would suffice for me though. My point with the second paragraph is that we could, instead of documenting the caveat about null printing as empty strings is to instead issue an implicit "\pset null ''" as part of these commands, and a "\pset null" afterward, conditioned upon it not already being set to a non-empty value. IOW, the special-casing we do today but actually do it in a way that distinguishes the two cases instead of forcing them to be indistinguishable. David J.
Re: Fix output of zero privileges in psql
On Sun, 2023-10-08 at 19:58 -0700, David G. Johnston wrote: > For us, I would suggest the following wording: > > In addition to the situation of printing all acl items, the Access and Column > privileges columns report two other situations specially. In the rare case > where all privileges for an object have been explicitly removed, including > from the owner and PUBLIC, (i.e., has empty privileges) these columns will > display NULL. The other case is where the built-in default privileges are > in effect, in which case these columns will display the empty string. > (Note that by default psql will print NULL as an empty string, so in order > to visually distinguish these two cases you will need to issue the \pset null > meta-command and choose some other string to print for NULLs). Built-in > default privileges include all privileges for the owner, as well as those > granted to PUBLIC per for relevant object types as described above. That doesn't look like an improvement over the latest patches to me. > The built-in default privileges are only in effect if the object has not been > the target of a GRANT or REVOKE and also has not had its default privileges > modified using ALTER DEFAULT PRIVILEGES. (???: if it is possible to revert > back to the state of built-in privileges that would need to be described > here.) I don't think that we need to mention ALTER DEFAULT PRIVILEGES there. If the default privileges have been altered, the ACL will not be stored as NULL in the catalogs. > The above removes the parenthetical regarding null in the catalogs, this is > intentional as it seems that the goal here is to use psql instead of the > catalogs and adding its use of null being printed as the empty string just > seems likely to add confusion. To me, mentioning the default privileges are stored as NULLs in the catalogs is not an invitation to view the privileges with catalog queries, but information about implementation details that explains why default privileges are displayed the way they are. > We probably should add the two terms to the glossary: > > empty privileges: all privileges explicitly revoked from the owner and PUBLIC > (where applicable), and none otherwise granted. > > built-in default privileges: owner having all privileges and no privileges > granted or removed via ALTER DEFAULT PRIVILEGES "Empty privileges" are too unimportant to warrant an index entry. I can see the value of an index entry privilege default Done in the attached v5 of the patch, even though this is not really the business of this patch. > > > Perhaps it would also be good to mention this in the psql documentation. > > We've chosen a poor default and I'd rather inform the user of specific > meta-commands > to be wary of this poor default and change it at the point they will be > learning > about the meta-commands adversely affected. > > That said, I'd be willing to document that these commands, because they are > affected > by empty string versus null, require a non-empty-string value for \pset null > and will > choose the string '' for the duration of the meta-command's execution > if the > user's setting is incompatible. I am not certain I understood you correctly. Are you advocating for adding a mention of "\pset null" to every backslash command that displays privileges? That is excessive, in my opinion. Yours, Laurenz Albe From 2afe3cbf674e163c146ea29582f7aa3839bd184d Mon Sep 17 00:00:00 2001 From: Laurenz Albe Date: Mon, 9 Oct 2023 10:27:58 +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. Add this missing detail to the documentation. In passing, add entry for "privileges, default" to the index. Discussion: https://postgr.es/m/96d6885a-5e25-9ae8-4a1a-d7e557a5fe9c%40mtneva.com --- doc/src/sgml/ddl.sgml | 14 --- doc/src/sgml/ref/psql-ref.sgml | 4 +++- src/bin/psql/describe.c| 43 -- 3 files changed, 14 insertions(+), 47 deletions(-) diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml index 075ff32991..76e62250e4 100644 --- a/doc/src/sgml/ddl.sgml +++ b/doc/src/sgml/ddl.sgml @@ -1737,6 +1737,11 @@ ALTER TABLE products RENAME TO items; ACL + + privilege + default + + 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. - + PostgreSQL grants privileges on some types of objects to PUBLIC by default when the objects are created. No privileges
Re: Fix output of zero privileges in psql
On Mon, 2023-10-09 at 03:53 +0200, Erik Wienhold wrote:
> On 2023-10-08 06:14 +0200, Laurenz Albe write:
> > On Sat, 2023-10-07 at 20:41 +0200, Erik Wienhold wrote:
> > > > If you are happy enough with my patch, shall we mark it as ready for
> > > > committer?
> > >
> > > I amended your patch to also document the effect of \pset null in this
> > > case. See the attached v2.
> >
> > +1
> >
> > If you mention in ddl.sgml that you can use "\pset null" to distinguish
> > default from no privileges, you should mention that this only works with
> > psql. Many people out there don't use psql.
>
> I don't think this is necessary because that section in ddl.sgml is
> already about psql and \dp.
You are right.
> > Also, I'm not sure if "zero privileges" will be readily understood by
> > everybody. Perhaps "no privileges at all, even for the object owner"
> > would be a better wording.
>
> Changed in v3 to "empty privileges" with an explanation that this means
> "no privileges at all, even for the object owner".
Looks good.
> > Perhaps it would also be good to mention this in the psql documentation.
>
> Just once under \pset null with a reference to section 5.7? Something
> like "This is also useful for distinguishing default privileges from
> empty privileges as explained in Section 5.7."
>
> Or instead under every command affected by this change? \dp and \ddp
> already have such a reference ("The meaning of the privilege display is
> explained in Section 5.7.")
>
> I prefer the first one because it's less effort ;) Also done in v3.
I think that sufficient.
I tinkered a bit with your documentation. For example, the suggestion to
"\pset null" seemed to be in an inappropriate place. Tell me what you think.
Yours,
Laurenz Albe
From 2be3e63a6330cbf7767e19f86940ae97f87bf5f2 Mon Sep 17 00:00:00 2001
From: Laurenz Albe
Date: Mon, 9 Oct 2023 09:49:20 +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. Add this missing detail to the documentation.
Discussion: https://postgr.es/m/96d6885a-5e25-9ae8-4a1a-d7e557a5fe9c%40mtneva.com
---
doc/src/sgml/ddl.sgml | 7 --
doc/src/sgml/ref/psql-ref.sgml | 4 +++-
src/bin/psql/describe.c| 43 --
3 files changed, 8 insertions(+), 46 deletions(-)
diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index 075ff32991..52f17e79ed 100644
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -2353,8 +2353,11 @@ GRANT SELECT (col1), UPDATE (col1) ON mytable TO miriam_rw;
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). Default
- privileges always include all privileges for the owner, and can include
+ privileges entry in the relevant system catalog is null) or empty privileges
+ (that is, no privileges at all, even for the object owner a rare
+ occurrence). One way to distinguish default privileges from empty privileges
+ is to set \pset null '(default)'.
+ Default privileges always include all privileges for the owner, and can include
some privileges for PUBLIC depending on the object
type, as explained above. The first GRANT
or REVOKE on an object will instantiate the default
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index d94e3cacfc..7cd12eb867 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -3079,7 +3079,9 @@ lo_import 152801
Sets the string to be printed in place of a null value.
The default is to print nothing, which can easily be mistaken for
an empty string. For example, one might prefer \pset null
- '(null)'.
+ '(null)'. Setting a non-empty null display string will
+ help to distinguish between default and empty privileges, as
+ explained in .
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;
@@
Re: Fix output of zero privileges in psql
On Sun, Oct 8, 2023 at 6:55 PM Erik Wienhold wrote:
> On 2023-10-08 06:14 +0200, Laurenz Albe write:
> > On Sat, 2023-10-07 at 20:41 +0200, Erik Wienhold wrote:
> > > > If you are happy enough with my patch, shall we mark it as ready for
> > > > committer?
> > >
> > > I amended your patch to also document the effect of \pset null in this
> > > case. See the attached v2.
> >
> > +1
> >
> > If you mention in ddl.sgml that you can use "\pset null" to distinguish
> > default from no privileges, you should mention that this only works with
> > psql. Many people out there don't use psql.
>
> I don't think this is necessary because that section in ddl.sgml is
> already about psql and \dp.
>
I agree that we are simply detailing how psql makes this information
available to the reader and leave users of other clients on their own to
figure out their own methods - which many clients probably have handled for
them anyway.
For us, I would suggest the following wording:
In addition to the situation of printing all acl items, the Access and
Column privileges columns report two other situations specially. In the
rare case where all privileges for an object have been explicitly removed,
including from the owner and PUBLIC, (i.e., has empty privileges) these
columns will display NULL. The other case is where the built-in default
privileges are in effect, in which case these columns will display the
empty string. (Note that by default psql will print NULL as an empty
string, so in order to visually distinguish these two cases you will need
to issue the \pset null meta-command and choose some other string to print
for NULLs). Built-in default privileges include all privileges for the
owner, as well as those granted to PUBLIC per for relevant object types as
described above. The built-in default privileges are only in effect if the
object has not been the target of a GRANT or REVOKE and also has not had
its default privileges modified using ALTER DEFAULT PRIVILEGES. (???: if it
is possible to revert back to the state of built-in privileges that would
need to be described here.)
The above removes the parenthetical regarding null in the catalogs, this is
intentional as it seems that the goal here is to use psql instead of the
catalogs and adding its use of null being printed as the empty string just
seems likely to add confusion.
> > Also, I'm not sure if "zero privileges" will be readily understood by
> > everybody. Perhaps "no privileges at all, even for the object owner"
> > would be a better wording.
>
> Changed in v3 to "empty privileges" with an explanation that this means
> "no privileges at all, even for the object owner".
>
+1
We probably should add the two terms to the glossary:
empty privileges: all privileges explicitly revoked from the owner and
PUBLIC (where applicable), and none otherwise granted.
built-in default privileges: owner having all privileges and no privileges
granted or removed via ALTER DEFAULT PRIVILEGES
> > Perhaps it would also be good to mention this in the psql documentation.
>
> Just once under \pset null with a reference to section 5.7? Something
> like "This is also useful for distinguishing default privileges from
> empty privileges as explained in Section 5.7."
>
> Or instead under every command affected by this change? \dp and \ddp
> already have such a reference ("The meaning of the privilege display is
> explained in Section 5.7.")
>
> I prefer the first one because it's less effort ;) Also done in v3.
>
We've chosen a poor default and I'd rather inform the user of specific
meta-commands to be wary of this poor default and change it at the point
they will be learning about the meta-commands adversely affected.
That said, I'd be willing to document that these commands, because they are
affected by empty string versus null, require a non-empty-string value for
\pset null and will choose the string '' for the duration of the
meta-command's execution if the user's setting is incompatible.
David J.
Re: Fix output of zero privileges in psql
On 2023-10-08 06:14 +0200, Laurenz Albe write:
> On Sat, 2023-10-07 at 20:41 +0200, Erik Wienhold wrote:
> > > If you are happy enough with my patch, shall we mark it as ready for
> > > committer?
> >
> > I amended your patch to also document the effect of \pset null in this
> > case. See the attached v2.
>
> +1
>
> If you mention in ddl.sgml that you can use "\pset null" to distinguish
> default from no privileges, you should mention that this only works with
> psql. Many people out there don't use psql.
I don't think this is necessary because that section in ddl.sgml is
already about psql and \dp.
> Also, I'm not sure if "zero privileges" will be readily understood by
> everybody. Perhaps "no privileges at all, even for the object owner"
> would be a better wording.
Changed in v3 to "empty privileges" with an explanation that this means
"no privileges at all, even for the object owner".
> Perhaps it would also be good to mention this in the psql documentation.
Just once under \pset null with a reference to section 5.7? Something
like "This is also useful for distinguishing default privileges from
empty privileges as explained in Section 5.7."
Or instead under every command affected by this change? \dp and \ddp
already have such a reference ("The meaning of the privilege display is
explained in Section 5.7.")
I prefer the first one because it's less effort ;) Also done in v3.
--
Erik
>From b7ddd4daa87baab83ad7e857c996b5a4a0b3ffa7 Mon Sep 17 00:00:00 2001
From: Laurenz Albe
Date: Fri, 6 Oct 2023 22:12:33 +0200
Subject: [PATCH v3] 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. Add this missing detail to the documentation.
Discussion:
https://postgr.es/m/96d6885a-5e25-9ae8-4a1a-d7e557a5fe9c%40mtneva.com
---
doc/src/sgml/ddl.sgml | 7 --
doc/src/sgml/ref/psql-ref.sgml | 3 ++-
src/bin/psql/describe.c| 43 --
3 files changed, 7 insertions(+), 46 deletions(-)
diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index 075ff32991..436d655d3c 100644
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -2353,8 +2353,9 @@ GRANT SELECT (col1), UPDATE (col1) ON mytable TO
miriam_rw;
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). Default
- privileges always include all privileges for the owner, and can include
+ privileges entry in the relevant system catalog is null) or empty privileges
+ (that is, no privileges at all, even for the object owner).
+ Default privileges always include all privileges for the owner, and can
include
some privileges for PUBLIC depending on the object
type, as explained above. The first GRANT
or REVOKE on an object will instantiate the default
@@ -2362,6 +2363,8 @@ GRANT SELECT (col1), UPDATE (col1) ON mytable TO
miriam_rw;
example, miriam=arwdDxt/miriam) and then modify them
per the specified request. Similarly, entries are shown in Column
privileges only for columns with nondefault privileges.
+ You can, for example, set \pset null '(default)' to
+ distinguish default privileges from empty privileges.
(Note: for this purpose, default privileges always means
the built-in default privileges for the object's type. An object whose
privileges have been affected by an ALTER DEFAULT
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index d94e3cacfc..966697eb50 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -3079,7 +3079,8 @@ lo_import 152801
Sets the string to be printed in place of a null value.
The default is to print nothing, which can easily be mistaken for
an empty string. For example, one might prefer \pset null
- '(null)'.
+ '(null)'. This is also useful for distinguishing default
+ privileges from empty privileges as explained in .
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;
-
Re: Fix output of zero privileges in psql
On Sat, 2023-10-07 at 20:41 +0200, Erik Wienhold wrote: > > If you are happy enough with my patch, shall we mark it as ready for > > committer? > > I amended your patch to also document the effect of \pset null in this > case. See the attached v2. +1 If you mention in ddl.sgml that you can use "\pset null" to distinguish default from no privileges, you should mention that this only works with psql. Many people out there don't use psql. Also, I'm not sure if "zero privileges" will be readily understood by everybody. Perhaps "no privileges at all, even for the object owner" would be a better wording. Perhaps it would also be good to mention this in the psql documentation. Yours, Laurenz Albe
Re: Fix output of zero privileges in psql
On 2023-10-07 14:29 +0200, Laurenz Albe write: > On Sat, 2023-10-07 at 05:07 +0200, Erik Wienhold wrote: > > On 2023-10-06 22:32 +0200, Laurenz Albe write: > > > On Sun, 2023-09-17 at 21:31 +0200, Erik Wienhold wrote: > > > > I wrote a patch to change psql's display of zero privileges after a > > > > user's > > > > reported confusion with the psql output for zero vs. default privileges > > > > [1]. > > > > Admittedly, zero privileges is a rare use case [2] but I think psql > > > > should not > > > > confuse the user in the off chance that this happens. > > > > > > > > [1] > > > > https://www.postgresql.org/message-id/efdd465d-a795-6188-7f71-7cdb4b2be031%40mtneva.com > > > > [2] > > > > https://www.postgresql.org/message-id/31246.1693337238%40sss.pgh.pa.us > > > > > > Reading that thread, I had the impression that there was more support for > > > honoring "\pset null" rather than unconditionally displaying "(none)". > > > > For example with your patch applied: > > > > create table t1 (a int); > > create table t2 (a int); > > create table t3 (a int); > > > > revoke all on t2 from :USER; > > > > \pset null > > \dp t1|t2|t3 > > Access privileges > > Schema | Name | Type | Access privileges | Column privileges | > > Policies > > > > +--+---+---+---+-- > > public | t1 | table | | | > > public | t2 | table | | | > > public | t3 | table | | | > > (3 rows) > > > > Instead of only displaying the zero privileges with my patch and default > > \pset null: > > > > \pset null '' > > \dp t1|t2|t3 > > Access privileges > > Schema | Name | Type | Access privileges | Column privileges | > > Policies > > > > +--+---+---+---+-- > > public | t1 | table | | | > > public | t2 | table | (none) | | > > public | t3 | table | | | > > (3 rows) > > > > I guess if most tables have any non-default privileges then both > > solutions are equally good. > > It is a tough call. > > For somebody who knows PostgreSQL well enough to know that default > privileges are represented by NULL values, my solution is probably > more appealing. > > It seems that we both had the goal of distinguishing the cases of > default and zero privileges, but for a beginner, both versions are > confusing. better would probably be > > Access privileges > Schema | Name | Type | Access privileges | Column privileges | Policies > +--+---+---+---+-- > public | t1 | table | default | default | > public | t2 | table | | default | > public | t3 | table | default | default | Ah yes. The problem seems to be more with default privileges producing no output right now. I was just focusing on the zero privs edge case. > The disadvantage of this (and the advantage of my proposal) is that it > might confuse experienced users (and perhaps automated tools) if the > output changes too much. I agree that your patch is less invasive under default settings. But is the output of meta commands considered part of the interface where we need to be cautious about not breaking clients? I've written quite a few scripts that parse results from psql's stdout, but always with simple queries to have control over columns and the formatting of values. I always expect meta command output to change with the next release because to me they look more like a human-readable interface, e.g. the localizable header which of course one can still hide with --tuples-only. > > > The simple attached patch does it like that. What do you think? > > > > LGTM. > > If you are happy enough with my patch, shall we mark it as ready for > committer? I amended your patch to also document the effect of \pset null in this case. See the attached v2. > Or do you want to have a stab at something like I suggested above? Not right now if the user can just use \pset null 'default' with your patch. -- Erik >From a3ea9549a4467fb4ab186d652e0afec6adc2f725 Mon Sep 17 00:00:00 2001 From: Laurenz Albe Date: Fri, 6 Oct 2023 22:12:33 +0200 Subject: [PATCH v2] 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
Re: Fix output of zero privileges in psql
On Sat, 2023-10-07 at 05:07 +0200, Erik Wienhold wrote: > On 2023-10-06 22:32 +0200, Laurenz Albe write: > > On Sun, 2023-09-17 at 21:31 +0200, Erik Wienhold wrote: > > > I wrote a patch to change psql's display of zero privileges after a user's > > > reported confusion with the psql output for zero vs. default privileges > > > [1]. > > > Admittedly, zero privileges is a rare use case [2] but I think psql > > > should not > > > confuse the user in the off chance that this happens. > > > > > > [1] > > > https://www.postgresql.org/message-id/efdd465d-a795-6188-7f71-7cdb4b2be031%40mtneva.com > > > [2] https://www.postgresql.org/message-id/31246.1693337238%40sss.pgh.pa.us > > > > Reading that thread, I had the impression that there was more support for > > honoring "\pset null" rather than unconditionally displaying "(none)". > > For example with your patch applied: > > create table t1 (a int); > create table t2 (a int); > create table t3 (a int); > > revoke all on t2 from :USER; > > \pset null > \dp t1|t2|t3 > Access privileges > Schema | Name | Type | Access privileges | Column privileges | > Policies > > +--+---+---+---+-- > public | t1 | table | | | > public | t2 | table | | | > public | t3 | table | | | > (3 rows) > > Instead of only displaying the zero privileges with my patch and default > \pset null: > > \pset null '' > \dp t1|t2|t3 > Access privileges > Schema | Name | Type | Access privileges | Column privileges | > Policies > > +--+---+---+---+-- > public | t1 | table | | | > public | t2 | table | (none) | | > public | t3 | table | | | > (3 rows) > > I guess if most tables have any non-default privileges then both > solutions are equally good. It is a tough call. For somebody who knows PostgreSQL well enough to know that default privileges are represented by NULL values, my solution is probably more appealing. It seems that we both had the goal of distinguishing the cases of default and zero privileges, but for a beginner, both versions are confusing. better would probably be Access privileges Schema | Name | Type | Access privileges | Column privileges | Policies +--+---+---+---+-- public | t1 | table | default | default | public | t2 | table | | default | public | t3 | table | default | default | The disadvantage of this (and the advantage of my proposal) is that it might confuse experienced users (and perhaps automated tools) if the output changes too much. > > The simple attached patch does it like that. What do you think? > > LGTM. If you are happy enough with my patch, shall we mark it as ready for committer? Or do you want to have a stab at something like I suggested above? Yours, Laurenz Albe
Re: Fix output of zero privileges in psql
On 2023-10-06 22:32 +0200, Laurenz Albe write: > On Sun, 2023-09-17 at 21:31 +0200, Erik Wienhold wrote: > > I wrote a patch to change psql's display of zero privileges after a user's > > reported confusion with the psql output for zero vs. default privileges [1]. > > Admittedly, zero privileges is a rare use case [2] but I think psql should > > not > > confuse the user in the off chance that this happens. > > > > With this change psql now prints "(none)" for zero privileges instead of > > nothing. This affects the following meta commands: > > > > \db+ \dD+ \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 array. Using > > \pset null '(null)' as a workaround to spot default privileges does not work > > because the meta commands ignore 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 those privileges are > > reset > > to NULL 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. > > > > [1] > > https://www.postgresql.org/message-id/efdd465d-a795-6188-7f71-7cdb4b2be031%40mtneva.com > > [2] https://www.postgresql.org/message-id/31246.1693337238%40sss.pgh.pa.us > > Reading that thread, I had the impression that there was more support for > honoring "\pset null" rather than unconditionally displaying "(none)". I took Tom's response in the -general thread to mean that we could fix \pset null also as a "nice to have" but not as a solution to the display of zero privileges. Only fixing \pset null has one drawback IMO because it only affects how default privileges (more common) are printed. The edge case of zero privileges (less common) gets lost in a bunch of NULL output. And I assume most users change the default \pset null to some non-empty string in their psqlrc (I do). For example with your patch applied: create table t1 (a int); create table t2 (a int); create table t3 (a int); revoke all on t2 from :USER; \pset null \dp t1|t2|t3 Access privileges Schema | Name | Type | Access privileges | Column privileges | Policies +--+---+---+---+-- public | t1 | table | | | public | t2 | table | | | public | t3 | table | | | (3 rows) Instead of only displaying the zero privileges with my patch and default \pset null: \pset null '' \dp t1|t2|t3 Access privileges Schema | Name | Type | Access privileges | Column privileges | Policies +--+---+---+---+-- public | t1 | table | | | public | t2 | table | (none)| | public | t3 | table | | | (3 rows) I guess if most tables have any non-default privileges then both solutions are equally good. > The simple attached patch does it like that. What do you think? LGTM. -- Erik
Re: Fix output of zero privileges in psql
On Sun, 2023-09-17 at 21:31 +0200, Erik Wienhold wrote:
> I wrote a patch to change psql's display of zero privileges after a user's
> reported confusion with the psql output for zero vs. default privileges [1].
> Admittedly, zero privileges is a rare use case [2] but I think psql should not
> confuse the user in the off chance that this happens.
>
> With this change psql now prints "(none)" for zero privileges instead of
> nothing. This affects the following meta commands:
>
> \db+ \dD+ \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 array. Using
> \pset null '(null)' as a workaround to spot default privileges does not work
> because the meta commands ignore 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 those privileges are
> reset
> to NULL 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.
>
> [1]
> https://www.postgresql.org/message-id/efdd465d-a795-6188-7f71-7cdb4b2be031%40mtneva.com
> [2] https://www.postgresql.org/message-id/31246.1693337238%40sss.pgh.pa.us
Reading that thread, I had the impression that there was more support for
honoring "\pset null" rather than unconditionally displaying "(none)".
The simple attached patch does it like that. What do you think?
Yours,
Laurenz Albe
From 6c67f15f011ddf1e309cb7e84580b266d674a1e2 Mon Sep 17 00:00:00 2001
From: Laurenz Albe
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(, _("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(, _("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
Re: Fix output of zero privileges in psql
On 17/09/2023 21:31 CEST Erik Wienhold wrote: > This affects the following meta commands: > > \db+ \dD+ \df+ \dL+ \dl+ \dn+ \dp \dT+ \l also \des+ and \dew+ -- Erik
Fix output of zero privileges in psql
I wrote a patch to change psql's display of zero privileges after a user's
reported confusion with the psql output for zero vs. default privileges [1].
Admittedly, zero privileges is a rare use case [2] but I think psql should not
confuse the user in the off chance that this happens.
With this change psql now prints "(none)" for zero privileges instead of
nothing. This affects the following meta commands:
\db+ \dD+ \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 array. Using
\pset null '(null)' as a workaround to spot default privileges does not work
because the meta commands ignore 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 those privileges are reset
to NULL 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.
[1]
https://www.postgresql.org/message-id/efdd465d-a795-6188-7f71-7cdb4b2be031%40mtneva.com
[2] https://www.postgresql.org/message-id/31246.1693337238%40sss.pgh.pa.us
--
ErikFrom 16af995acb4c0e5fb5981308610a76b7e87c3358 Mon Sep 17 00:00:00 2001
From: Erik Wienhold
Date: Sun, 17 Sep 2023 20:54:48 +0200
Subject: [PATCH v1] Fix output of zero privileges in psql
Print "(none)" for zero privileges instead of nothing so that the user
is able to distinguish zero from default privileges. This affects the
following meta commands:
\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 array.
Using \pset null '(null)' as a workaround to spot default privileges
does not work because the meta commands ignore 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 those
privileges are reset to NULL 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.
---
src/bin/psql/describe.c | 6 +-
src/test/regress/expected/privileges.out | 93
src/test/regress/sql/privileges.sql | 46
3 files changed, 144 insertions(+), 1 deletion(-)
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index bac94a338c..154d244d97 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -6718,7 +6718,11 @@ static void
printACLColumn(PQExpBuffer buf, const char *colname)
{
appendPQExpBuffer(buf,
- "pg_catalog.array_to_string(%s, E'\\n') AS \"%s\"",
+ "CASE pg_catalog.cardinality(%s)\n"
+ " WHEN 0 THEN '%s'\n"
+ " ELSE pg_catalog.array_to_string(%s, E'\\n')\n"
+ "END AS \"%s\"",
+ colname, gettext_noop("(none)"),
colname, gettext_noop("Access privileges"));
}
diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out
index c1e610e62f..b9eb52f7b1 100644
--- a/src/test/regress/expected/privileges.out
+++ b/src/test/regress/expected/privileges.out
@@ -2895,3 +2895,96 @@ DROP SCHEMA regress_roleoption;
DROP ROLE regress_roleoption_protagonist;
DROP ROLE regress_roleoption_donor;
DROP ROLE regress_roleoption_recipient;
+-- Test zero 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
+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)
