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

On Thu, Dec 15, 2022 at 09:12:26AM +0900, Michael Paquier wrote:
> On Wed, Dec 14, 2022 at 03:29:39PM -0800, Nathan Bossart wrote:
>> On Wed, Dec 14, 2022 at 11:05:13AM -0800, Jeff Davis wrote:
>>> On Wed, 2022-12-14 at 10:16 -0800, Nathan Bossart wrote:
>>>> Okay.  Should all the privileges governed by MAINTAIN apply to a
>>>> relation's
>>>> TOAST table as well?
>>>
>>> Yes, I agree.
>>
>> This might be tricky, because AFAICT you have to scan pg_class to find a
>> TOAST table's main relation.
>
> Ugh, yeah. Are we talking about a case where we know the toast
> information but need to look back at some information of its parent to
> do a decision? I don't recall a case where we do that. CLUSTER,
> REINDEX and VACUUM lock first the parent when working on it, and no
> AEL is taken on the parent if doing directly a VACUUM or a REINDEX on
> the toast table, so that could lead to deadlock scenarios. Shouldn't
> MAINTAIN be sent down to the toast table as well if that's not done
> this way?

Another option I'm looking at is skipping the privilege checks when VACUUM
recurses to a TOAST table. This won't allow you to VACUUM the TOAST table
directly, but it would at least address the originally-reported issue [0].

Since you can't ANALYZE, REFRESH, or LOCK TOAST tables, this isn't a
problem for those commands. CLUSTER and REINDEX seem to process relations'
TOAST tables without extra privilege checks already. So with the attached
patch applied, you wouldn't be able to VACUUM, CLUSTER, and REINDEX TOAST
tableѕ directly (unless you were given MAINTAIN or pg_maintain), but you
could indirectly process them by specifying the main relation.

I don't know if this is good enough. It seems like ideally you should be
able to VACUUM a TOAST table directly if you have MAINTAIN on its main
relation.

[0] https://postgr.es/m/b572d238-0de2-9cad-5f34-4741dc627834%40postgrespro.ru

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