pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedadd \dpS to psql
26+ messages / 10 participants
[nested] [flat]
Commits from this thread:
2023-01-07 d913928c9c psql: Add support for \dpS and \zS.
2022-12-14 60684dd834 Add grantable MAINTAIN privilege and pg_maintain role.
* add \dpS to psql
@ 2022-12-06 19:36 Nathan Bossart <nathandbossart@gmail.com>
0 siblings, 1 reply; 26+ messages in thread
From: Nathan Bossart @ 2022-12-06 19:36 UTC (permalink / raw)
To: pgsql-hackers
Hi hackers,
As discussed elsewhere [0], \dp doesn't show privileges on system objects,
and this behavior is not mentioned in the docs. I've attached a small
patch that adds support for the S modifier (i.e., \dpS) and the adjusts the
docs.
Thoughts?
[0] https://postgr.es/m/a2382acd-e465-85b2-9d8e-f9ed1a5a66e9%40postgrespro.ru
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
Attachments:
add_s_modifier_to_dp.patch (text/x-diff, 3.5K) download
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index d3dd638b14..406936dd1c 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -1825,14 +1825,16 @@ INSERT INTO tbl1 VALUES ($1, $2) \bind 'first value' 'second value' \g
<varlistentry>
- <term><literal>\dp [ <link linkend="app-psql-patterns"><replaceable class="parameter">pattern</replaceable></link> ]</literal></term>
+ <term><literal>\dp[S] [ <link linkend="app-psql-patterns"><replaceable class="parameter">pattern</replaceable></link> ]</literal></term>
<listitem>
<para>
Lists tables, views and sequences with their
associated access privileges.
If <replaceable class="parameter">pattern</replaceable> is
specified, only tables, views and sequences whose names match the
- pattern are listed.
+ pattern are listed. By default only user-created objects are shown;
+ supply a pattern or the <literal>S</literal> modifier to include system
+ objects.
</para>
<para>
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index de6a3a71f8..3520655dc0 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -875,7 +875,7 @@ exec_command_d(PsqlScanState scan_state, bool active_branch, const char *cmd)
success = listCollations(pattern, show_verbose, show_system);
break;
case 'p':
- success = permissionsList(pattern);
+ success = permissionsList(pattern, show_system);
break;
case 'P':
{
@@ -2831,7 +2831,7 @@ exec_command_z(PsqlScanState scan_state, bool active_branch)
char *pattern = psql_scan_slash_option(scan_state,
OT_NORMAL, NULL, true);
- success = permissionsList(pattern);
+ success = permissionsList(pattern, false);
free(pattern);
}
else
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 2eae519b1d..eb98797d67 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -1002,7 +1002,7 @@ listAllDbs(const char *pattern, bool verbose)
* \z (now also \dp -- perhaps more mnemonic)
*/
bool
-permissionsList(const char *pattern)
+permissionsList(const char *pattern, bool showSystem)
{
PQExpBufferData buf;
PGresult *res;
@@ -1121,15 +1121,12 @@ permissionsList(const char *pattern)
CppAsString2(RELKIND_FOREIGN_TABLE) ","
CppAsString2(RELKIND_PARTITIONED_TABLE) ")\n");
- /*
- * Unless a schema pattern is specified, we suppress system and temp
- * tables, since they normally aren't very interesting from a permissions
- * point of view. You can see 'em by explicit request though, eg with \z
- * pg_catalog.*
- */
+ if (!showSystem && !pattern)
+ appendPQExpBufferStr(&buf, "AND n.nspname !~ '^pg_'\n");
+
if (!validateSQLNamePattern(&buf, pattern, true, false,
"n.nspname", "c.relname", NULL,
- "n.nspname !~ '^pg_' AND pg_catalog.pg_table_is_visible(c.oid)",
+ "pg_catalog.pg_table_is_visible(c.oid)",
NULL, 3))
goto error_return;
diff --git a/src/bin/psql/describe.h b/src/bin/psql/describe.h
index bd051e09cb..58d0cf032b 100644
--- a/src/bin/psql/describe.h
+++ b/src/bin/psql/describe.h
@@ -38,7 +38,7 @@ extern bool describeRoles(const char *pattern, bool verbose, bool showSystem);
extern bool listDbRoleSettings(const char *pattern, const char *pattern2);
/* \z (or \dp) */
-extern bool permissionsList(const char *pattern);
+extern bool permissionsList(const char *pattern, bool showSystem);
/* \ddp */
extern bool listDefaultACLs(const char *pattern);
* Re: add \dpS to psql
@ 2022-12-07 07:48 Pavel Luzanov <p.luzanov@postgrespro.ru>
parent: Nathan Bossart <nathandbossart@gmail.com>
0 siblings, 2 replies; 26+ messages in thread
From: Pavel Luzanov @ 2022-12-07 07:48 UTC (permalink / raw)
To: Nathan Bossart <nathandbossart@gmail.com>; pgsql-hackers
On 06.12.2022 22:36, Nathan Bossart wrote:
> As discussed elsewhere [0], \dp doesn't show privileges on system objects,
> and this behavior is not mentioned in the docs. I've attached a small
> patch that adds support for the S modifier (i.e., \dpS) and the adjusts the
> docs.
>
> Thoughts?
>
> [0] https://postgr.es/m/a2382acd-e465-85b2-9d8e-f9ed1a5a66e9%40postgrespro.ru
A few words in support of this patch, since I was the initiator of the
discussion.
Before VACUUM, ANALYZE privileges, there was no such question.
Why check privileges on system catalog objects? But now it doesn't.
It is now possible to grant privileges on system tables,
so it should be possible to see privileges with psql commands.
However, the \dp command does not support the S modifier, which is
inconsistent.
Furthermore. The VACUUM privilege allows you to also execute VACUUM FULL.
VACUUM and VACUUM FULL are commands with similar names, but work
completely differently.
It may be worth clarifying on this page:
https://www.postgresql.org/docs/devel/ddl-priv.html
Something like: Allows VACUUM on a relation, including VACUUM FULL.
But that's not all.
There is a very similar command to VACUUM FULL with a different name -
CLUSTER.
The VACUUM privilege does not apply to the CLUSTER command. This is
probably correct.
However, the documentation for the CLUSTER command does not say
who can perform this command. I think it would be correct to add a sentence
to the Notes section
(https://www.postgresql.org/docs/devel/sql-cluster.html)
similar to the one in the VACUUM documentation:
"To cluster a table, one must ordinarily be the table's owner or a
superuser."
Ready to participate, if it seems reasonable.
--
Pavel Luzanov
Postgres Professional: https://postgrespro.com
^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: add \dpS to psql
@ 2022-12-07 22:41 Nathan Bossart <nathandbossart@gmail.com>
parent: Pavel Luzanov <p.luzanov@postgrespro.ru>
1 sibling, 0 replies; 26+ messages in thread
From: Nathan Bossart @ 2022-12-07 22:41 UTC (permalink / raw)
To: Pavel Luzanov <p.luzanov@postgrespro.ru>; +Cc: pgsql-hackers
On Wed, Dec 07, 2022 at 10:48:49AM +0300, Pavel Luzanov wrote:
> There is a very similar command to VACUUM FULL with a different name -
> CLUSTER.
> The VACUUM privilege does not apply to the CLUSTER command. This is probably
> correct.
> However, the documentation for the CLUSTER command does not say
> who can perform this command. I think it would be correct to add a sentence
> to the Notes section
> (https://www.postgresql.org/docs/devel/sql-cluster.html)
> similar to the one in the VACUUM documentation:
>
> "To cluster a table, one must ordinarily be the table's owner or a
> superuser."
I created a new thread for this:
https://postgr.es/m/20221207223924.GA4182184%40nathanxps13
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: add \dpS to psql
@ 2022-12-08 02:39 Justin Pryzby <pryzby@telsasoft.com>
parent: Pavel Luzanov <p.luzanov@postgrespro.ru>
1 sibling, 1 reply; 26+ messages in thread
From: Justin Pryzby @ 2022-12-08 02:39 UTC (permalink / raw)
To: Pavel Luzanov <p.luzanov@postgrespro.ru>; +Cc: Nathan Bossart <nathandbossart@gmail.com>; pgsql-hackers
On Wed, Dec 07, 2022 at 10:48:49AM +0300, Pavel Luzanov wrote:
> Furthermore. The VACUUM privilege allows you to also execute VACUUM FULL.
> VACUUM and VACUUM FULL are commands with similar names, but work completely
> differently.
> It may be worth clarifying on this page:
> https://www.postgresql.org/docs/devel/ddl-priv.html
>
> Something like: Allows VACUUM on a relation, including VACUUM FULL.
Since (as you said) they work completely differently, I think it'd be
more useful if vacuum_full were a separate privilege, rather than being
included in vacuum. And cluster could be allowed whenever vacuum_full
is allowed.
> There is a very similar command to VACUUM FULL with a different name -
> CLUSTER. The VACUUM privilege does not apply to the CLUSTER command.
> This is probably correct.
I think if vacuum privilege allows vacuum full, then it ought to also
allow cluster. But I suggest that it'd be even better if it doesn't
allow either, and there was a separate privilege for those.
Disclaimer: I have not been following these threads.
--
Justin
^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: add \dpS to psql
@ 2022-12-08 04:19 Nathan Bossart <nathandbossart@gmail.com>
parent: Justin Pryzby <pryzby@telsasoft.com>
0 siblings, 1 reply; 26+ messages in thread
From: Nathan Bossart @ 2022-12-08 04:19 UTC (permalink / raw)
To: Justin Pryzby <pryzby@telsasoft.com>; +Cc: Pavel Luzanov <p.luzanov@postgrespro.ru>; pgsql-hackers
On Wed, Dec 07, 2022 at 08:39:30PM -0600, Justin Pryzby wrote:
> I think if vacuum privilege allows vacuum full, then it ought to also
> allow cluster. But I suggest that it'd be even better if it doesn't
> allow either, and there was a separate privilege for those.
>
> Disclaimer: I have not been following these threads.
I haven't formed an opinion on whether VACUUM FULL should get its own bit,
but FWIW І just finished writing the first draft of a patch set to add bits
for CLUSTER, REFRESH MATERIALIZED VIEW, and REINDEX. I plan to post that
tomorrow.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: add \dpS to psql
@ 2022-12-08 04:25 Tom Lane <tgl@sss.pgh.pa.us>
parent: Nathan Bossart <nathandbossart@gmail.com>
0 siblings, 2 replies; 26+ messages in thread
From: Tom Lane @ 2022-12-08 04:25 UTC (permalink / raw)
To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: Justin Pryzby <pryzby@telsasoft.com>; Pavel Luzanov <p.luzanov@postgrespro.ru>; pgsql-hackers
Nathan Bossart <nathandbossart@gmail.com> writes:
> I haven't formed an opinion on whether VACUUM FULL should get its own bit,
> but FWIW І just finished writing the first draft of a patch set to add bits
> for CLUSTER, REFRESH MATERIALIZED VIEW, and REINDEX. I plan to post that
> tomorrow.
The fact that we just doubled the number of available bits doesn't
mean we should immediately strive to use them up. Perhaps it'd
be better to subsume these retail privileges under some generic
"maintenance action" privilege?
regards, tom lane
^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: add \dpS to psql
@ 2022-12-08 04:43 Nathan Bossart <nathandbossart@gmail.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 0 replies; 26+ messages in thread
From: Nathan Bossart @ 2022-12-08 04:43 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Justin Pryzby <pryzby@telsasoft.com>; Pavel Luzanov <p.luzanov@postgrespro.ru>; pgsql-hackers
On Wed, Dec 07, 2022 at 11:25:32PM -0500, Tom Lane wrote:
> The fact that we just doubled the number of available bits doesn't
> mean we should immediately strive to use them up. Perhaps it'd
> be better to subsume these retail privileges under some generic
> "maintenance action" privilege?
That's fine with me, but I wouldn't be surprised if there's disagreement on
how to group the commands. I certainly don't want to use up the rest of
the bits right away, but there might not be too many more existing
privileges after these three that deserve them. Perhaps I should take
inventory and offer a plan for all the remaining privileges.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: add \dpS to psql
@ 2022-12-08 04:48 Isaac Morland <isaac.morland@gmail.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 2 replies; 26+ messages in thread
From: Isaac Morland @ 2022-12-08 04:48 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Nathan Bossart <nathandbossart@gmail.com>; Justin Pryzby <pryzby@telsasoft.com>; Pavel Luzanov <p.luzanov@postgrespro.ru>; pgsql-hackers
On Wed, 7 Dec 2022 at 23:25, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Nathan Bossart <nathandbossart@gmail.com> writes:
> > I haven't formed an opinion on whether VACUUM FULL should get its own
> bit,
> > but FWIW І just finished writing the first draft of a patch set to add
> bits
> > for CLUSTER, REFRESH MATERIALIZED VIEW, and REINDEX. I plan to post that
> > tomorrow.
>
> The fact that we just doubled the number of available bits doesn't
> mean we should immediately strive to use them up. Perhaps it'd
> be better to subsume these retail privileges under some generic
> "maintenance action" privilege?
>
That was my original suggestion:
https://www.postgresql.org/message-id/CAMsGm5c4DycKBYZCypfV02s-SC8GwF%2BKeTt%3D%3DvbWrFn%2Bdz%3DKeg%...
In that message I review the history of permission bit growth. A bit later
in the discussion, I did some investigation into the history of demand for
new permission bits and I proposed calling the new privilege MAINTAIN:
https://www.postgresql.org/message-id/CAMsGm5d%3D2gi4kyKONUJyYFwen%3DbsWm4hz_KxLXkEhMmg5WSWTA%40mail...
For what it's worth, I wouldn't bother changing the format of the
permission bits to expand the pool of available bits. My previous analysis
shows that there is no vast hidden demand for new privilege bits. If we
implement MAINTAIN to control access to VACUUM, ANALYZE, REFRESH, CLUSTER,
and REINDEX, we will cover everything that I can find that has seriously
discussed on this list, and still leave 3 unused bits for future expansion.
There is even justification for stopping after this expansion: if it is
done, then schema changes (DDL) will only be able to be done by owner; data
changes (insert, update, delete, as well as triggering of automatic data
maintenance actions) will be able to be done by anybody who is granted
permission.
My guess is that if we ever do expand the privilege bit system, it should
be in a way that removes the limit entirely, replacing a bit map model with
something more like a table with one row for each individual grant, with a
field indicating which grant is involved. But that is a hypothetical future.
^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: add \dpS to psql
@ 2022-12-08 05:07 Nathan Bossart <nathandbossart@gmail.com>
parent: Isaac Morland <isaac.morland@gmail.com>
1 sibling, 2 replies; 26+ messages in thread
From: Nathan Bossart @ 2022-12-08 05:07 UTC (permalink / raw)
To: Isaac Morland <isaac.morland@gmail.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Justin Pryzby <pryzby@telsasoft.com>; Pavel Luzanov <p.luzanov@postgrespro.ru>; pgsql-hackers
On Wed, Dec 07, 2022 at 11:48:20PM -0500, Isaac Morland wrote:
> For what it's worth, I wouldn't bother changing the format of the
> permission bits to expand the pool of available bits.
7b37823 expanded AclMode to 64 bits, so we now have room for 16 additional
privileges (after the addition of VACUUM and ANALYZE in b5d6382).
> My previous analysis
> shows that there is no vast hidden demand for new privilege bits. If we
> implement MAINTAIN to control access to VACUUM, ANALYZE, REFRESH, CLUSTER,
> and REINDEX, we will cover everything that I can find that has seriously
> discussed on this list, and still leave 3 unused bits for future expansion.
If we added CLUSTER, REFRESH MATERIALIZED VIEW, and REINDEX as individual
privilege bits, we'd still have 13 remaining for future use.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: add \dpS to psql
@ 2022-12-08 05:12 Isaac Morland <isaac.morland@gmail.com>
parent: Nathan Bossart <nathandbossart@gmail.com>
1 sibling, 0 replies; 26+ messages in thread
From: Isaac Morland @ 2022-12-08 05:12 UTC (permalink / raw)
To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Justin Pryzby <pryzby@telsasoft.com>; Pavel Luzanov <p.luzanov@postgrespro.ru>; pgsql-hackers
On Thu, 8 Dec 2022 at 00:07, Nathan Bossart <nathandbossart@gmail.com>
wrote:
> On Wed, Dec 07, 2022 at 11:48:20PM -0500, Isaac Morland wrote:
> > For what it's worth, I wouldn't bother changing the format of the
> > permission bits to expand the pool of available bits.
>
> 7b37823 expanded AclMode to 64 bits, so we now have room for 16 additional
> privileges (after the addition of VACUUM and ANALYZE in b5d6382).
>
> > My previous analysis
> > shows that there is no vast hidden demand for new privilege bits. If we
> > implement MAINTAIN to control access to VACUUM, ANALYZE, REFRESH,
> CLUSTER,
> > and REINDEX, we will cover everything that I can find that has seriously
> > discussed on this list, and still leave 3 unused bits for future
> expansion.
>
> If we added CLUSTER, REFRESH MATERIALIZED VIEW, and REINDEX as individual
> privilege bits, we'd still have 13 remaining for future use.
>
I was a bit imprecise. I was comparing to the state before the recent
changes - so 12 bits used out of 16, with MAINTAIN being the 13th bit. I
think in my mind it's still approximately 2019 on some level.
^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: add \dpS to psql
@ 2022-12-08 05:15 Tom Lane <tgl@sss.pgh.pa.us>
parent: Nathan Bossart <nathandbossart@gmail.com>
1 sibling, 1 reply; 26+ messages in thread
From: Tom Lane @ 2022-12-08 05:15 UTC (permalink / raw)
To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: Isaac Morland <isaac.morland@gmail.com>; Justin Pryzby <pryzby@telsasoft.com>; Pavel Luzanov <p.luzanov@postgrespro.ru>; pgsql-hackers
Nathan Bossart <nathandbossart@gmail.com> writes:
> On Wed, Dec 07, 2022 at 11:48:20PM -0500, Isaac Morland wrote:
>> My previous analysis
>> shows that there is no vast hidden demand for new privilege bits. If we
>> implement MAINTAIN to control access to VACUUM, ANALYZE, REFRESH, CLUSTER,
>> and REINDEX, we will cover everything that I can find that has seriously
>> discussed on this list, and still leave 3 unused bits for future expansion.
> If we added CLUSTER, REFRESH MATERIALIZED VIEW, and REINDEX as individual
> privilege bits, we'd still have 13 remaining for future use.
I think the appropriate question is not "have we still got bits left?".
It should be more like "under what plausible scenario would it be useful
to grant somebody CLUSTER but not VACUUM privileges on a table?".
I'm really thinking that MAINTAIN is the right level of granularity
here. Or maybe it's worth segregating exclusive-lock from
not-exclusive-lock maintenance. But I really fail to see how it's
useful to distinguish CLUSTER from REINDEX, say.
regards, tom lane
^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: add \dpS to psql
@ 2022-12-08 11:03 Pavel Luzanov <p.luzanov@postgrespro.ru>
parent: Isaac Morland <isaac.morland@gmail.com>
1 sibling, 1 reply; 26+ messages in thread
From: Pavel Luzanov @ 2022-12-08 11:03 UTC (permalink / raw)
To: Isaac Morland <isaac.morland@gmail.com>; Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Nathan Bossart <nathandbossart@gmail.com>; Justin Pryzby <pryzby@telsasoft.com>; pgsql-hackers
On 08.12.2022 07:48, Isaac Morland wrote:
> If we implement MAINTAIN to control access to VACUUM, ANALYZE,
> REFRESH, CLUSTER, and REINDEX, we will cover everything that I can
> find that has seriously discussed on this list
I like this approach with MAINTAIN privilege. I'm trying to find any
disadvantages ... and I can't.
For the complete picture, I tried to see what other actions with the
table could *potentially* be considered as maintenance.
Here is the list:
- create|alter|drop on extended statistics objects
- alter table|index alter column set statistics
- alter table|index [re]set (storage_parameters)
- alter table|index set tablespace
- alter table alter column set storage|compression
- any actions with the TOAST table that can be performed separately from
the main table
I have to admit that the discussion has moved away from the $subject.
--
Pavel Luzanov
Postgres Professional: https://postgrespro.com
^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: add \dpS to psql
@ 2022-12-08 11:35 Alvaro Herrera <alvherre@alvh.no-ip.org>
parent: Pavel Luzanov <p.luzanov@postgrespro.ru>
0 siblings, 0 replies; 26+ messages in thread
From: Alvaro Herrera @ 2022-12-08 11:35 UTC (permalink / raw)
To: Pavel Luzanov <p.luzanov@postgrespro.ru>; +Cc: Isaac Morland <isaac.morland@gmail.com>; Tom Lane <tgl@sss.pgh.pa.us>; Nathan Bossart <nathandbossart@gmail.com>; Justin Pryzby <pryzby@telsasoft.com>; pgsql-hackers
On 2022-Dec-08, Pavel Luzanov wrote:
> For the complete picture, I tried to see what other actions with the table
> could *potentially* be considered as maintenance.
> Here is the list:
>
> - create|alter|drop on extended statistics objects
> - alter table|index alter column set statistics
> - alter table|index [re]set (storage_parameters)
> - alter table|index set tablespace
> - alter table alter column set storage|compression
> - any actions with the TOAST table that can be performed separately from the
> main table
Well, I can't see that any of these is valuable to grant separately from
the table's owner. The maintenance ones are the ones that are
interesting to run from a database-owner perspective, but these ones do
not seem to need that treatment.
If you're extremely generous you could think that ALTER .. SET STORAGE
would be reasonable to be run by the db-owner. However, that's not
something you do on an ongoing basis -- you just do it once -- so it
seems pointless.
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: add \dpS to psql
@ 2022-12-08 17:15 Nathan Bossart <nathandbossart@gmail.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 2 replies; 26+ messages in thread
From: Nathan Bossart @ 2022-12-08 17:15 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Isaac Morland <isaac.morland@gmail.com>; Justin Pryzby <pryzby@telsasoft.com>; Pavel Luzanov <p.luzanov@postgrespro.ru>; pgsql-hackers
On Thu, Dec 08, 2022 at 12:15:23AM -0500, Tom Lane wrote:
> Nathan Bossart <nathandbossart@gmail.com> writes:
>> On Wed, Dec 07, 2022 at 11:48:20PM -0500, Isaac Morland wrote:
>>> My previous analysis
>>> shows that there is no vast hidden demand for new privilege bits. If we
>>> implement MAINTAIN to control access to VACUUM, ANALYZE, REFRESH, CLUSTER,
>>> and REINDEX, we will cover everything that I can find that has seriously
>>> discussed on this list, and still leave 3 unused bits for future expansion.
>
>> If we added CLUSTER, REFRESH MATERIALIZED VIEW, and REINDEX as individual
>> privilege bits, we'd still have 13 remaining for future use.
>
> I think the appropriate question is not "have we still got bits left?".
> It should be more like "under what plausible scenario would it be useful
> to grant somebody CLUSTER but not VACUUM privileges on a table?".
>
> I'm really thinking that MAINTAIN is the right level of granularity
> here. Or maybe it's worth segregating exclusive-lock from
> not-exclusive-lock maintenance. But I really fail to see how it's
> useful to distinguish CLUSTER from REINDEX, say.
The main idea behind this work is breaking out privileges into more
granular pieces. If I want to create a role that only runs VACUUM on some
tables on the weekend, why ѕhould I have to also give it the ability to
ANALYZE, REFRESH, CLUSTER, and REINDEX? IMHO we should really let the user
decide what set of privileges makes sense for their use-case. I'm unsure
the grouping all these privileges together serves much purpose besides
preserving ACL bits.
The other reason I'm hesitant to group the privileges together is because I
suspect it will be difficult to reach agreement on how to do so, as
evidenced by past discussion [0]. That being said, I'm open to it if we
find a way that folks are happy with. For example, separating
exclusive-lock and non-exclusive-lock maintenance actions seems like a
reasonable idea (which perhaps is an argument for moving VACUUM FULL out of
the VACUUM privilege).
[0] https://postgr.es/m/67a1d667e8ec228b5e07f232184c80348c5d93f4.camel%40j-davis.com
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: add \dpS to psql
@ 2022-12-08 18:39 Nathan Bossart <nathandbossart@gmail.com>
parent: Nathan Bossart <nathandbossart@gmail.com>
1 sibling, 0 replies; 26+ messages in thread
From: Nathan Bossart @ 2022-12-08 18:39 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Isaac Morland <isaac.morland@gmail.com>; Justin Pryzby <pryzby@telsasoft.com>; Pavel Luzanov <p.luzanov@postgrespro.ru>; pgsql-hackers
I've created a new thread for making CLUSTER, REFRESH MATERIALIZED VIEW,
and REINDEX grantable:
https://postgr.es/m/20221208183707.GA55474%40nathanxps13
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: add \dpS to psql
@ 2022-12-09 04:36 Michael Paquier <michael@paquier.xyz>
parent: Nathan Bossart <nathandbossart@gmail.com>
1 sibling, 1 reply; 26+ messages in thread
From: Michael Paquier @ 2022-12-09 04:36 UTC (permalink / raw)
To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Isaac Morland <isaac.morland@gmail.com>; Justin Pryzby <pryzby@telsasoft.com>; Pavel Luzanov <p.luzanov@postgrespro.ru>; pgsql-hackers
On Thu, Dec 08, 2022 at 09:15:03AM -0800, Nathan Bossart wrote:
> The main idea behind this work is breaking out privileges into more
> granular pieces. If I want to create a role that only runs VACUUM on some
> tables on the weekend, why ѕhould I have to also give it the ability to
> ANALYZE, REFRESH, CLUSTER, and REINDEX? IMHO we should really let the user
> decide what set of privileges makes sense for their use-case. I'm unsure
> the grouping all these privileges together serves much purpose besides
> preserving ACL bits.
Hmm. I'd like to think that we should keep a frugal mind here. More
bits are now available, but it does not strike me as a good idea to
force their usage more than necessary, so grouping all these no-quite
DDL commands into the same bag does not sound that bad to me.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: add \dpS to psql
@ 2022-12-09 18:40 Nathan Bossart <nathandbossart@gmail.com>
parent: Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 26+ messages in thread
From: Nathan Bossart @ 2022-12-09 18:40 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Isaac Morland <isaac.morland@gmail.com>; Justin Pryzby <pryzby@telsasoft.com>; Pavel Luzanov <p.luzanov@postgrespro.ru>; pgsql-hackers
On Fri, Dec 09, 2022 at 01:36:03PM +0900, Michael Paquier wrote:
> On Thu, Dec 08, 2022 at 09:15:03AM -0800, Nathan Bossart wrote:
>> The main idea behind this work is breaking out privileges into more
>> granular pieces. If I want to create a role that only runs VACUUM on some
>> tables on the weekend, why ѕhould I have to also give it the ability to
>> ANALYZE, REFRESH, CLUSTER, and REINDEX? IMHO we should really let the user
>> decide what set of privileges makes sense for their use-case. I'm unsure
>> the grouping all these privileges together serves much purpose besides
>> preserving ACL bits.
>
> Hmm. I'd like to think that we should keep a frugal mind here. More
> bits are now available, but it does not strike me as a good idea to
> force their usage more than necessary, so grouping all these no-quite
> DDL commands into the same bag does not sound that bad to me.
Okay, it seems I am outnumbered. I will work on updating the patch to add
an ACL_MAINTAIN bit and a pg_maintain_all_tables predefined role.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: add \dpS to psql
@ 2022-12-09 18:44 Nathan Bossart <nathandbossart@gmail.com>
parent: Nathan Bossart <nathandbossart@gmail.com>
0 siblings, 1 reply; 26+ messages in thread
From: Nathan Bossart @ 2022-12-09 18:44 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Isaac Morland <isaac.morland@gmail.com>; Justin Pryzby <pryzby@telsasoft.com>; Pavel Luzanov <p.luzanov@postgrespro.ru>; pgsql-hackers
On Fri, Dec 09, 2022 at 10:40:55AM -0800, Nathan Bossart wrote:
> On Fri, Dec 09, 2022 at 01:36:03PM +0900, Michael Paquier wrote:
>> On Thu, Dec 08, 2022 at 09:15:03AM -0800, Nathan Bossart wrote:
>>> The main idea behind this work is breaking out privileges into more
>>> granular pieces. If I want to create a role that only runs VACUUM on some
>>> tables on the weekend, why ѕhould I have to also give it the ability to
>>> ANALYZE, REFRESH, CLUSTER, and REINDEX? IMHO we should really let the user
>>> decide what set of privileges makes sense for their use-case. I'm unsure
>>> the grouping all these privileges together serves much purpose besides
>>> preserving ACL bits.
>>
>> Hmm. I'd like to think that we should keep a frugal mind here. More
>> bits are now available, but it does not strike me as a good idea to
>> force their usage more than necessary, so grouping all these no-quite
>> DDL commands into the same bag does not sound that bad to me.
>
> Okay, it seems I am outnumbered. I will work on updating the patch to add
> an ACL_MAINTAIN bit and a pg_maintain_all_tables predefined role.
Any thoughts on $SUBJECT?
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: add \dpS to psql
@ 2022-12-12 12:01 Andrew Dunstan <andrew@dunslane.net>
parent: Nathan Bossart <nathandbossart@gmail.com>
0 siblings, 1 reply; 26+ messages in thread
From: Andrew Dunstan @ 2022-12-12 12:01 UTC (permalink / raw)
To: Nathan Bossart <nathandbossart@gmail.com>; Michael Paquier <michael@paquier.xyz>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Isaac Morland <isaac.morland@gmail.com>; Justin Pryzby <pryzby@telsasoft.com>; Pavel Luzanov <p.luzanov@postgrespro.ru>; pgsql-hackers
On 2022-12-09 Fr 13:44, Nathan Bossart wrote:
> On Fri, Dec 09, 2022 at 10:40:55AM -0800, Nathan Bossart wrote:
>> On Fri, Dec 09, 2022 at 01:36:03PM +0900, Michael Paquier wrote:
>>> On Thu, Dec 08, 2022 at 09:15:03AM -0800, Nathan Bossart wrote:
>>>> The main idea behind this work is breaking out privileges into more
>>>> granular pieces. If I want to create a role that only runs VACUUM on some
>>>> tables on the weekend, why ѕhould I have to also give it the ability to
>>>> ANALYZE, REFRESH, CLUSTER, and REINDEX? IMHO we should really let the user
>>>> decide what set of privileges makes sense for their use-case. I'm unsure
>>>> the grouping all these privileges together serves much purpose besides
>>>> preserving ACL bits.
>>> Hmm. I'd like to think that we should keep a frugal mind here. More
>>> bits are now available, but it does not strike me as a good idea to
>>> force their usage more than necessary, so grouping all these no-quite
>>> DDL commands into the same bag does not sound that bad to me.
>> Okay, it seems I am outnumbered. I will work on updating the patch to add
>> an ACL_MAINTAIN bit and a pg_maintain_all_tables predefined role.
> Any thoughts on $SUBJECT?
Yeah, the discussion got way off into the weeds here. I think the
original proposal seems reasonable. Please add it to the next CF if you
haven't already.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: add \dpS to psql
@ 2022-12-12 17:18 Nathan Bossart <nathandbossart@gmail.com>
parent: Andrew Dunstan <andrew@dunslane.net>
0 siblings, 1 reply; 26+ messages in thread
From: Nathan Bossart @ 2022-12-12 17:18 UTC (permalink / raw)
To: Andrew Dunstan <andrew@dunslane.net>; +Cc: Michael Paquier <michael@paquier.xyz>; Tom Lane <tgl@sss.pgh.pa.us>; Isaac Morland <isaac.morland@gmail.com>; Justin Pryzby <pryzby@telsasoft.com>; Pavel Luzanov <p.luzanov@postgrespro.ru>; pgsql-hackers
On Mon, Dec 12, 2022 at 07:01:01AM -0500, Andrew Dunstan wrote:
> On 2022-12-09 Fr 13:44, Nathan Bossart wrote:
>> Any thoughts on $SUBJECT?
>
> Yeah, the discussion got way off into the weeds here. I think the
> original proposal seems reasonable. Please add it to the next CF if you
> haven't already.
Here it is: https://commitfest.postgresql.org/41/4043/
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: add \dpS to psql
@ 2022-12-28 11:46 Maxim Orlov <orlovmg@gmail.com>
parent: Nathan Bossart <nathandbossart@gmail.com>
0 siblings, 1 reply; 26+ messages in thread
From: Maxim Orlov @ 2022-12-28 11:46 UTC (permalink / raw)
To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: Andrew Dunstan <andrew@dunslane.net>; Michael Paquier <michael@paquier.xyz>; Tom Lane <tgl@sss.pgh.pa.us>; Isaac Morland <isaac.morland@gmail.com>; Justin Pryzby <pryzby@telsasoft.com>; Pavel Luzanov <p.luzanov@postgrespro.ru>; pgsql-hackers
> Here it is: https://commitfest.postgresql.org/41/4043/
>
Hi!
The patch applies with no problem, implements what it declared, CF bot is
happy.
Without patch \dpS shows 0 rows, after applying system objects are shown.
Consider this patch useful, hope it will be committed soon.
--
Best regards,
Maxim Orlov.
^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: add \dpS to psql
@ 2022-12-28 21:26 Nathan Bossart <nathandbossart@gmail.com>
parent: Maxim Orlov <orlovmg@gmail.com>
0 siblings, 1 reply; 26+ messages in thread
From: Nathan Bossart @ 2022-12-28 21:26 UTC (permalink / raw)
To: Maxim Orlov <orlovmg@gmail.com>; +Cc: Andrew Dunstan <andrew@dunslane.net>; Michael Paquier <michael@paquier.xyz>; Tom Lane <tgl@sss.pgh.pa.us>; Isaac Morland <isaac.morland@gmail.com>; Justin Pryzby <pryzby@telsasoft.com>; Pavel Luzanov <p.luzanov@postgrespro.ru>; pgsql-hackers
On Wed, Dec 28, 2022 at 02:46:23PM +0300, Maxim Orlov wrote:
> The patch applies with no problem, implements what it declared, CF bot is
> happy.
> Without patch \dpS shows 0 rows, after applying system objects are shown.
> Consider this patch useful, hope it will be committed soon.
Thanks for reviewing.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: add \dpS to psql
@ 2023-01-06 18:52 Dean Rasheed <dean.a.rasheed@gmail.com>
parent: Nathan Bossart <nathandbossart@gmail.com>
0 siblings, 1 reply; 26+ messages in thread
From: Dean Rasheed @ 2023-01-06 18:52 UTC (permalink / raw)
To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: Maxim Orlov <orlovmg@gmail.com>; Andrew Dunstan <andrew@dunslane.net>; Michael Paquier <michael@paquier.xyz>; Tom Lane <tgl@sss.pgh.pa.us>; Isaac Morland <isaac.morland@gmail.com>; Justin Pryzby <pryzby@telsasoft.com>; Pavel Luzanov <p.luzanov@postgrespro.ru>; pgsql-hackers
On Wed, 28 Dec 2022 at 21:26, Nathan Bossart <nathandbossart@gmail.com> wrote:
>
> On Wed, Dec 28, 2022 at 02:46:23PM +0300, Maxim Orlov wrote:
> > The patch applies with no problem, implements what it declared, CF bot is
> > happy.
> > Without patch \dpS shows 0 rows, after applying system objects are shown.
> > Consider this patch useful, hope it will be committed soon.
>
> Thanks for reviewing.
>
Looking this over this, I have a couple of comments:
Firstly, I think it should allow \zS in the same fashion as \dpS,
since \z is an alias for \dp, so the 2 should be kept in sync.
Secondly, I don't think the following is the right SQL clause to use
in the absence of "S":
if (!showSystem && !pattern)
appendPQExpBufferStr(&buf, "AND n.nspname !~ '^pg_'\n");
I know that's the condition it used before, but the problem with using
that now is that it will cause temporary relations to be excluded
unless the "S" modifier is used, which goes against the expectation
that "S" just causes system relations to be included. Also, it fails
to exclude information_schema relations, if that happens to be on the
user's search_path.
So I think we should use the same SQL clauses as every other psql
command that supports "S", namely:
if (!showSystem && !pattern)
appendPQExpBufferStr(&buf, " AND n.nspname <> 'pg_catalog'\n"
" AND n.nspname <> 'information_schema'\n");
Updated patch attached.
Regards,
Dean
Attachments:
add_s_modifier_to_dp-v2.patch (text/x-patch, 6.1K) download
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
new file mode 100644
index 8a5285d..3f994a3
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -1825,14 +1825,16 @@ INSERT INTO tbl1 VALUES ($1, $2) \bind '
<varlistentry>
- <term><literal>\dp [ <link linkend="app-psql-patterns"><replaceable class="parameter">pattern</replaceable></link> ]</literal></term>
+ <term><literal>\dp[S] [ <link linkend="app-psql-patterns"><replaceable class="parameter">pattern</replaceable></link> ]</literal></term>
<listitem>
<para>
Lists tables, views and sequences with their
associated access privileges.
If <replaceable class="parameter">pattern</replaceable> is
specified, only tables, views and sequences whose names match the
- pattern are listed.
+ pattern are listed. By default only user-created objects are shown;
+ supply a pattern or the <literal>S</literal> modifier to include
+ system objects.
</para>
<para>
@@ -3575,14 +3577,16 @@ testdb=> <userinput>\setenv LESS -imx
<varlistentry>
- <term><literal>\z [ <link linkend="app-psql-patterns"><replaceable class="parameter">pattern</replaceable></link> ]</literal></term>
+ <term><literal>\z[S] [ <link linkend="app-psql-patterns"><replaceable class="parameter">pattern</replaceable></link> ]</literal></term>
<listitem>
<para>
Lists tables, views and sequences with their
associated access privileges.
If a <replaceable class="parameter">pattern</replaceable> is
specified, only tables, views and sequences whose names match the
- pattern are listed.
+ pattern are listed. By default only user-created objects are shown;
+ supply a pattern or the <literal>S</literal> modifier to include
+ system objects.
</para>
<para>
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
new file mode 100644
index 00b89d9..b5201ed
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -140,7 +140,8 @@ static backslashResult exec_command_writ
static backslashResult exec_command_watch(PsqlScanState scan_state, bool active_branch,
PQExpBuffer query_buf, PQExpBuffer previous_buf);
static backslashResult exec_command_x(PsqlScanState scan_state, bool active_branch);
-static backslashResult exec_command_z(PsqlScanState scan_state, bool active_branch);
+static backslashResult exec_command_z(PsqlScanState scan_state, bool active_branch,
+ const char *cmd);
static backslashResult exec_command_shell_escape(PsqlScanState scan_state, bool active_branch);
static backslashResult exec_command_slash_command_help(PsqlScanState scan_state, bool active_branch);
static char *read_connect_arg(PsqlScanState scan_state);
@@ -413,8 +414,8 @@ exec_command(const char *cmd,
query_buf, previous_buf);
else if (strcmp(cmd, "x") == 0)
status = exec_command_x(scan_state, active_branch);
- else if (strcmp(cmd, "z") == 0)
- status = exec_command_z(scan_state, active_branch);
+ else if (strcmp(cmd, "z") == 0 || strcmp(cmd, "zS") == 0)
+ status = exec_command_z(scan_state, active_branch, cmd);
else if (strcmp(cmd, "!") == 0)
status = exec_command_shell_escape(scan_state, active_branch);
else if (strcmp(cmd, "?") == 0)
@@ -875,7 +876,7 @@ exec_command_d(PsqlScanState scan_state,
success = listCollations(pattern, show_verbose, show_system);
break;
case 'p':
- success = permissionsList(pattern);
+ success = permissionsList(pattern, show_system);
break;
case 'P':
{
@@ -2822,16 +2823,22 @@ exec_command_x(PsqlScanState scan_state,
* \z -- list table privileges (equivalent to \dp)
*/
static backslashResult
-exec_command_z(PsqlScanState scan_state, bool active_branch)
+exec_command_z(PsqlScanState scan_state, bool active_branch, const char *cmd)
{
bool success = true;
if (active_branch)
{
- char *pattern = psql_scan_slash_option(scan_state,
- OT_NORMAL, NULL, true);
+ char *pattern;
+ bool show_system;
+
+ pattern = psql_scan_slash_option(scan_state,
+ OT_NORMAL, NULL, true);
+
+ show_system = strchr(cmd, 'S') ? true : false;
+
+ success = permissionsList(pattern, show_system);
- success = permissionsList(pattern);
free(pattern);
}
else
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
new file mode 100644
index 523fab6..e280b6f
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -1002,7 +1002,7 @@ listAllDbs(const char *pattern, bool ver
* \z (now also \dp -- perhaps more mnemonic)
*/
bool
-permissionsList(const char *pattern)
+permissionsList(const char *pattern, bool showSystem)
{
PQExpBufferData buf;
PGresult *res;
@@ -1121,15 +1121,13 @@ permissionsList(const char *pattern)
CppAsString2(RELKIND_FOREIGN_TABLE) ","
CppAsString2(RELKIND_PARTITIONED_TABLE) ")\n");
- /*
- * Unless a schema pattern is specified, we suppress system and temp
- * tables, since they normally aren't very interesting from a permissions
- * point of view. You can see 'em by explicit request though, eg with \z
- * pg_catalog.*
- */
+ if (!showSystem && !pattern)
+ appendPQExpBufferStr(&buf, " AND n.nspname <> 'pg_catalog'\n"
+ " AND n.nspname <> 'information_schema'\n");
+
if (!validateSQLNamePattern(&buf, pattern, true, false,
"n.nspname", "c.relname", NULL,
- "n.nspname !~ '^pg_' AND pg_catalog.pg_table_is_visible(c.oid)",
+ "pg_catalog.pg_table_is_visible(c.oid)",
NULL, 3))
goto error_return;
diff --git a/src/bin/psql/describe.h b/src/bin/psql/describe.h
new file mode 100644
index 15f62b9..554fe86
--- a/src/bin/psql/describe.h
+++ b/src/bin/psql/describe.h
@@ -38,7 +38,7 @@ extern bool describeRoles(const char *pa
extern bool listDbRoleSettings(const char *pattern, const char *pattern2);
/* \z (or \dp) */
-extern bool permissionsList(const char *pattern);
+extern bool permissionsList(const char *pattern, bool showSystem);
/* \ddp */
extern bool listDefaultACLs(const char *pattern);
* Re: add \dpS to psql
@ 2023-01-07 00:36 Nathan Bossart <nathandbossart@gmail.com>
parent: Dean Rasheed <dean.a.rasheed@gmail.com>
0 siblings, 1 reply; 26+ messages in thread
From: Nathan Bossart @ 2023-01-07 00:36 UTC (permalink / raw)
To: Dean Rasheed <dean.a.rasheed@gmail.com>; +Cc: Maxim Orlov <orlovmg@gmail.com>; Andrew Dunstan <andrew@dunslane.net>; Michael Paquier <michael@paquier.xyz>; Tom Lane <tgl@sss.pgh.pa.us>; Isaac Morland <isaac.morland@gmail.com>; Justin Pryzby <pryzby@telsasoft.com>; Pavel Luzanov <p.luzanov@postgrespro.ru>; pgsql-hackers
On Fri, Jan 06, 2023 at 06:52:33PM +0000, Dean Rasheed wrote:
> Looking this over this, I have a couple of comments:
Thanks for reviewing.
> Firstly, I think it should allow \zS in the same fashion as \dpS,
> since \z is an alias for \dp, so the 2 should be kept in sync.
That seems reasonable to me.
> Secondly, I don't think the following is the right SQL clause to use
> in the absence of "S":
>
> if (!showSystem && !pattern)
> appendPQExpBufferStr(&buf, "AND n.nspname !~ '^pg_'\n");
>
> I know that's the condition it used before, but the problem with using
> that now is that it will cause temporary relations to be excluded
> unless the "S" modifier is used, which goes against the expectation
> that "S" just causes system relations to be included. Also, it fails
> to exclude information_schema relations, if that happens to be on the
> user's search_path.
>
> So I think we should use the same SQL clauses as every other psql
> command that supports "S", namely:
>
> if (!showSystem && !pattern)
> appendPQExpBufferStr(&buf, " AND n.nspname <> 'pg_catalog'\n"
> " AND n.nspname <> 'information_schema'\n");
Good catch. I should have noticed this. The deleted comment mentions that
the system/temp tables normally aren't very interesting from a permissions
perspective, so perhaps there is an argument for always excluding temp
tables without a pattern. After all, \dp always excludes indexes and TOAST
tables. However, it looks like \dt includes temp tables, and I didn't see
any other meta-commands that excluded them.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: add \dpS to psql
@ 2023-01-07 11:18 Dean Rasheed <dean.a.rasheed@gmail.com>
parent: Nathan Bossart <nathandbossart@gmail.com>
0 siblings, 1 reply; 26+ messages in thread
From: Dean Rasheed @ 2023-01-07 11:18 UTC (permalink / raw)
To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: Maxim Orlov <orlovmg@gmail.com>; Andrew Dunstan <andrew@dunslane.net>; Michael Paquier <michael@paquier.xyz>; Tom Lane <tgl@sss.pgh.pa.us>; Isaac Morland <isaac.morland@gmail.com>; Justin Pryzby <pryzby@telsasoft.com>; Pavel Luzanov <p.luzanov@postgrespro.ru>; pgsql-hackers
On Sat, 7 Jan 2023 at 00:36, Nathan Bossart <nathandbossart@gmail.com> wrote:
>
> On Fri, Jan 06, 2023 at 06:52:33PM +0000, Dean Rasheed wrote:
> >
> > So I think we should use the same SQL clauses as every other psql
> > command that supports "S", namely:
> >
> > if (!showSystem && !pattern)
> > appendPQExpBufferStr(&buf, " AND n.nspname <> 'pg_catalog'\n"
> > " AND n.nspname <> 'information_schema'\n");
>
> Good catch. I should have noticed this. The deleted comment mentions that
> the system/temp tables normally aren't very interesting from a permissions
> perspective, so perhaps there is an argument for always excluding temp
> tables without a pattern. After all, \dp always excludes indexes and TOAST
> tables. However, it looks like \dt includes temp tables, and I didn't see
> any other meta-commands that excluded them.
>
It might be true that temp tables aren't usually interesting from a
permissions point of view, but it's not hard to imagine situations
where interesting things do happen. It's also probably the case that
most users won't have many temp tables, so I don't think including
them by default will be particularly intrusive.
Also, from a user perspective, I think it would be something of a POLA
violation for \dp[S] and \dt[S] to include different sets of tables,
though I appreciate that we do that now. There's nothing in the docs
to indicate that that's the case.
Anyway, I've pushed the v2 patch as-is. If anyone feels strongly
enough that we should change its behaviour for temp tables, then we
can still discuss that.
Regards,
Dean
^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: add \dpS to psql
@ 2023-01-09 17:45 Nathan Bossart <nathandbossart@gmail.com>
parent: Dean Rasheed <dean.a.rasheed@gmail.com>
0 siblings, 0 replies; 26+ messages in thread
From: Nathan Bossart @ 2023-01-09 17:45 UTC (permalink / raw)
To: Dean Rasheed <dean.a.rasheed@gmail.com>; +Cc: Maxim Orlov <orlovmg@gmail.com>; Andrew Dunstan <andrew@dunslane.net>; Michael Paquier <michael@paquier.xyz>; Tom Lane <tgl@sss.pgh.pa.us>; Isaac Morland <isaac.morland@gmail.com>; Justin Pryzby <pryzby@telsasoft.com>; Pavel Luzanov <p.luzanov@postgrespro.ru>; pgsql-hackers
On Sat, Jan 07, 2023 at 11:18:59AM +0000, Dean Rasheed wrote:
> It might be true that temp tables aren't usually interesting from a
> permissions point of view, but it's not hard to imagine situations
> where interesting things do happen. It's also probably the case that
> most users won't have many temp tables, so I don't think including
> them by default will be particularly intrusive.
>
> Also, from a user perspective, I think it would be something of a POLA
> violation for \dp[S] and \dt[S] to include different sets of tables,
> though I appreciate that we do that now. There's nothing in the docs
> to indicate that that's the case.
Agreed.
> Anyway, I've pushed the v2 patch as-is. If anyone feels strongly
> enough that we should change its behaviour for temp tables, then we
> can still discuss that.
Thanks!
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 26+ messages in thread
end of thread, other threads:[~2023-01-09 17:45 UTC | newest]
Thread overview: 26+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2022-12-06 19:36 add \dpS to psql Nathan Bossart <nathandbossart@gmail.com>
2022-12-07 07:48 ` Pavel Luzanov <p.luzanov@postgrespro.ru>
2022-12-07 22:41 ` Nathan Bossart <nathandbossart@gmail.com>
2022-12-08 02:39 ` Justin Pryzby <pryzby@telsasoft.com>
2022-12-08 04:19 ` Nathan Bossart <nathandbossart@gmail.com>
2022-12-08 04:25 ` Tom Lane <tgl@sss.pgh.pa.us>
2022-12-08 04:43 ` Nathan Bossart <nathandbossart@gmail.com>
2022-12-08 04:48 ` Isaac Morland <isaac.morland@gmail.com>
2022-12-08 05:07 ` Nathan Bossart <nathandbossart@gmail.com>
2022-12-08 05:12 ` Isaac Morland <isaac.morland@gmail.com>
2022-12-08 05:15 ` Tom Lane <tgl@sss.pgh.pa.us>
2022-12-08 17:15 ` Nathan Bossart <nathandbossart@gmail.com>
2022-12-08 18:39 ` Nathan Bossart <nathandbossart@gmail.com>
2022-12-09 04:36 ` Michael Paquier <michael@paquier.xyz>
2022-12-09 18:40 ` Nathan Bossart <nathandbossart@gmail.com>
2022-12-09 18:44 ` Nathan Bossart <nathandbossart@gmail.com>
2022-12-12 12:01 ` Andrew Dunstan <andrew@dunslane.net>
2022-12-12 17:18 ` Nathan Bossart <nathandbossart@gmail.com>
2022-12-28 11:46 ` Maxim Orlov <orlovmg@gmail.com>
2022-12-28 21:26 ` Nathan Bossart <nathandbossart@gmail.com>
2023-01-06 18:52 ` Dean Rasheed <dean.a.rasheed@gmail.com>
2023-01-07 00:36 ` Nathan Bossart <nathandbossart@gmail.com>
2023-01-07 11:18 ` Dean Rasheed <dean.a.rasheed@gmail.com>
2023-01-09 17:45 ` Nathan Bossart <nathandbossart@gmail.com>
2022-12-08 11:03 ` Pavel Luzanov <p.luzanov@postgrespro.ru>
2022-12-08 11:35 ` Alvaro Herrera <alvherre@alvh.no-ip.org>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox