Date: Thu, 15 Dec 2022 13:48:13 -0600
From: Justin Pryzby <pryzby@telsasoft.com>
To: Jeff Davis <pgsql@j-davis.com>
Cc: Pavel Luzanov <p.luzanov@postgrespro.ru>,
	Nathan Bossart <nathandbossart@gmail.com>,
	pgsql-hackers@postgresql.org
Subject: Re: allow granting CLUSTER, REFRESH MATERIALIZED VIEW, and REINDEX
Message-ID: <20221215194813.GK1153@telsasoft.com>
References: <20221212210136.GA449764@nathanxps13>
 <b19eaf6601d142f96ff4c3c3591f430cd81befbd.camel@j-davis.com>
 <20221214032332.GA671806@nathanxps13>
 <b572d238-0de2-9cad-5f34-4741dc627834@postgrespro.ru>
 <f746293762ae262e321f89153d653173ba759636.camel@j-davis.com>
 <8f7172da-2b58-3bd0-97ae-5126e2a7970c@postgrespro.ru>
 <20221214221140.GA1153@telsasoft.com>
 <cf56dc1a36e5c15e19c4be634038029096d96dab.camel@j-davis.com>
 <b41c0577-8e16-c0fc-0d98-f17155047354@postgrespro.ru>
 <295e86c7aeafb8e2623f8eccdc846855bf2c7e0c.camel@j-davis.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <295e86c7aeafb8e2623f8eccdc846855bf2c7e0c.camel@j-davis.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
Archived-At: 
 <https://www.postgresql.org/message-id/20221215194813.GK1153%40telsasoft.com>
Precedence: bulk

On Thu, Dec 15, 2022 at 10:10:43AM -0800, Jeff Davis wrote:
> On Thu, 2022-12-15 at 12:31 +0300, Pavel Luzanov wrote:
> > I think the approach that Nathan implemented [1] for TOAST tables
> > in the latest version can be used for partitioned tables as well.
> > Skipping the privilege check for partitions while working with
> > a partitioned table. In that case we would get exactly the same
> > behavior
> > as for INSERT, SELECT, etc privileges - the MAINTAIN privilege would 
> > work for
> > the whole partitioned table, but not for individual partitions.
> 
> There is some weirdness in 15, too:

I gather you mean postgresql v15.1 and master ?

>    -- the following commands seem inconsistent to me:
>    vacuum p; -- skips p1 with warning
>    analyze p; -- skips p1 with warning
>    cluster p using p_idx; -- silently skips p1
>    reindex table p; -- reindexes p0 and p1 (owned by su)

Clustering on a partitioned table is new in v15, and this behavior is
from 3f19e176ae0 and cfdd03f45e6, which added
get_tables_to_cluster_partitioned(), borrowing from expand_vacuum_rel()
and get_tables_to_cluster().

vacuum initially calls vacuum_is_permitted_for_relation() only for the
partitioned table, and *later* locks the partition and then checks its
permissions, which is when the message is output.

Since v15, cluster calls get_tables_to_cluster_partitioned(), which
silently discards partitions failing ACL.

We could change it to emit a message, which would seem to behave like
vacuum, except that the check is happening earlier, and (unlike vacuum)
partitions skipped later during CLUOPT_RECHECK wouldn't have any message
output.

Or we could change cluster_rel() to output a message when skipping.  But
these patches hardly touched that function at all.  I suppose we could
change to emit a message during RECHECK (maybe only in master branch).
If need be, that could be controlled by a new CLUOPT_*.

-- 
Justin