Re: Allow CLUSTER, VACUUM FULL and REINDEX to change tablespace on the fly

On 2021-01-21 17:06, Alexey Kondratov wrote:
> On 2021-01-21 04:41, Michael Paquier wrote:
>
>> There are no tests for partitioned tables, aka we'd want to make sure
>> that the new partitioned index is on the correct tablespace, as well
>> as all its leaves. It may be better to have at least two levels of
>> partitioned tables, as well as a partitioned table with no leaves in
>> the cases dealt with.
>>
>
> Yes, sure, it makes sense.
>
>> + *
>> + * Even if a table's indexes were moved to a new tablespace,
>> the index
>> + * on its toast table is not normally moved.
>> */
>> Still, REINDEX (TABLESPACE) TABLE should move all of them to be
>> consistent with ALTER TABLE SET TABLESPACE, but that's not the case
>> with this code, no? This requires proper test coverage, but there is
>> nothing of the kind in this patch.
>
> You are right, we do not move TOAST indexes now, since
> IsSystemRelation() is true for TOAST indexes, so I thought that we
> should not allow moving them without allow_system_table_mods=true. Now
> I wonder why ALTER TABLE does that.
>
> I am going to attach the new version of patch set today or tomorrow.
>

Attached is a new patch set of first two patches, that should resolve
all the issues raised before (ACL, docs, tests) excepting TOAST. Double
thanks for suggestion to add more tests with nested partitioning. I have
found and squashed a huge bug related to the returning back to the
default tablespace using newly added tests.

Regarding TOAST. Now we skip moving toast indexes or throw error if
someone wants to move TOAST index directly. I had a look on ALTER TABLE
SET TABLESPACE and it has a bit complicated logic:

1) You cannot move TOAST table directly.
2) But if you move basic relation that TOAST table belongs to, then they
are moved altogether.
3) Same logic as 2) happens if one does ALTER TABLE ALL IN TABLESPACE
...

That way, ALTER TABLE allows moving TOAST tables (with indexes)
implicitly, but does not allow doing that explicitly. In the same time I
found docs to be vague about such behavior it only says:

All tables in the current database in a tablespace can be moved
by using the ALL IN TABLESPACE ... Note that system catalogs are
not moved by this command

Changing any part of a system catalog table is not permitted.

So actually ALTER TABLE treats TOAST relations as system sometimes, but
sometimes not.

From the end user perspective it makes sense to move TOAST with main
table when doing ALTER TABLE SET TABLESPACE. But should we touch indexes
on TOAST table with REINDEX? We cannot move TOAST relation itself, since
we are doing only a reindex, so we end up in the state when TOAST table
and its index are placed in the different tablespaces. This state is not
reachable with ALTER TABLE/INDEX, so it seem we should not allow it with
REINDEX as well, should we?

Regards
--
Alexey Kondratov

Postgres Professional https://www.postgrespro.com
Russian Postgres Company