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

On 2020-03-30 21:34, Justin Pryzby wrote:
> On Mon, Mar 30, 2020 at 09:02:22PM +0300, Alexey Kondratov wrote:
>> Hmm, I went through the well known to me SQL commands in Postgres and 
>> a bit
>> more. Parenthesized options list is mostly used in two common cases:
> 
> There's also ANALYZE(VERBOSE), REINDEX(VERBOSE).
> There was debate a year ago [0] as to whether to make "reindex 
> CONCURRENTLY" a
> separate command, or to use parenthesized syntax "REINDEX 
> (CONCURRENTLY)".  I
> would propose to support that now (and implemented that locally).
> 

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.

> 
> ..and explain(...)
> 
>> - In the beginning for boolean options only, e.g. VACUUM
> 
> You're right that those are currently boolean, but note that 
> explain(FORMAT ..)
> is not boolean.
> 

Yep, I forgot EXPLAIN, this is a good example.

> 
> .. and create table (LIKE ..)
> 

LIKE is used in the table definition, so it is a slightly different 
case.

> 
>> Putting it into the WITH (...) options list looks like an option to 
>> me.
>> However, doing it only for VACUUM will ruin the consistency, while 
>> doing it
>> for CLUSTER and REINDEX is not necessary, so I do not like it either.
> 
> It's not necessary but I think it's a more flexible way to add new
> functionality (requiring no changes to the grammar for vacuum, and for
> REINDEX/CLUSTER it would allow future options to avoid changing the 
> grammar).
> 
> If we use parenthesized syntax for vacuum, my proposal is to do it for
> REINDEX, and
> consider adding parenthesized syntax for cluster, too.
> 
>> To summarize, currently I see only 2 + 1 extra options:
>> 
>> 1) Keep everything with syntax as it is in 0001-0002
>> 2) Implement tail syntax for VACUUM, but with limitation for VACUUM 
>> FULL of
>> the entire database + TABLESPACE change
>> 3) Change TABLESPACE to a fully reserved word
> 
> + 4) Use parenthesized syntax for all three.
> 
> Note, I mentioned that maybe VACUUM/CLUSTER should support not only 
> "TABLESPACE
> foo" but also "INDEX TABLESPACE bar" (I would use that, too).  I think 
> that
> would be easy to implement, and for sure it would suggest using () for 
> both.
> (For sure we don't want to implement "VACUUM t TABLESPACE foo" now, and 
> then
> later implement "INDEX TABLESPACE bar" and realize that for consistency 
> we
> cannot parenthesize it.
> 
> Michael ? Alvaro ? Robert ?
> 

Yes, I would be glad to hear other opinions too, before doing this 
preliminary refactoring.


-- 
Alexey Kondratov

Postgres Professional https://www.postgrespro.com
Russian Postgres Company