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