On Thu, Jan 21, 2021 at 11:48:08PM +0300, Alexey Kondratov wrote: > 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?
> + * Even if a table's indexes were moved to a new tablespace, > the index > + * on its toast table is not normally moved. > */ > ReindexParams newparams = *params; > > newparams.options &= ~(REINDEXOPT_MISSING_OK); > + if (!allowSystemTableMods) > + newparams.tablespaceOid = InvalidOid; I think you're right. So actually TOAST should never move, even if allowSystemTableMods, right ? > @@ -292,7 +315,11 @@ REINDEX [ ( <replaceable > class="parameter">option</replaceable> [, ...] ) ] { IN > with <command>REINDEX INDEX</command> or <command>REINDEX TABLE</command>, > respectively. Each partition of the specified partitioned relation is > reindexed in a separate transaction. Those commands cannot be used inside > - a transaction block when working on a partitioned table or index. > + a transaction block when working on a partitioned table or index. If > + <command>REINDEX</command> with <literal>TABLESPACE</literal> executed > + on partitioned relation fails it may have moved some partitions to the new > + tablespace. Repeated command will still reindex all partitions even if > they > + are already in the new tablespace. Minor corrections here: If a <command>REINDEX</command> command fails when run on a partitioned relation, and <literal>TABLESPACE</literal> was specified, then it may have moved indexes on some partitions to the new tablespace. Re-running the command will reindex all partitions and move previously-unprocessed indexes to the new tablespace. -- Justin