On Wed, Feb 03, 2021 at 07:54:42PM +0900, Michael Paquier wrote: > Not sure I like that. Here is a proposal: > "it is recommended to separately use ALTER TABLE ONLY on them so as > any new partitions attached inherit the new tablespace value."
So, I have done more work on this stuff today, and applied that as of c5b2860. While reviewing my changes, I have noticed that I have managed to break ALTER TABLE SET TABLESPACE which would have failed when cascading to a toast relation, the extra check placed previously in CheckRelationTableSpaceMove() being incorrect. The most surprising part was that we had zero in-core tests to catch this mistake, so I have added an extra test to cover this scenario while on it. A second thing I have come back to is allow_system_table_mods for toast relations, and decided to just forbid TABLESPACE if attempting to use it directly on a system table even if allow_system_table_mods is true. This was leading to inconsistent behaviors and weirdness in the concurrent case because all the indexes are processed in series after building a list. As we want to ignore the move of toast indexes when moving the indexes of the parent table, this was leading to extra conditions that are not really worth supporting after thinking about it. One other issue was the lack of consistency when using pg_global that was a no-op for the concurrent case but failed in the non-concurrent case. I have put in place more regression tests for all that. Regarding the VACUUM and CLUSTER cases, I am not completely sure if going through these for a tablespace case is the best move we can do, as ALTER TABLE is able to mix multiple operations and all of them require already an AEL to work. REINDEX was different thanks to the case of CONCURRENTLY. Anyway, as a lot of work has been done here already, I would recommend to create new threads about those two topics. I am also closing this patch in the CF app. -- Michael
signature.asc
Description: PGP signature