agora inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feedFrom: Alexey Kondratov <a.kondratov@postgrespro.ru>
To: Justin Pryzby <pryzby@telsasoft.com>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: Masahiko Sawada <masahiko.sawada@2ndquadrant.com>
Cc: Steve Singer <steve@ssinger.info>
Cc: pgsql-hackers@lists.postgresql.org, Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: Alexander Korotkov <a.korotkov@postgrespro.ru>
Cc: Masahiko Sawada <sawada.mshk@gmail.com>
Cc: Jose Luis Tallon <jltallon@adv-solutions.net>
Subject: Re: Allow CLUSTER, VACUUM FULL and REINDEX to change tablespace on the fly
Date: Thu, 26 Mar 2020 22:22:08 +0300
Message-ID: <39c239a12c4dde0a3c78e2ab5d8eb55b@postgrespro.ru> (raw)
In-Reply-To: <20200326180112.GH17431@telsasoft.com>
References: <3ae48673-283c-3e99-3dd8-36ebb81614b5@postgrespro.ru>
<20191202082134.GI1696@paquier.xyz>
<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>
On 2020-03-26 21:01, Justin Pryzby wrote:
>
>> @@ -262,7 +280,7 @@ cluster(ClusterStmt *stmt, bool isTopLevel)
>> * and error messages should refer to the operation as VACUUM not
>> CLUSTER.
>> */
>> void
>> -cluster_rel(Oid tableOid, Oid indexOid, int options)
>> +cluster_rel(Oid tableOid, Oid indexOid, Oid tablespaceOid, int
>> options)
>
> Add a comment here about the tablespaceOid parameter, like the other
> functions
> where it's added.
>
> The permission checking is kind of duplicitive, so I'd suggest to
> factor it
> out. Ideally we'd only have one place that checks for
> pg_global/system/mapped.
> It needs to check that it's not a system relation, or that
> system_table_mods
> are allowed, and in any case that if it's a mapped rel, that it's not
> being
> moved. I would pass a boolean indicating if the tablespace is being
> changed.
>
Yes, but I wanted to make sure first that all necessary validations are
there to do not miss something as I did last time. I do not like
repetitive code either, so I would like to introduce more common check
after reviewing the code as a whole.
>
> 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".
>
Initially I tried "SET TABLESPACE", but also failed to completely get
rid of shift/reduce conflicts. I will try to rewrite VACUUM's part again
with OptTableSpace. Maybe I will manage it this time.
I will take into account all your text edits as well.
Thanks
--
Alexey Kondratov
Postgres Professional https://www.postgrespro.com
Russian Postgres Company
view thread (154+ messages) latest in thread
Message-ID: <39c239a12c4dde0a3c78e2ab5d8eb55b@postgrespro.ru>
Permalink: ../39c239a12c4dde0a3c78e2ab5d8eb55b@postgrespro.ru/
Also on: postgresql.org/message-id/39c239a12c4dde0a3c78e2ab5d8eb55b@postgrespro.ru
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: a.kondratov@postgrespro.ru, pryzby@telsasoft.com, michael@paquier.xyz, masahiko.sawada@2ndquadrant.com, steve@ssinger.info, alvherre@2ndquadrant.com, robertmhaas@gmail.com, a.korotkov@postgrespro.ru, sawada.mshk@gmail.com, jltallon@adv-solutions.net
Subject: Re: Allow CLUSTER, VACUUM FULL and REINDEX to change tablespace on the fly
In-Reply-To: <39c239a12c4dde0a3c78e2ab5d8eb55b@postgrespro.ru>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox