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

From: Alexey Kondratov <a(dot)kondratov(at)postgrespro(dot)ru>
To: Justin Pryzby <pryzby(at)telsasoft(dot)com>
Cc: Michael Paquier <michael(at)paquier(dot)xyz>, Masahiko Sawada <masahiko(dot)sawada(at)2ndquadrant(dot)com>, Steve Singer <steve(at)ssinger(dot)info>, pgsql-hackers(at)lists(dot)postgresql(dot)org, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Jose Luis Tallon <jltallon(at)adv-solutions(dot)net>
Subject: Re: Allow CLUSTER, VACUUM FULL and REINDEX to change tablespace on the fly
Date: 2020-03-30 18:02:22
Message-ID: f85b48beddc77c953f444cef06e1c16f@postgrespro.ru
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Ranier Vilela 2020-03-30 18:07:40 [PATCH] remove condition always true (/src/backend/access/heap/vacuumlazy.c)
Previous Message Tom Lane 2020-03-30 18:00:42 Re: fix for BUG #3720: wrong results at using ltree