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