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

From: Justin Pryzby <pryzby(at)telsasoft(dot)com>
To: Alexey Kondratov <a(dot)kondratov(at)postgrespro(dot)ru>
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-04-07 20:44:06
Message-ID: 20200407204406.GR2228@telsasoft.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Apr 07, 2020 at 03:40:18PM +0300, Alexey Kondratov wrote:
> On 2020-04-06 21:44, Justin Pryzby wrote:
> > On Mon, Apr 06, 2020 at 08:43:46PM +0300, Alexey Kondratov wrote:
> > >
> > > +/* XXX: reusing reindex_option_list */
> > > + | CLUSTER opt_verbose '(' reindex_option_list ')' qualified_name
> > > cluster_index_specification
> > >
> > > Could we actually simply reuse vac_analyze_option_list? From the first
> > > sight it does just the right thing, excepting the special handling of
> > > spelling ANALYZE/ANALYSE, but it does not seem to be a problem.
> >
> > Hm, do you mean to let cluster.c reject the other options like "analyze" ?
> > I'm not sure why that would be better than reusing reindex? I think the
> > suggestion will probably be to just copy+paste the reindex option list and
> > rename it to cluster (possibly with the explanation that they're separate
> > and independant and so their behavior shouldn't be tied together).
>
> I mean to literally use vac_analyze_option_list for reindex and cluster as
> well. Please, check attached 0007. Now, vacuum, reindex and cluster filter
> options list and reject everything that is not supported, so it seems
> completely fine to just reuse vac_analyze_option_list, doesn't it?

It's fine with me :)

Possibly we could rename vac_analyze_option_list as generic_option_list.

I'm resending the patchset like that, and fixed cluster/index to handle not
just "VERBOSE" but "verbose OFF", rather than just ignoring the argument.

That's the last known issue with the patch. I doubt anyone will elect to pick
it up in the next 8 hours, but I think it's in very good shape for v14 :)

BTW, if you resend a *.patch or *.diff file to a thread, it's best to also
include all the previous patches. Otherwise the CF bot is likely to complain
that the patch "doesn't apply", or else it'll only test the one patch instead
of the whole series.
http://cfbot.cputube.org/alexey-kondratov.html

--
Justin

Attachment Content-Type Size
v19-0001-tab-completion-for-reindex-verbose.patch text/x-diff 1.8 KB
v19-0002-Change-REINDEX-CLUSTER-to-accept-an-option-list.patch text/x-diff 23.1 KB
v19-0003-Allow-REINDEX-to-change-tablespace.patch text/x-diff 26.9 KB
v19-0004-Allow-CLUSTER-and-VACUUM-FULL-to-change-tablespa.patch text/x-diff 24.0 KB
v19-0005-Implement-vacuum-full-cluster-INDEX_TABLESPACE-t.patch text/x-diff 19.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tomas Vondra 2020-04-07 20:54:36 implicit declaration of datumIsEqual in parse_coerce.c
Previous Message Andres Freund 2020-04-07 20:42:10 Re: Improving connection scalability: GetSnapshotData()