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

From: Justin Pryzby <pryzby(at)telsasoft(dot)com>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Alexey Kondratov <a(dot)kondratov(at)postgrespro(dot)ru>, 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-01 11:57:18
Message-ID: 20200401115718.GQ14618@telsasoft.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Apr 01, 2020 at 03:03:34PM +0900, Michael Paquier wrote:
> On Tue, Mar 31, 2020 at 01:56:07PM +0300, Alexey Kondratov wrote:
> > I am fine with allowing REINDEX (CONCURRENTLY), but then we will have to
> > support both syntaxes as we already do for VACUUM. Anyway, if we agree to
> > add parenthesized options to REINDEX/CLUSTER, then it should be done as a
> > separated patch before the current patch set.
>
> would honestly prefer that for now on we only add the parenthesized
> version of an option if something new is added to such utility
> commands (vacuum, analyze, reindex, etc.) as that's much more
> extensible from the point of view of the parser. And this, even if
> you need to rework things a bit more things around
> reindex_option_elem for the tablespace option proposed here.

Thanks for your input.

I'd already converted VACUUM and REINDEX to use a parenthesized TABLESPACE
option, and just converted CLUSTER to take an option list and do the same.

Alexey suggested that those changes should be done as a separate patch, with
the tablespace options built on top. Which makes sense. I had quite some fun
rebasing these with patches in that order.

However, I've kept my changes separate from Alexey's patch, to make it easier
for him to integrate. So there's "fix!" commits which are not logically
separate and should be read as if they're merged with their parent commits.
That makes the patchset look kind of dirty. So I'm first going to send the
"before rebase" patchset. There's a few fixme items, but I think this is in
pretty good shape, and I'd appreciate review.

I'll follow up later with the "after rebase" patchset. Maybe Alexey will want
to integrate that.

I claimed it would be easy, so I also implemented (INDEX_TABESPACE ..) option:

template1=# VACUUM (TABLESPACE pg_default, INDEX_TABLESPACE ts, FULL) t;
template1=# CLUSTER (TABLESPACE pg_default, INDEX_TABLESPACE ts) t;

--
Justin

Attachment Content-Type Size
v15-0001-Allow-REINDEX-to-change-tablespace.patch text/x-diff 34.1 KB
v15-0002-Allow-CLUSTER-and-VACUUM-FULL-to-change-tablespa.patch text/x-diff 24.8 KB
v15-0003-fixes2.patch text/x-diff 4.2 KB
v15-0004-Parenthesized-syntax-VACUUM-FULL-TABLESPACE.patch text/x-diff 11.1 KB
v15-0005-Change-REINDEX-CLUSTER-to-accept-an-option-list.patch text/x-diff 31.6 KB
v15-0006-Implement-VACUUM-FULL-CLUSTER-INDEX_TABLESPACE-t.patch text/x-diff 17.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Justin Pryzby 2020-04-01 11:59:06 Re: [PATCH] Opclass parameters
Previous Message Alexander Korotkov 2020-04-01 11:53:41 Re: [PATCH] Opclass parameters