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

On Mon, Nov 30, 2020 at 05:12:42PM +0300, Alexey Kondratov wrote:
> Thanks. I have rebased the remaining patches on top of 873ea9ee to use
> 'utility_option_list' instead of 'common_option_list'.

Thanks, that helps a lot. I have gone through 0002, and tweaked it as
the attached (note that this patch is also interesting for another
thing in development: backend-side reindex filtering of
collation-sensitive indexes). Does that look right to you?

These are mostly matters of consistency with the other commands using
DefElem, but I think that it is important to get things right:
- Having the list of options in parsenodes.h becomes incorrect,
because these get now only used at execution time, like VACUUM. So I
have moved that to cluster.h and index.h.
- Let's use an enum for REINDEX, like the others.
- Having parse_reindex_params() in utility.c is wrong for something
aimed at being used only for REINDEX, so I have moved that to
indexcmds.c, and renamed the routine to be more consistent with the
rest. I think that we could more here by having an ExecReindex() that
does all the work based on object types, but I have left that out for
now to keep the change minimal.
- Switched one of the existing tests to stress CONCURRENTLY within
parenthesis.
- Indented the whole.

A couple of extra things below.

* CLUSTER [VERBOSE] <qualified_name> [ USING <index_name> ]
+ * CLUSTER [VERBOSE] [(options)] <qualified_name> [ USING <index_name> ]
This line is wrong, and should be:
CLUSTER [ (options) ] <qualified_name> [ USING <index_name> ]

+CLUSTER [VERBOSE] [ ( <replaceable class="parameter">option</replaceable>
+CLUSTER [VERBOSE] [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ]
The docs in cluster.sgml are wrong as well, you can have VERBOSE as a
single option or as a parenthesized option, but never both at the same
time. On the contrary, psql completion got that right. I was first a
bit surprised that you would not allow the parenthesized set for the
case where a relation is not specified in the command, but I agree
that this does not seem worth the extra complexity now as this thread
aims at being able to use TABLESPACE which makes little sense
database-wide.

- VERBOSE
+ VERBOSE [ <replaceable class="parameter">boolean</replaceable> ]
Forgot about CONCURRENTLY as an option here, as this becomes
possible.
--
Michael