Re: predefined role(s) for VACUUM and ANALYZE

From: Nathan Bossart <nathandbossart(at)gmail(dot)com>
To: Stephen Frost <sfrost(at)snowman(dot)net>
Cc: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: predefined role(s) for VACUUM and ANALYZE
Date: 2022-09-08 05:50:35
Message-ID: 20220908055035.GA2100193@nathanxps13
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Sep 07, 2022 at 07:09:05PM -0400, Stephen Frost wrote:
> Yes, that seems to be the consensus among those involved in this thread
> thus far. Basically, I imagine this involves passing around the object
> type along with the acl info and then using that to check the bits and
> such. I doubt it’s worth inventing a new structure to combine the two …
> but that’s just gut feeling and you may find it does make sense to once you
> get into it.

I've done some preliminary research for this approach, and I've found some
interesting challenges.

* aclparse() will need to handle ambiguous strings. For example, USAGE is
available for most catalogs, so which ACL bit should be chosen? One
possible solution would be to make sure the common privilege types always
use the same bit.

* When comparing ACLs, there probably should be some way to differentiate
overloaded privilege bits, else ACLs for different catalogs that have
nothing in common could evaluate as equal. Such comparisons may be
unlikely, but this still doesn't strike me as acceptable.

* aclitemout() needs some way to determine what privilege an ACL bit
actually refers to. I can think of a couple of ways to do this: 1) we
could create different aclitem types for each catalog (or maybe just one
for pg_class and another for everything else), or 2) we could include the
type in AclItem, perhaps by adding a uint8 field. I noticed that Tom
called out this particular challenge back in 2018 [0].

Am I overlooking an easier way to handle these things? From my admittedly
brief analysis thus far, I'm worried this could devolve into something
overly complex or magical, especially when simply moving to a uint64 might
be a reasonable way to significantly extend AclItem's life span. Robert
suggested upthread that Tom might have concerns with adding another 32 bits
to AclItem, but the archives indicate he has previously proposed exactly
that [1]. Of course, I don't know how everyone feels about the uint64 idea
today, but ISTM like it might be the path of least resistance.

So, here is a new patch set. 0001 expands AclMode to a uint64. 0002
simplifies some WARNING messages for VACUUM/ANALYZE. 0003 introduces
privilege bits for VACUUM and ANALYZE on relations. And 0004 introduces
the pg_vacuum/analyze_all_tables predefined roles.

[0] https://postgr.es/m/18391.1521419120%40sss.pgh.pa.us
[1] https://postgr.es/m/11414.1526422062%40sss.pgh.pa.us

--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

Attachment Content-Type Size
v5-0001-Change-AclMode-from-a-uint32-to-a-uint64.patch text/x-diff 6.9 KB
v5-0002-Simplify-WARNING-messages-emitted-when-skipping-v.patch text/x-diff 15.8 KB
v5-0003-Allow-granting-VACUUM-and-ANALYZE-privileges-on-r.patch text/x-diff 38.8 KB
v5-0004-Add-pg_vacuum_all_tables-and-pg_analyze_all_table.patch text/x-diff 9.4 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Pavel Stehule 2022-09-08 05:50:45 Re: broken table formatting in psql
Previous Message John Naylor 2022-09-08 05:39:19 Re: broken table formatting in psql