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

Hi hackers,

On 2018-12-27 04:57, Michael Paquier wrote:
> On Wed, Dec 26, 2018 at 03:19:06PM -0300, Alvaro Herrera wrote:
>> As for REINDEX, I think it's valuable to move tablespace together with
>> the reindexing.  You can already do it with the CREATE INDEX
>> CONCURRENTLY recipe we recommend, of course; but REINDEX CONCURRENTLY 
>> is
>> not going to provide that, and it seems worth doing.
> 
> Even for plain REINDEX that seems useful.
> 

I've rebased the patch and put it on the closest commitfest. It is 
updated to allow user to do REINDEX CONCURRENTLY + SET TABLESPACE 
altogether, since plain REINDEX CONCURRENTLY became available this year.

On 2019-06-07 21:27, Alexander Korotkov wrote:
> On Fri, Dec 28, 2018 at 11:32 AM Masahiko Sawada 
> <sawada.mshk@gmail.com> wrote:
>> On Thu, Dec 27, 2018 at 10:24 PM Alvaro Herrera
>> <alvherre@2ndquadrant.com> wrote:
>> >
>> > On 2018-Dec-27, Alexey Kondratov wrote:
>> >
>> > > To summarize:
>> > >
>> > > 1) Alvaro and Michael agreed, that REINDEX with tablespace move may be
>> > > useful. This is done in the patch attached to my initial email. Adding
>> > > REINDEX to ALTER TABLE as new action seems quite questionable for me and not
>> > > completely semantically correct. ALTER already looks bulky.
>> >
>> > Agreed on these points.
>> 
>> As an alternative idea, I think we can have a new ALTER INDEX variants
>> that rebuilds the index while moving tablespace, something like ALTER
>> INDEX ... REBUILD SET TABLESPACE ....
> 
> +1
> 
> It seems the easiest way to have feature-full commands. If we put
> functionality of CLUSTER and VACUUM FULL to ALTER TABLE, and put
> functionality of REINDEX to ALTER INDEX, then CLUSTER, VACUUM FULL and
> REINDEX would be just syntax sugar.
> 

I definitely bought into the idea of 'change a data type, cluster, and 
change tablespace all in a single SQL command', but stuck with some 
architectural questions, when it got to the code.

Currently, the only one kind of table rewrite is done by ALTER TABLE. It 
is preformed by simply reading tuples one by one via 
table_scan_getnextslot and inserting into the new table via tuple_insert 
table access method (AM). In the same time, CLUSTER table rewrite is 
implemented as a separated table AM relation_copy_for_cluster, which is 
actually a direct link to the heap AM heapam_relation_copy_for_cluster. 
Basically speaking, CLUSTER table rewrite happens 2 abstraction layers 
lower than ALTER TABLE one. Furthermore, CLUSTER seems to be a 
heap-specific AM and may be meaningless for some other storages, which 
is even more important because of coming pluggable storages, isn't it?

Maybe I overly complicate the problem, but to perform a data type change 
(or any other ALTER TABLE modification), cluster, and change tablespace 
in a row we have to bring all this high-level stuff done by ALTER TABLE 
to heapam_relation_copy_for_cluster. But is it even possible without 
leaking abstractions?

I'm working toward adding REINDEX to ALTER INDEX, so it was possible to 
do 'ALTER INDEX ... REINDEX CONCURRENTLY SET TABLESPACE ...', but ALTER 
TABLE + CLUSTER/VACUUM FULL is quite questionable for me now.

Anyway, new patch, which adds SET TABLESPACE to REINDEX is attached and 
this functionality seems really useful, so I will be very appreciate if 
someone will take a look on it.


Regards
--
Alexey Kondratov
Postgres Professional https://www.postgrespro.com
The Russian Postgres Company