Date: Wed, 20 Jan 2021 15:34:39 -0300
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Alexey Kondratov <a.kondratov@postgrespro.ru>
Cc: Michael Paquier <michael@paquier.xyz>,
	Justin Pryzby <pryzby@telsasoft.com>,
	Peter Eisentraut <peter.eisentraut@enterprisedb.com>,
	Masahiko Sawada <masahiko.sawada@2ndquadrant.com>,
	Steve Singer <steve@ssinger.info>,
	pgsql-hackers@lists.postgresql.org,
	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
Message-ID: <20210120183439.GA21339@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <03f88f70618ce73e75837b6125a143f7@postgrespro.ru>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk

On 2021-Jan-20, Alexey Kondratov wrote:

> On 2021-01-20 21:08, Alexey Kondratov wrote:
> > 
> > I did a refactoring of ATExecSetTableSpaceNoStorage() in the 0001. New
> > function SetRelTablesapce() is placed into the tablecmds.c. Following
> > 0002 gets use of it. Is it close to what you and Michael suggested?
> 
> Ugh, forgot to attach the patches. Here they are.

Yeah, looks reasonable.

> +	/* No work if no change in tablespace. */
> +	oldTablespaceOid = rd_rel->reltablespace;
> +	if (tablespaceOid != oldTablespaceOid ||
> +		(tablespaceOid == MyDatabaseTableSpace && OidIsValid(oldTablespaceOid)))
> +	{
> +		/* Update the pg_class row. */
> +		rd_rel->reltablespace = (tablespaceOid == MyDatabaseTableSpace) ?
> +			InvalidOid : tablespaceOid;
> +		CatalogTupleUpdate(pg_class, &tuple->t_self, tuple);
> +
> +		changed = true;
> +	}
> +
> +	if (changed)
> +		/* Record dependency on tablespace */
> +		changeDependencyOnTablespace(RelationRelationId,
> +									 reloid, rd_rel->reltablespace);

Why have a separate "if (changed)" block here instead of merging with
the above?


-- 
Álvaro Herrera                            39°49'30"S 73°17'W