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

From: Alexey Kondratov <a(dot)kondratov(at)postgrespro(dot)ru>
To: Justin Pryzby <pryzby(at)telsasoft(dot)com>
Cc: Michael Paquier <michael(at)paquier(dot)xyz>, Masahiko Sawada <masahiko(dot)sawada(at)2ndquadrant(dot)com>, Steve Singer <steve(at)ssinger(dot)info>, pgsql-hackers(at)lists(dot)postgresql(dot)org, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Jose Luis Tallon <jltallon(at)adv-solutions(dot)net>
Subject: Re: Allow CLUSTER, VACUUM FULL and REINDEX to change tablespace on the fly
Date: 2020-02-29 12:35:27
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 2020-02-11 19:48, Justin Pryzby wrote:
> For your v7 patch, which handles REINDEX to a new tablespace, I have a
> few
> minor comments:
> + * the relation will be rebuilt. If InvalidOid is used, the default
> => should say "currrent", not default ?

Yes, it keeps current index tablespace in that case, thanks.

> +++ b/doc/src/sgml/ref/reindex.sgml
> + <term><literal>TABLESPACE</literal></term>
> ...
> + <term><replaceable
> class="parameter">new_tablespace</replaceable></term>
> => I saw you split the description of TABLESPACE from new_tablespace
> based on
> comment earlier in the thread, but I suggest that the descriptions for
> these
> should be merged, like:
> + <varlistentry>
> + <term><literal>TABLESPACE</literal><replaceable
> class="parameter">new_tablespace</replaceable></term>
> + <listitem>
> + <para>
> + Allow specification of a tablespace where all rebuilt indexes
> will be created.
> + Cannot be used with "mapped" relations. If
> <literal>SCHEMA</literal>,
> + <literal>DATABASE</literal> or <literal>SYSTEM</literal> are
> specified, then
> + all unsuitable relations will be skipped and a single
> <literal>WARNING</literal>
> + will be generated.
> + </para>
> + </listitem>
> + </varlistentry>

It sounds good to me, but here I just obey the structure, which is used
all around. Documentation of ALTER TABLE/DATABASE, REINDEX and many
others describes each literal/parameter in a separate entry, e.g.
new_tablespace. So I would prefer to keep it as it is for now.

> The existing patch is very natural, especially the parts in the
> original patch
> handling vacuum full and cluster. Those were removed to concentrate on
> REINDEX, and based on comments that it might be nice if ALTER handled
> and VACUUM FULL. On a separate thread, I brought up the idea of ALTER
> using
> clustered order. Tom pointed out some issues with my implementation,
> but
> didn't like the idea, either.
> So I suggest to re-include the CLUSTER/VAC FULL parts as a separate
> 0002 patch,
> the same way they were originally implemented.
> BTW, I think if "ALTER" were updated to support REINDEX (to allow
> multiple
> operations at once), it might be either:
> |ALTER INDEX i SET TABLESPACE , REINDEX -- to reindex a single index
> on a given tlbspc
> or
> inds on table inds moved to a given tblspc
> "USING INDEX TABLESPACE" is already used for ALTER..ADD column/table

Yes, I also think that allowing REINDEX/CLUSTER/VACUUM FULL to put
resulting relation in a different tablespace is a very natural
operation. However, I did a couple of attempts to integrate latter two
with ALTER TABLE and failed with it, since it is already complex enough.
I am still willing to proceed with it, but not sure how soon it will be.

Anyway, new version is attached. It is rebased in order to resolve
conflicts with a recent fix of REINDEX CONCURRENTLY + temp relations,
and includes this small comment fix.

Alexey Kondratov

Postgres Professional
The Russian Postgres Company

Attachment Content-Type Size
v8-0001-Allow-REINDEX-to-change-tablespace.patch text/x-diff 33.0 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Fabien COELHO 2020-02-29 14:29:19 Re: pgbench: option delaying queries till connections establishment?
Previous Message Tomas Vondra 2020-02-29 12:24:41 Re: SLRU statistics