Re: Allow CLUSTER, VACUUM FULL and REINDEX to change tablespace on the 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(dot)mshk(at)gmail(dot)com> wrote:
>> On Thu, Dec 27, 2018 at 10:24 PM Alvaro Herrera
>> <alvherre(at)2ndquadrant(dot)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