Date: Tue, 13 Jun 2023 14:12:46 -0700
From: Nathan Bossart <nathandbossart@gmail.com>
To: Jeff Davis <pgsql@j-davis.com>
Cc: Ted Yu <yuzhihong@gmail.com>, Pavel Luzanov <p.luzanov@postgrespro.ru>,
	Justin Pryzby <pryzby@telsasoft.com>, pgsql-hackers@postgresql.org
Subject: Re: allow granting CLUSTER, REFRESH MATERIALIZED VIEW, and REINDEX
Message-ID: <20230613211246.GA219055@nathanxps13>
References: <20221217060408.GA1256247@nathanxps13>
 <CALte62zW+s+V3fDUxP2ZYk2f=DZsyZO-Aa_bbiA39=wrz=UHuQ@mail.gmail.com>
 <20221218233018.GA1476904@nathanxps13>
 <20230103234549.GA289060@nathanxps13>
 <20230109225157.GA1288965@nathanxps13>
 <7d2a8b72e23c8236281c6e00ae790327f965a8e5.camel@j-davis.com>
 <20230113203334.GA2206335@nathanxps13>
 <e25c84fafae4eb08d8a6e83207678096bac102d6.camel@j-davis.com>
 <20230113225626.GA2380176@nathanxps13>
 <20230113231339.GA2422750@nathanxps13>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230113231339.GA2422750@nathanxps13>
Archived-At: 
 <https://www.postgresql.org/message-id/20230613211246.GA219055%40nathanxps13>
Precedence: bulk

I've been reviewing ff9618e lately, and I'm wondering whether it has the
same problem that 19de0ab solved.  Specifically, ff9618e introduces
has_partition_ancestor_privs(), which is used to check whether a user has
MAINTAIN on any partition ancestors.  This involves syscache lookups, and
presently this function does not take any relation locks.  I did spend some
time trying to induce cache lookup errors, but I didn't have any luck.
However, unless this can be made safe without too much trouble, I think I'm
inclined to partially revert ff9618e, leaving the TOAST-related parts
intact.

By reverting the partition-related parts of ff9618e, users would need to
have MAINTAIN on the partition itself to perform the maintenance command.
MAINTAIN on the partitioned table would no longer be sufficient.  This is
more like how things work on supported versions today.  Privileges are
checked for each partition, so a command that flows down to all partitions
might refuse to process a partition (e.g., if the current user doesn't own
the partition).

In the future, perhaps we could reevaluate adding these partition ancestor
privilege checks, but I'd rather leave it out for now instead of
introducing behavior in v16 that is potentially buggy and difficult to
remove post-GA.

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