predefined role(s) for VACUUM and ANALYZE
Hi hackers,
The previous attempt to add a predefined role for VACUUM and ANALYZE [0]/messages/by-id/67a1d667e8ec228b5e07f232184c80348c5d93f4.camel@j-davis.com
resulted in the new pg_checkpoint role in v15. I'd like to try again to
add a new role (or multiple new roles) for VACUUM and ANALYZE.
The primary motivation for this is to continue chipping away at things that
require special privileges or even superuser. VACUUM and ANALYZE typically
require table ownership, database ownership, or superuser. And only
superusers can VACUUM/ANALYZE shared catalogs. A predefined role for these
operations would allow delegating such tasks (e.g., a nightly VACUUM
scheduled with pg_cron) to a role with fewer privileges.
The attached patch adds a pg_vacuum_analyze role that allows VACUUM and
ANALYZE commands on all relations. I started by trying to introduce
separate pg_vacuum and pg_analyze roles, but that quickly became
complicated because the VACUUM and ANALYZE code is intertwined. To
initiate the discussion, here's the simplest thing I could think of.
An alternate approach might be to allow using GRANT to manage these
privileges, as suggested in the previous thread [1]/messages/by-id/20211104224636.5qg6cfyjkw52rh4d@alap3.anarazel.de.
Thoughts?
[0]: /messages/by-id/67a1d667e8ec228b5e07f232184c80348c5d93f4.camel@j-davis.com
[1]: /messages/by-id/20211104224636.5qg6cfyjkw52rh4d@alap3.anarazel.de
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
Attachments:
pg_vacuum_analyze.patchtext/x-diff; charset=us-asciiDownload+43-17
On Fri, Jul 22, 2022 at 01:37:35PM -0700, Nathan Bossart wrote:
The attached patch adds a pg_vacuum_analyze role that allows VACUUM and
ANALYZE commands on all relations. I started by trying to introduce
separate pg_vacuum and pg_analyze roles, but that quickly became
complicated because the VACUUM and ANALYZE code is intertwined. To
initiate the discussion, here's the simplest thing I could think of.
And here's the same patch, but with docs that actually build.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
Attachments:
pg_vacuum_analyze_v2.patchtext/x-diff; charset=us-asciiDownload+43-17
On Sat, Jul 23, 2022 at 2:07 AM Nathan Bossart <nathandbossart@gmail.com> wrote:
Hi hackers,
The previous attempt to add a predefined role for VACUUM and ANALYZE [0]
resulted in the new pg_checkpoint role in v15. I'd like to try again to
add a new role (or multiple new roles) for VACUUM and ANALYZE.The primary motivation for this is to continue chipping away at things that
require special privileges or even superuser. VACUUM and ANALYZE typically
require table ownership, database ownership, or superuser. And only
superusers can VACUUM/ANALYZE shared catalogs. A predefined role for these
operations would allow delegating such tasks (e.g., a nightly VACUUM
scheduled with pg_cron) to a role with fewer privileges.
Thanks. I'm personally happy with more granular levels of control (as
we don't have to give full superuser access to just run a few commands
or maintenance operations) for various postgres commands. The only
concern is that we might eventually end up with many predefined roles
(perhaps one predefined role per command), spreading all around the
code base and it might be difficult for the users to digest all of the
roles in. It will be great if we can have some sort of rules or
methods to define a separate role for a command.
The attached patch adds a pg_vacuum_analyze role that allows VACUUM and
ANALYZE commands on all relations. I started by trying to introduce
separate pg_vacuum and pg_analyze roles, but that quickly became
complicated because the VACUUM and ANALYZE code is intertwined. To
initiate the discussion, here's the simplest thing I could think of.
pg_vacuum_analyze, immediately, makes me think if we need to have a
predefined role for CLUSTER command and maybe for other commands as
well such as EXECUTE, CALL, ALTER SYSTEM SET, LOAD, COPY and so on.
An alternate approach might be to allow using GRANT to manage these
privileges, as suggested in the previous thread [1].Thoughts?
[0] /messages/by-id/67a1d667e8ec228b5e07f232184c80348c5d93f4.camel@j-davis.com
[1] /messages/by-id/20211104224636.5qg6cfyjkw52rh4d@alap3.anarazel.de
I think GRANT approach [1] is worth considering or at least discussing
its pros and cons might give us a better idea as to why we need
separate predefined roles.
Regards,
Bharath Rupireddy.
On Mon, Jul 25, 2022 at 12:58:36PM +0530, Bharath Rupireddy wrote:
Thanks. I'm personally happy with more granular levels of control (as
we don't have to give full superuser access to just run a few commands
or maintenance operations) for various postgres commands. The only
concern is that we might eventually end up with many predefined roles
(perhaps one predefined role per command), spreading all around the
code base and it might be difficult for the users to digest all of the
roles in. It will be great if we can have some sort of rules or
methods to define a separate role for a command.
Yeah, in the future, I could see this growing to a couple dozen predefined
roles. Given they are relatively inexpensive and there are already 12 of
them, I'm personally not too worried about the list becoming too unwieldy.
Another way to help users might be to create additional aggregate
predefined roles (like pg_monitor) for common combinations.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
At Mon, 25 Jul 2022 09:40:49 -0700, Nathan Bossart <nathandbossart@gmail.com> wrote in
On Mon, Jul 25, 2022 at 12:58:36PM +0530, Bharath Rupireddy wrote:
Thanks. I'm personally happy with more granular levels of control (as
we don't have to give full superuser access to just run a few commands
or maintenance operations) for various postgres commands. The only
concern is that we might eventually end up with many predefined roles
(perhaps one predefined role per command), spreading all around the
code base and it might be difficult for the users to digest all of the
roles in. It will be great if we can have some sort of rules or
methods to define a separate role for a command.Yeah, in the future, I could see this growing to a couple dozen predefined
roles. Given they are relatively inexpensive and there are already 12 of
them, I'm personally not too worried about the list becoming too unwieldy.
Another way to help users might be to create additional aggregate
predefined roles (like pg_monitor) for common combinations.
I agree to the necessity of that execution control, but I fear a
little how many similar roles will come in future (but it doesn't seem
so much?). I didn't think so when pg_checkpoint was introdueced,
though. That being said, since we're going to control
maintenance'ish-command execution via predefined roles so it is fine
in that criteria.
One arguable point would be whether we will need to put restriction
the target relations that Bob can vacuum/analyze. If we need that, the
new predeefined role is not sufficient then need a new syntax for that.
GRANT EXECUTION COMMAND VACUUM ON TABLE rel1 TO bob.
GRANT EXECUTION COMMAND VACUUM ON TABLES OWNED BY alice TO bob.
GRANT EXECUTION COMMAND VACUUM ON ALL TABLES OWNED BY alice TO bob.
However, one problem of these syntaxes is they cannot do something to
future relations.
So, considering that aspect, I would finally agree to the proposal
here. (In short +1 for what the patch does.)
About the patch, it seems fine as the whole except the change in error
messages.
- (errmsg("skipping \"%s\" --- only superuser can analyze it",
+ (errmsg("skipping \"%s\" --- only superusers and roles with "
+ "privileges of pg_vacuum_analyze can analyze it",
The message looks a bit too verbose or lengty especially when many
relations are rejected.
WARNING: skipping "pg_statistic" --- only superusers, roles with privileges of pg_vacuum_analyze, or the database owner can vacuum it
WARNING: skipping "pg_type" --- only superusers, roles with privileges of pg_vacuum_analyze, or the database owner can vacuum it
<snip many lines>
WARNING: skipping "user_mappings" --- only table or database owner can vacuum it
VACUUM
Couldn't we simplify the message? For example "skipping \"%s\" ---
insufficient priviledges". We could add a detailed (not a DETAIL:)
message at the end to cover all of the skipped relations, but it may
be too much.
By the way the patch splits an error message into several parts but
that later makes it harder to search for the message in the tree. *I*
would suggest not splitting message strings.
# I refrain from suggesing removing parens surrounding errmsg() :p
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
At Tue, 26 Jul 2022 10:47:12 +0900 (JST), Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote in
WARNING: skipping "pg_statistic" --- only superusers, roles with privileges of pg_vacuum_analyze, or the database owner can vacuum it
WARNING: skipping "pg_type" --- only superusers, roles with privileges of pg_vacuum_analyze, or the database owner can vacuum it
<snip many lines>
WARNING: skipping "user_mappings" --- only table or database owner can vacuum it
By the way, the last error above dissapears by granting
pg_vacuum_analyze to the role. Is there a reason the message is left
alone? And If I specified the view directly, I would get the
following message.
postgres=> vacuum information_schema.user_mappings;
WARNING: skipping "user_mappings" --- cannot vacuum non-tables or special system tables
So, "VACUUM;" does something wrong? Or is it the designed behavior?
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
On Mon, Jul 25, 2022 at 9:47 PM Kyotaro Horiguchi
<horikyota.ntt@gmail.com> wrote:
One arguable point would be whether we will need to put restriction
the target relations that Bob can vacuum/analyze.
Yeah. pg_checkpoint makes sense because you can either CHECKPOINT or
you can't. But for a command with a target, you really ought to have a
permission on the object, not just a general permission. On the other
hand, we do have things like pg_read_all_tables, so we could have
pg_vacuum_all_tables too. Still, it seems somewhat appealing to give
people fine-grained control over this, rather than just "on" or "off".
--
Robert Haas
EDB: http://www.enterprisedb.com
On Tue, Jul 26, 2022 at 10:37 AM Robert Haas <robertmhaas@gmail.com> wrote:
On Mon, Jul 25, 2022 at 9:47 PM Kyotaro Horiguchi
<horikyota.ntt@gmail.com> wrote:One arguable point would be whether we will need to put restriction
the target relations that Bob can vacuum/analyze.
But for a command with a target, you really ought to have a
permission on the object, not just a general permission. On the other
hand, we do have things like pg_read_all_tables, so we could have
pg_vacuum_all_tables too.
I'm still more likely to create a specific security definer function owned
by the relevant table owner to give out ANALYZE (and maybe VACUUM)
permission to ETL-performing roles.
Still, it seems somewhat appealing to give
people fine-grained control over this, rather than just "on" or "off".
Appealing enough to consume a couple of permission bits?
/messages/by-id/CAKFQuwZ6dhjTFV7Bwmehe1N3=k484y4mM22zuYjVEU2dq9V1aQ@mail.gmail.com
David J.
On Tue, Jul 26, 2022 at 1:50 PM David G. Johnston
<david.g.johnston@gmail.com> wrote:
Still, it seems somewhat appealing to give
people fine-grained control over this, rather than just "on" or "off".Appealing enough to consume a couple of permission bits?
/messages/by-id/CAKFQuwZ6dhjTFV7Bwmehe1N3=k484y4mM22zuYjVEU2dq9V1aQ@mail.gmail.com
I think we're down to 0 remaining now, so it'd be hard to justify
consuming 2 of 0 remaining bits. However, I maintain that the solution
to this is either (1) change the aclitem representation to get another
32 bits or (2) invent a different system for less-commonly used
permission bits. Checking permissions for SELECT or UPDATE has to be
really fast, because most queries will need to do that sort of thing.
If we represented VACUUM or ANALYZE in some other way in the catalogs
that was more scalable but less efficient, it wouldn't be a big deal
(although there's the issue of code duplication to consider).
--
Robert Haas
EDB: http://www.enterprisedb.com
At Tue, 26 Jul 2022 13:54:38 -0400, Robert Haas <robertmhaas@gmail.com> wrote in
On Tue, Jul 26, 2022 at 1:50 PM David G. Johnston
<david.g.johnston@gmail.com> wrote:Still, it seems somewhat appealing to give
people fine-grained control over this, rather than just "on" or "off".Appealing enough to consume a couple of permission bits?
/messages/by-id/CAKFQuwZ6dhjTFV7Bwmehe1N3=k484y4mM22zuYjVEU2dq9V1aQ@mail.gmail.comI think we're down to 0 remaining now, so it'd be hard to justify
consuming 2 of 0 remaining bits. However, I maintain that the solution
to this is either (1) change the aclitem representation to get another
32 bits or (2) invent a different system for less-commonly used
permission bits. Checking permissions for SELECT or UPDATE has to be
really fast, because most queries will need to do that sort of thing.
If we represented VACUUM or ANALYZE in some other way in the catalogs
that was more scalable but less efficient, it wouldn't be a big deal
(although there's the issue of code duplication to consider).
I guess that we can use the last bit for ACL_SLOW_PATH or something
like. Furthermore we could move some existing ACL modeds to that slow
path to vacate some fast-ACL bits.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
On Tue, Jul 26, 2022 at 01:54:38PM -0400, Robert Haas wrote:
I think we're down to 0 remaining now, so it'd be hard to justify
consuming 2 of 0 remaining bits.
AFAICT there are 2 remaining. N_ACL_RIGHTS is only 14.
However, I maintain that the solution
to this is either (1) change the aclitem representation to get another
32 bits or (2) invent a different system for less-commonly used
permission bits. Checking permissions for SELECT or UPDATE has to be
really fast, because most queries will need to do that sort of thing.
If we represented VACUUM or ANALYZE in some other way in the catalogs
that was more scalable but less efficient, it wouldn't be a big deal
(although there's the issue of code duplication to consider).
Perhaps we could add something like a relacl_ext column to affected
catalogs with many more than 32 privilege bits. However, if we actually do
have 2 bits remaining, we wouldn't need to do that work now unless someone
else uses them first. That being said, it's certainly worth thinking about
the future of this stuff.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
Greetings,
* Robert Haas (robertmhaas@gmail.com) wrote:
On Tue, Jul 26, 2022 at 1:50 PM David G. Johnston
<david.g.johnston@gmail.com> wrote:Still, it seems somewhat appealing to give
people fine-grained control over this, rather than just "on" or "off".Appealing enough to consume a couple of permission bits?
/messages/by-id/CAKFQuwZ6dhjTFV7Bwmehe1N3=k484y4mM22zuYjVEU2dq9V1aQ@mail.gmail.comI think we're down to 0 remaining now, so it'd be hard to justify
consuming 2 of 0 remaining bits. However, I maintain that the solution
to this is either (1) change the aclitem representation to get another
32 bits or (2) invent a different system for less-commonly used
permission bits. Checking permissions for SELECT or UPDATE has to be
really fast, because most queries will need to do that sort of thing.
If we represented VACUUM or ANALYZE in some other way in the catalogs
that was more scalable but less efficient, it wouldn't be a big deal
(although there's the issue of code duplication to consider).
I've long felt that we should redefine the way the ACLs work to have a
distinct set of bits for each object type. We don't need to support a
CONNECT bit on a table, yet we do today and we expend quite a few bits
in that way. Having that handled on a per-object-type basis instead
would allow us to get quite a bit more mileage out of the existing 32bit
field before having to introduce more complicated storage methods like
using a bit to tell us to go look up more ACLs somewhere else.
Thanks,
Stephen
Here is a first attempt at allowing users to grant VACUUM or ANALYZE
per-relation. Overall, this seems pretty straightforward. I needed to
adjust the permissions logic for VACUUM/ANALYZE a bit, which causes some
extra WARNING messages for VACUUM (ANALYZE) in some cases, but this didn't
seem particularly worrisome. It may be desirable to allow granting ANALYZE
on specific columns or to allow granting VACUUM/ANALYZE at the schema or
database level, but that is left as a future exercise.
On Tue, Aug 23, 2022 at 07:46:47PM -0400, Stephen Frost wrote:
I've long felt that we should redefine the way the ACLs work to have a
distinct set of bits for each object type. We don't need to support a
CONNECT bit on a table, yet we do today and we expend quite a few bits
in that way. Having that handled on a per-object-type basis instead
would allow us to get quite a bit more mileage out of the existing 32bit
field before having to introduce more complicated storage methods like
using a bit to tell us to go look up more ACLs somewhere else.
There are 2 bits remaining at the moment, so I didn't redesign the ACL
system in the attached patch. However, I did some research on a couple
options. Using a distinct set of bits for each catalog table should free
up a handful of bits, which should indeed kick the can down the road a
little. Another easy option is to simply make AclMode a uint64, which
would immediately free up another 16 privilege bits. I was able to get
this approach building and passing tests in a few minutes, but there might
be performance/space concerns.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
Attachments:
v3-0001-Allow-granting-VACUUM-and-ANALYZE-privileges-on-r.patchtext/x-diff; charset=us-asciiDownload+249-78
On Mon, Sep 5, 2022 at 2:56 PM Nathan Bossart <nathandbossart@gmail.com> wrote:
There are 2 bits remaining at the moment, so I didn't redesign the ACL
system in the attached patch. However, I did some research on a couple
options. Using a distinct set of bits for each catalog table should free
up a handful of bits, which should indeed kick the can down the road a
little. Another easy option is to simply make AclMode a uint64, which
would immediately free up another 16 privilege bits. I was able to get
this approach building and passing tests in a few minutes, but there might
be performance/space concerns.
I believe Tom has expressed such concerns in the past, but it is not
clear to me that they are well-founded. I don't think we have much
code that manipulates large numbers of aclitems, so I can't quite see
where the larger size would be an issue. There may well be some
places, so I'm not saying that Tom or anyone else with concerns is
wrong, but I'm just having a hard time thinking of where it would be a
real issue.
--
Robert Haas
EDB: http://www.enterprisedb.com
Greetings,
* Nathan Bossart (nathandbossart@gmail.com) wrote:
On Tue, Aug 23, 2022 at 07:46:47PM -0400, Stephen Frost wrote:
I've long felt that we should redefine the way the ACLs work to have a
distinct set of bits for each object type. We don't need to support a
CONNECT bit on a table, yet we do today and we expend quite a few bits
in that way. Having that handled on a per-object-type basis instead
would allow us to get quite a bit more mileage out of the existing 32bit
field before having to introduce more complicated storage methods like
using a bit to tell us to go look up more ACLs somewhere else.There are 2 bits remaining at the moment, so I didn't redesign the ACL
system in the attached patch. However, I did some research on a couple
options. Using a distinct set of bits for each catalog table should free
up a handful of bits, which should indeed kick the can down the road a
little. Another easy option is to simply make AclMode a uint64, which
would immediately free up another 16 privilege bits. I was able to get
this approach building and passing tests in a few minutes, but there might
be performance/space concerns.
Considering our burn rate of ACL bits is really rather slow (2 this
year, but prior to that was TRUNCATE in 2008 and CONNECT in 2006), I'd
argue that moving away from the current one-size-fits-all situation
would kick the can down the road more than just 'a little' and wouldn't
have any performance or space concerns. Once we actually get to the
point where we've burned through all of those after the next few decades
then we can move to a uint64 or something else more complicated,
perhaps.
If we were to make the specific bits depend on the object type as I'm
suggesting, then we'd have 8 bits used for relations (10 with the vacuum
and analyze bits), leaving us with 6 remaining inside the existing
uint32, or more bits available than we've ever used since the original
implementation from what I can tell, or at least 15+ years. That seems
like pretty darn good future-proofing without a lot of complication or
any change in physical size. We would also be able to get rid of the
question of "well, is it more valuable to add the ability to GRANT
TRUNCATE on a relation, or GRANT CONNECT on databases" or other rather
odd debates between ultimately very different things.
Thanks,
Stephen
On Tue, Sep 6, 2022 at 11:11 AM Stephen Frost <sfrost@snowman.net> wrote:
Considering our burn rate of ACL bits is really rather slow (2 this
year, but prior to that was TRUNCATE in 2008 and CONNECT in 2006), I'd
argue that moving away from the current one-size-fits-all situation
would kick the can down the road more than just 'a little' and wouldn't
have any performance or space concerns. Once we actually get to the
point where we've burned through all of those after the next few decades
then we can move to a uint64 or something else more complicated,
perhaps.
Our burn rate is slow because there's been a lot of pushback - mostly
from Tom - about consuming the remaining bits. It's not because people
haven't had ideas about how to use them up.
If we were to make the specific bits depend on the object type as I'm
suggesting, then we'd have 8 bits used for relations (10 with the vacuum
and analyze bits), leaving us with 6 remaining inside the existing
uint32, or more bits available than we've ever used since the original
implementation from what I can tell, or at least 15+ years. That seems
like pretty darn good future-proofing without a lot of complication or
any change in physical size. We would also be able to get rid of the
question of "well, is it more valuable to add the ability to GRANT
TRUNCATE on a relation, or GRANT CONNECT on databases" or other rather
odd debates between ultimately very different things.
I mostly agree with this. I don't think it's entirely clear how we
should try to get more bits going forward, but it's clear that we
cannot just forever hold our breath and refuse to find any more bits.
And of the possible ways of doing it, this seems like the one with the
lowest impact, so I think it likely makes sense to do this one first.
--
Robert Haas
EDB: http://www.enterprisedb.com
On Tue, Sep 06, 2022 at 11:24:18AM -0400, Robert Haas wrote:
On Tue, Sep 6, 2022 at 11:11 AM Stephen Frost <sfrost@snowman.net> wrote:
If we were to make the specific bits depend on the object type as I'm
suggesting, then we'd have 8 bits used for relations (10 with the vacuum
and analyze bits), leaving us with 6 remaining inside the existing
uint32, or more bits available than we've ever used since the original
implementation from what I can tell, or at least 15+ years. That seems
like pretty darn good future-proofing without a lot of complication or
any change in physical size. We would also be able to get rid of the
question of "well, is it more valuable to add the ability to GRANT
TRUNCATE on a relation, or GRANT CONNECT on databases" or other rather
odd debates between ultimately very different things.I mostly agree with this. I don't think it's entirely clear how we
should try to get more bits going forward, but it's clear that we
cannot just forever hold our breath and refuse to find any more bits.
And of the possible ways of doing it, this seems like the one with the
lowest impact, so I think it likely makes sense to do this one first.
+1. My earlier note wasn't intended to suggest that one approach was
better than the other, merely that there are a couple of options to choose
from once we run out of bits. I don't think this work needs to be tied to
the VACUUM/ANALYZE stuff, but I am interested in it and hope to take it on
at some point.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
On Mon, Sep 05, 2022 at 11:56:30AM -0700, Nathan Bossart wrote:
Here is a first attempt at allowing users to grant VACUUM or ANALYZE
per-relation. Overall, this seems pretty straightforward. I needed to
adjust the permissions logic for VACUUM/ANALYZE a bit, which causes some
extra WARNING messages for VACUUM (ANALYZE) in some cases, but this didn't
seem particularly worrisome. It may be desirable to allow granting ANALYZE
on specific columns or to allow granting VACUUM/ANALYZE at the schema or
database level, but that is left as a future exercise.
Here is a new patch set with some follow-up patches to implement $SUBJECT.
0001 is the same as v3. 0002 simplifies some WARNING messages as suggested
upthread [0]/messages/by-id/20220726.104712.912995710251150228.horikyota.ntt@gmail.com. 0003 adds the new pg_vacuum_all_tables and
pg_analyze_all_tables predefined roles. Instead of adjusting the
permissions logic in vacuum.c, I modified pg_class_aclmask_ext() to return
the ACL_VACUUM and/or ACL_ANALYZE bits as appropriate.
[0]: /messages/by-id/20220726.104712.912995710251150228.horikyota.ntt@gmail.com
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
Attachments:
v4-0001-Allow-granting-VACUUM-and-ANALYZE-privileges-on-r.patchtext/x-diff; charset=us-asciiDownload+249-78
v4-0002-Simplify-WARNING-messages-emitted-when-skipping-v.patchtext/x-diff; charset=us-asciiDownload+78-99
v4-0003-Add-pg_vacuum_all_tables-and-pg_analyze_all_table.patchtext/x-diff; charset=us-asciiDownload+105-7
Greetings,
* Nathan Bossart (nathandbossart@gmail.com) wrote:
On Tue, Sep 06, 2022 at 11:24:18AM -0400, Robert Haas wrote:
On Tue, Sep 6, 2022 at 11:11 AM Stephen Frost <sfrost@snowman.net> wrote:
If we were to make the specific bits depend on the object type as I'm
suggesting, then we'd have 8 bits used for relations (10 with the vacuum
and analyze bits), leaving us with 6 remaining inside the existing
uint32, or more bits available than we've ever used since the original
implementation from what I can tell, or at least 15+ years. That seems
like pretty darn good future-proofing without a lot of complication or
any change in physical size. We would also be able to get rid of the
question of "well, is it more valuable to add the ability to GRANT
TRUNCATE on a relation, or GRANT CONNECT on databases" or other rather
odd debates between ultimately very different things.I mostly agree with this. I don't think it's entirely clear how we
should try to get more bits going forward, but it's clear that we
cannot just forever hold our breath and refuse to find any more bits.
And of the possible ways of doing it, this seems like the one with the
lowest impact, so I think it likely makes sense to do this one first.+1. My earlier note wasn't intended to suggest that one approach was
better than the other, merely that there are a couple of options to choose
from once we run out of bits. I don't think this work needs to be tied to
the VACUUM/ANALYZE stuff, but I am interested in it and hope to take it on
at some point.
I disagree that we should put the onus for addressing this on the next
person who wants to add bits and just willfully use up the last of them
right now for what strikes me, at least, as a relatively marginal use
case. If we had plenty of bits then, sure, let's use a couple of for
this, but that isn't currently the case. If you want this feature then
the onus is on you to do the legwork to make it such that we have plenty
of bits.
My 2c anyway.
Thanks,
Stephen
On Jul 22, 2022, at 1:37 PM, Nathan Bossart <nathandbossart@gmail.com> wrote:
The primary motivation for this is to continue chipping away at things that
require special privileges or even superuser. VACUUM and ANALYZE typically
require table ownership, database ownership, or superuser. And only
superusers can VACUUM/ANALYZE shared catalogs. A predefined role for these
operations would allow delegating such tasks (e.g., a nightly VACUUM
scheduled with pg_cron) to a role with fewer privileges.The attached patch adds a pg_vacuum_analyze role that allows VACUUM and
ANALYZE commands on all relations.
Granting membership in a role that can VACUUM and ANALYZE any relation seems to grant a subset of a more general category, the ability to perform modifying administrative operations on a relation without necessarily being able to read or modify the logical contents of that relation. That more general category would seem to also include CLUSTER, REINDEX, REFRESH MATERIALIZED VIEW and more broadly ALTER SUBSCRIPTION ... REFRESH PUBLICATION and ALTER DATABASE ... REFRESH COLLATION VERSION. These latter operations may be less critical to database maintenance than is VACUUM, but arguably ANALYZE isn't as critical as is VACUUM, either.
Assuming for the sake of argument that we should create a role something like you propose, can you explain why we should draw the line around just VACUUM and ANALYZE? I am not arguing for including these other commands, but don't want to regret having drawn the line in the wrong place when later we decide to add more roles like the one you are proposing.
—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company