MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII;
 format=flowed
Content-Transfer-Encoding: 7bit
Date: Mon, 30 Mar 2020 21:02:22 +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: <20200328001112.GX20103@telsasoft.com>
References: <c8ed43871e171ca2077aa9e1d0ed970a@postgrespro.ru>
 <20200211164848.GO1412@telsasoft.com>
 <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>
User-Agent: Roundcube Webmail/1.4.0
Message-ID: <f85b48beddc77c953f444cef06e1c16f@postgrespro.ru>
Precedence: bulk

On 2020-03-28 03:11, Justin Pryzby wrote:
> On Thu, Mar 26, 2020 at 11:01:06PM -0500, Justin Pryzby wrote:
>> > Another issue is this:
>> > > +VACUUM ( FULL [, ...] ) [ TABLESPACE <replaceable class="parameter">new_tablespace</replaceable> ] [ <replaceable class="parameter">table_and_columns</replaceable> [, ...] ]
>> > As you mentioned in your v1 patch, in the other cases, "tablespace
>> > [tablespace]" is added at the end of the command rather than in the middle.  I
>> > wasn't able to make that work, maybe because "tablespace" isn't a fully
>> > reserved word (?).  I didn't try with "SET TABLESPACE", although I understand
>> > it'd be better without "SET".
> 

SET does not change anything in my experience. The problem is that 
opt_vacuum_relation_list is... optional and TABLESPACE is not a fully 
reserved word (why?) as you correctly noted. I have managed to put 
TABLESPACE to the end, but with vacuum_relation_list, like:

| VACUUM opt_full opt_freeze opt_verbose opt_analyze 
vacuum_relation_list TABLESPACE name
| VACUUM '(' vac_analyze_option_list ')' vacuum_relation_list TABLESPACE 
name

It means that one would not be able to do VACUUM FULL of the entire 
database + TABLESPACE change. I do not think that it is a common 
scenario, but this limitation would be very annoying, wouldn't it?

> 
>> 
>> I think we should use the parenthesized syntax for vacuum - it seems 
>> clear in
>> hindsight.
>> 
>> Possibly REINDEX should use that, too, instead of adding OptTablespace 
>> at the
>> end.  I'm not sure.
> 
> The attached mostly implements generic parenthesized options to 
> REINDEX(...),
> so I'm soliciting opinions: should TABLESPACE be implemented in 
> parenthesized
> syntax or non?
> 
>> CLUSTER doesn't support parenthesized syntax, but .. maybe it should?
> 
> And this ?
> 

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:

- In the beginning for boolean options only, e.g. VACUUM
- In the end for options of a various type, but accompanied by WITH, 
e.g. COPY, CREATE SUBSCRIPTION

Moreover, TABLESPACE is already used in CREATE TABLE/INDEX in the same 
way I did in 0001-0002. That way, putting TABLESPACE option into the 
parenthesized options list does not look to be convenient and 
semantically correct, so I do not like it. Maybe others will have a 
different opinion.

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.

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


Regards
-- 
Alexey Kondratov

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