Re: strange CREATE INDEX tab completion cases

These two tab completion pieces look strange to me:

/* If we have CREATE|UNIQUE INDEX <sth> CONCURRENTLY, then add "ON" */
else if ((pg_strcasecmp(prev3_wd, "INDEX") == 0 ||
pg_strcasecmp(prev2_wd, "INDEX") == 0) &&
pg_strcasecmp(prev_wd, "CONCURRENTLY") == 0)
COMPLETE_WITH_CONST("ON");
/* If we have CREATE|UNIQUE INDEX <sth>, then add "ON" or "CONCURRENTLY" */
else if ((pg_strcasecmp(prev3_wd, "CREATE") == 0 ||
pg_strcasecmp(prev3_wd, "UNIQUE") == 0) &&
pg_strcasecmp(prev2_wd, "INDEX") == 0)
{
static const char *const list_CREATE_INDEX[] =
{"CONCURRENTLY", "ON", NULL};

COMPLETE_WITH_LIST(list_CREATE_INDEX);
}

They appear to support a syntax along the lines of

CREATE INDEX name CONCURRENTLY

which is not the actual syntax.

On Sat, Dec 12, 2015 at 11:17 AM, Peter Eisentraut <peter_e(at)gmx(dot)net> wrote:
> These two tab completion pieces look strange to me:
>
> /* If we have CREATE|UNIQUE INDEX <sth> CONCURRENTLY, then add "ON" */
> else if ((pg_strcasecmp(prev3_wd, "INDEX") == 0 ||
> pg_strcasecmp(prev2_wd, "INDEX") == 0) &&
> pg_strcasecmp(prev_wd, "CONCURRENTLY") == 0)
> COMPLETE_WITH_CONST("ON");
> /* If we have CREATE|UNIQUE INDEX <sth>, then add "ON" or "CONCURRENTLY" */
> else if ((pg_strcasecmp(prev3_wd, "CREATE") == 0 ||
> pg_strcasecmp(prev3_wd, "UNIQUE") == 0) &&
> pg_strcasecmp(prev2_wd, "INDEX") == 0)
> {
> static const char *const list_CREATE_INDEX[] =
> {"CONCURRENTLY", "ON", NULL};
>
> COMPLETE_WITH_LIST(list_CREATE_INDEX);
> }
>
> They appear to support a syntax along the lines of
>
> CREATE INDEX name CONCURRENTLY
>
> which is not the actual syntax.

Yep. That's visibly a bug introduced by this commit:
commit: 37ec19a15ce452ee94f32ebc3d6a9a45868e82fd
author: Itagaki Takahiro <itagaki(dot)takahiro(at)gmail(dot)com>
date: Wed, 17 Feb 2010 04:09:40 +0000
Support new syntax and improve handling of parentheses in psql tab-completion.

The current implementation is missing a correct completion in a couple
of cases, among them:
-- Should be only ON
=# create index asd
CONCURRENTLY ON
-- should give list of table
=# create index CONCURRENTLY aaa on
-- Should give ON and list of existing indexes
=# create index concurrently

Please see the attached to address those things (and others) with
extra fixes for a couple of comments.
Regards,
--
Michael

On 12/13/15 9:16 AM, Michael Paquier wrote:
> Please see the attached to address those things (and others) with
> extra fixes for a couple of comments.

I have ported these changes to the new world order and divided them up
into more logical changes that are more clearly documented. Please
check that this matches what you had in mind.

On Mon, Jan 11, 2016 at 2:16 AM, Peter Eisentraut <peter_e(at)gmx(dot)net> wrote:
> On 12/13/15 9:16 AM, Michael Paquier wrote:
>> Please see the attached to address those things (and others) with
>> extra fixes for a couple of comments.
>
> I have ported these changes to the new world order and divided them up
> into more logical changes that are more clearly documented. Please
> check that this matches what you had in mind.

Thanks for the new patches, this fell of my radar. This new version
looks fine to me.

+ /* If we have CREATE|UNIQUE INDEX CONCURRENTLY, then add "ON"
and existing
+ indexes */
This comment format is not correct.
--
Michael

Peter Eisentraut wrote:
> On 12/13/15 9:16 AM, Michael Paquier wrote:
> > Please see the attached to address those things (and others) with
> > extra fixes for a couple of comments.
>
> I have ported these changes to the new world order and divided them up
> into more logical changes that are more clearly documented. Please
> check that this matches what you had in mind.

One thing I just noticed is that CREATE INDEX CONCURRENTLY cannot be
used within CREATE SCHEMA, so perhaps the lines that match the
CONCURRENTLY keyword should use Matches() rather than TailMatches().
Similarly (but perhaps this is not workable) the lines that TailMatch()
but do not Match() should not offer CONCURRENTLY after INDEX.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> writes:
> One thing I just noticed is that CREATE INDEX CONCURRENTLY cannot be
> used within CREATE SCHEMA, so perhaps the lines that match the
> CONCURRENTLY keyword should use Matches() rather than TailMatches().
> Similarly (but perhaps this is not workable) the lines that TailMatch()
> but do not Match() should not offer CONCURRENTLY after INDEX.

This seems overcomplicated. I don't think there's any expectation that
tab completion is 100% right all the time. Let's just treat CREATE INDEX
CONCURRENTLY the same as CREATE INDEX.

regards, tom lane