Date: Wed, 9 Sep 2020 10:36:29 -0500
From: Justin Pryzby <pryzby@telsasoft.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>,
	Alexey Kondratov <a.kondratov@postgrespro.ru>,
	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: <20200909153629.GG18552@telsasoft.com>
References: <20200908233951.GC18552@telsasoft.com>
 <20200909000238.GA1563@alvherre.pgsql>
 <20200909001757.GD18552@telsasoft.com>
 <20200909122200.GA2743@paquier.xyz>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20200909122200.GA2743@paquier.xyz>
User-Agent: Mutt/1.9.4 (2018-02-28)
Precedence: bulk

On Wed, Sep 09, 2020 at 09:22:00PM +0900, Michael Paquier wrote:
> On Tue, Sep 08, 2020 at 07:17:58PM -0500, Justin Pryzby wrote:
> > Initially I added List *params, and Michael suggested to retire
> > ReindexStmt->concurrent.  I provided a patch to do so, initially by leaving int
> > options and then, after this, removing it to "complete the thought", and get
> > rid of the remnants of the "old way" of doing it.  This is also how vacuum and
> > explain are done.
> > https://www.postgresql.org/message-id/20200902022410.GA20149%40telsasoft.com
> 
> Defining a set of DefElem when parsing and then using the int
> "options" with bitmasks where necessary at the beginning of the
> execution looks like a good balance to me.  This way, you can extend
> the grammar to use things like (verbose = true), etc.

It doesn't need to be extended - defGetBoolean already handles that.  I don't
see what good can come from storing the information in two places in the same
structure.

|postgres=# CLUSTER (VERBOSE on) pg_attribute USING pg_attribute_relid_attnum_index ;
|INFO:  clustering "pg_catalog.pg_attribute" using index scan on "pg_attribute_relid_attnum_index"
|INFO:  "pg_attribute": found 0 removable, 2968 nonremovable row versions in 55 pages
|DETAIL:  0 dead row versions cannot be removed yet.
|CPU: user: 0.01 s, system: 0.00 s, elapsed: 0.01 s.
|CLUSTER

-- 
Justin