agora inbox for pgsql-hackers@postgresql.org  
help / color / mirror / Atom feed
From: Justin Pryzby <pryzby@telsasoft.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Alexey Kondratov <a.kondratov@postgrespro.ru>
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: Sun, 9 Aug 2020 21:24:43 -0500
Message-ID: <20200810022443.GI20473@telsasoft.com> (raw)
In-Reply-To: <20200809110252.GD17986@paquier.xyz>
References: <20200401115718.GQ14618@telsasoft.com>
	<20200401130836.GT14618@telsasoft.com>
	<20200403182712.GR14618@telsasoft.com>
	<a0fd6af4bdf3cd209a69cea112ab10ea@postgrespro.ru>
	<20200406184406.GF2228@telsasoft.com>
	<9b6a6cec134451550b74aa2ffd1446d0@postgrespro.ru>
	<20200407204406.GR2228@telsasoft.com>
	<20200412013352.GA2491@telsasoft.com>
	<20200426175614.GT28974@telsasoft.com>
	<20200809110252.GD17986@paquier.xyz>

On Sun, Aug 09, 2020 at 08:02:52PM +0900, Michael Paquier wrote:
> I have been looking at 0001 as a start, and your patch is incorrect on
> a couple of aspects for the completion of REINDEX:
> - "(" is not proposed as a completion option after the initial
> REINDEX, and I think that it should.

That part of your patch handles REINDEX and REINDEX(*) differently than mine.
Yours is technically more correct/complete.  But, I recall Tom objected a
different patch because of completing to a single char.  I think the case is
arguable either way: if only some completions are shown, then it hides the
others..
https://www.postgresql.org/message-id/14255.1536781029@sss.pgh.pa.us

-       else if (Matches("REINDEX") || Matches("REINDEX", "(*)"))
+       else if (Matches("REINDEX"))
+               COMPLETE_WITH("TABLE", "INDEX", "SYSTEM", "SCHEMA", "DATABASE", "(");
+       else if (Matches("REINDEX", "(*)"))
                COMPLETE_WITH("TABLE", "INDEX", "SYSTEM", "SCHEMA", "DATABASE");

> - completion gets incorrect for all the commands once a parenthesized
> list of options is present, as CONCURRENTLY goes missing.

The rest of your patch looks fine.  In my mind, REINDEX(CONCURRENTLY) was the
"new way" to write things, and it's what's easy to support, so I think I didn't
put special effort into making tab completion itself complete.

-- 
Justin





view thread (154+ messages)  latest in thread

Message-ID: <20200810022443.GI20473@telsasoft.com>
Permalink:  ../20200810022443.GI20473@telsasoft.com/
Also on:    postgresql.org/message-id/20200810022443.GI20473@telsasoft.com

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: pryzby@telsasoft.com, michael@paquier.xyz, a.kondratov@postgrespro.ru, 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: <20200810022443.GI20473@telsasoft.com>

* 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