Re: Allow CLUSTER, VACUUM FULL and REINDEX to change tablespace on the fly
Justin Pryzby <pryzby@telsasoft.com>
From: Justin Pryzby <pryzby@telsasoft.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Alexey Kondratov <a.kondratov@postgrespro.ru>, Masahiko Sawada <masahiko.sawada@2ndquadrant.com>, Steve Singer <steve@ssinger.info>, pgsql-hackers@lists.postgresql.org, Alvaro Herrera <alvherre@2ndquadrant.com>, Robert Haas <robertmhaas@gmail.com>, Alexander Korotkov <a.korotkov@postgrespro.ru>, Masahiko Sawada <sawada.mshk@gmail.com>, Jose Luis Tallon <jltallon@adv-solutions.net>
Date: 2020-04-01T11:57:18Z
Lists: pgsql-hackers
Attachments
- v15-0001-Allow-REINDEX-to-change-tablespace.patch (text/x-diff)
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
Commits
-
Add TABLESPACE option to REINDEX
- c5b286047cd6 14.0 landed
-
Refactor code in tablecmds.c to check and process tablespace moves
- 4c9c359d38ff 14.0 landed
-
Refactor option handling of CLUSTER, REINDEX and VACUUM
- a3dc926009be 14.0 landed
-
pg_dump: Don't use enums for defining bit mask values
- d2a2808eb444 14.0 landed
-
Refactor CLUSTER and REINDEX grammar to use DefElem for option lists
- b5913f612079 14.0 landed
-
Refactor parsing rules for option lists of EXPLAIN, VACUUM and ANALYZE
- 873ea9ee692e 14.0 landed
-
Improve tab completion of REINDEX in psql
- 1f75b454134c 14.0 landed
-
Fix possible crash during FATAL exit from reindexing.
- d12bdba77b0f 13.0 cited