MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII;
 format=flowed
Content-Transfer-Encoding: 7bit
Date: Tue, 31 Mar 2020 13:56:07 +0300
From: Alexey Kondratov <a.kondratov@postgrespro.ru>
To: Justin Pryzby <pryzby@telsasoft.com>
Cc: Michael Paquier <michael@paquier.xyz>, 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>
Subject: Re: Allow CLUSTER, VACUUM FULL and REINDEX to change tablespace on
 the fly
In-Reply-To: <20200330183439.GQ20103@telsasoft.com>
References: <eb4cdddc0d6197f3fef15d36758c93fe@postgrespro.ru>
 <20200229145304.GI29456@telsasoft.com>
 <20200309200447.GA32459@telsasoft.com>
 <cb874a90-3946-91e4-4c31-5075f425b280@postgrespro.ru>
 <20200325234027.GH21443@telsasoft.com>
 <39a055a7a8cd19552f35f4cb98a8cd80@postgrespro.ru>
 <20200326180112.GH17431@telsasoft.com>
 <20200327040106.GC20103@telsasoft.com>
 <20200328001112.GX20103@telsasoft.com>
 <f85b48beddc77c953f444cef06e1c16f@postgrespro.ru>
 <20200330183439.GQ20103@telsasoft.com>
User-Agent: Roundcube Webmail/1.4.0
Message-ID: <624921b1ebca9897173d85c424c00646@postgrespro.ru>
Precedence: bulk

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