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>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Masahiko Sawada <masahiko(dot)sawada(at)2ndquadrant(dot)com>, Steve Singer <steve(at)ssinger(dot)info>, pgsql-hackers(at)lists(dot)postgresql(dot)org, 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-09-02 21:00:17
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 2020-09-02 07:56, Justin Pryzby wrote:
> Done in the attached, which is also rebased on 1d6541666.
> And added RelationAssumeNewRelfilenode() as I mentioned - but I'm
> hoping to
> hear from Michael about any reason not to call
> RelationSetNewRelfilenode()
> instead of directly calling the things it would itself call.

The latest patch set immediately got a conflict with Michael's fixup
01767533, so I've rebased it first of all.

+ Prints a progress report as each table is clustered.
+<!-- When specified within parenthesis, <literal>VERBOSE</literal> may
be followed by a boolean ...-->

I think that we can remove this comment completely as it is already
explained in the docs later.

+ | CLUSTER opt_verbose '(' vac_analyze_option_list ')' qualified_name
+ {

What's the point in allowing a mixture of old options with new
parenthesized option list? VACUUM doesn't do so. I can understand it for
REINDEX CONCURRENTLY, since parenthesized options were already there,
but not for CLUSTER.

With v23 it is possible to write:


which is untidy. Furthermore, 'CLUSTER VERBOSE (' is tab-completed to
'CLUSTER VERBOSE (USING'. That way I propose to only allow either new
options or old similarly to the VACUUM. See attached 0002.


Tab completion in the CLUSTER was broken for parenthesized options, so
I've fixed it in the 0005.

Also, I noticed that you used vac_analyze_option_list instead of
reindex_option_list and I checked other option lists in the grammar.
I've found that explain_option_list and vac_analyze_option_list are
identical, so it makes sense to leave just one of them and rename it to,
e.g., common_option_list in order to use it everywhere needed (REINDEX,
VACUUM, EXPLAIN, CLUSTER, ANALYZE). The whole grammar is already
complicated enough to keep the exact duplicates and new options will be
added to the lists in the backend code, not parser. What do you think?

It is done in the 0007 attached. I think it should be applied altogether
with 0001 or before/after, but I put this as the last patch in the set
in order to easier discard it if others would disagree.

Otherwise, everything seems to be working fine. Cannot find any problems
so far.

Alexey Kondratov

Postgres Professional
Russian Postgres Company

Attachment Content-Type Size
v24-0007-Refactor-gram.y-in-order-to-add-a-common-parenth.patch text/x-diff 6.0 KB
v24-0006-Implement-vacuum-full-cluster-INDEX_TABLESPACE-t.patch text/x-diff 19.3 KB
v24-0005-Allow-CLUSTER-and-VACUUM-FULL-to-change-tablespa.patch text/x-diff 23.6 KB
v24-0004-Allow-REINDEX-to-change-tablespace.patch text/x-diff 29.6 KB
v24-0003-Deprecate-ReindexStmt-concurrent-and-options.patch text/x-diff 15.9 KB
v24-0002-Allow-either-new-or-old-option-syntax-for-CLUSTE.patch text/x-diff 2.4 KB
v24-0001-Change-REINDEX-CLUSTER-to-accept-an-option-list.patch text/x-diff 23.6 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2020-09-02 21:18:53 Re: Fix for configure error in 9.5/9.6 on macOS 11.0 Big Sur
Previous Message Jesse Zhang 2020-09-02 20:43:54 Re: Fix for configure error in 9.5/9.6 on macOS 11.0 Big Sur