Re: CLUSTER on partitioned index

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. ?