Re: CLUSTER on partitioned index

On Mon, Apr 11, 2022 at 7:06 AM Justin Pryzby <pryzby@telsasoft.com> wrote:

> On Sat, Apr 02, 2022 at 07:21:11PM +0200, Alvaro Herrera wrote:
> > Small things here.
>
> > 1. in VACUUM FULL we only process partitions that are owned by the
> > invoking user.  We don't have this test in the new code.  I'm not sure
> > why do we do that there; is it worth doing the same here?
>
> That dates to a556549d7 (see also cbe24a6dd8 for an earlier commit in
> CLUSTER
> itself).  The reason was to avoid blocking if an unprivileged user runs
> VACUUM
> FULL which would try to lock things (including shared catalogs) before
> checking
> if they have permission to vacuum them.  That commit also initially checks
> the
> owner of the partitioned table, and then re-checking owner of partitions
> later
> on.
>
> A similar issue exists here.  But 1) catalog tables are not partitioned,
> and,
> 2) ownership of a partitioned table is checked immediately.  So the
> problem can
> only occur if a user who owns a partitioned table but doesn't own all its
> partitions tries to cluster it, and it blocks behind another session.
> Fixing
> this is probably a good idea, but seems improbable that it would avoid a
> DOS.
>
> > 2. We should silently skip a partition that's a foreign table, I
> > suppose.
>
> I think it's not needed, since the loop over index children doesn't see a
> child
> index on the foreign table.  ?
>
> > 3. We do mark the index on the partitions as indisclustered AFAICS (we
> > claim that the partitioned table's index is not marked, which is
> > accurate).  So users doing unadorned CLUSTER afterwards will get the
> > partitions clustered too, once they cluster the partitioned table.  If
> > they don't want this, they would have to ALTER TABLE to remove the
> > marking.  How likely is that this will be a problem?  Maybe documenting
> > this point is enough.
>
> It seems at least as likely that someone would *want* the partitions to be
> marked clustered as that someone would want them to be unchanged.
>
> The cluster mark accurately reflects having been clustered.  It seems
> unlikely
> that a user would want something else to be clustered later by "cluster;".
> Since clustering on a partitioned table wasn't supported before, nothing
> weird
> will happen to someone who upgrades to v15 unless they elect to use the new
> feature.  As this seems to be POLA, it doesn't even need to be
> documented.  ?
>
Hi,
For v13-0002-cluster-early-ownership-check-of-partitions.patch :

only for it to fails ownership check anyway

to fails -> to fail

Cheers