Re: allow granting CLUSTER, REFRESH MATERIALIZED VIEW, and REINDEX

Ted Yu <yuzhihong@gmail.com>

From: Ted Yu <yuzhihong@gmail.com>
To: Nathan Bossart <nathandbossart@gmail.com>
Cc: Jeff Davis <pgsql@j-davis.com>, Pavel Luzanov <p.luzanov@postgrespro.ru>, Justin Pryzby <pryzby@telsasoft.com>, pgsql-hackers@postgresql.org
Date: 2022-12-17T12:39:29Z
Lists: pgsql-hackers
On Fri, Dec 16, 2022 at 10:04 PM Nathan Bossart <nathandbossart@gmail.com>
wrote:

> On Thu, Dec 15, 2022 at 10:10:43AM -0800, Jeff Davis wrote:
> > The proposal to skip privilege checks for partitions would be
> > consistent with INSERT, SELECT, REINDEX that flow through to the
> > underlying partitions regardless of permissions/ownership (and even
> > RLS). It would be very minor behavior change on 15 for this weird case
> > of superuser-owned partitions, but I doubt anyone would be relying on
> > that.
>
> I've attached a work-in-progress patch that aims to accomplish this.
> Instead of skipping the privilege checks, I added logic to trawl through
> pg_inherits and pg_class to check whether the user has privileges for the
> partitioned table or for the main relation of a TOAST table.  This means
> that MAINTAIN on a partitioned table is enough to execute maintenance
> commands on all the partitions, and MAINTAIN on a main relation is enough
> to execute maintenance commands on its TOAST table.  Also, the maintenance
> commands that flow through to the partitions or the TOAST table should no
> longer error due to permissions when the user only has MAINTAIN on the
> paritioned table or main relation.
>
> --
> Nathan Bossart
> Amazon Web Services: https://aws.amazon.com

Hi,

+cluster_is_permitted_for_relation(Oid relid, Oid userid)
+{
+       return pg_class_aclcheck(relid, userid, ACL_MAINTAIN) ==
ACLCHECK_OK ||
+                  has_parent_privs(relid, userid, ACL_MAINTAIN);

Since the func only contains one statement, it seems this can be defined as
a macro instead.

+       List       *ancestors = get_partition_ancestors(relid);
+       Oid                     root = InvalidOid;

nit: it would be better if the variable `root` can be aligned with variable
`ancestors`.

Cheers

Commits

  1. Improve privilege documentation for maintenance commands.

  2. Fix cache lookup hazards introduced by ff9618e82a.

  3. Move bool parameter for vacuum_rel() to option bits.

  4. Fix MAINTAIN privileges for toast tables and partitions.

  5. Add grantable MAINTAIN privilege and pg_maintain role.