strange CREATE INDEX tab completion cases

Started by Peter Eisentrautover 10 years ago6 messageshackers
Jump to latest
#1Peter Eisentraut
peter_e@gmx.net

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.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#2Michael Paquier
michael@paquier.xyz
In reply to: Peter Eisentraut (#1)
Re: strange CREATE INDEX tab completion cases

On Sat, Dec 12, 2015 at 11:17 AM, Peter Eisentraut <peter_e@gmx.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.takahiro@gmail.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

Attachments:

20151213_psql_completion_index.patchbinary/octet-stream; name=20151213_psql_completion_index.patchDownload+22-12
#3Peter Eisentraut
peter_e@gmx.net
In reply to: Michael Paquier (#2)
Re: strange CREATE INDEX tab completion cases

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.

Attachments:

0001-psql-Update-tab-completion-comment.patchapplication/x-patch; name=0001-psql-Update-tab-completion-comment.patchDownload+2-2
0002-psql-Fix-CREATE-INDEX-tab-completion.patchapplication/x-patch; name=0002-psql-Fix-CREATE-INDEX-tab-completion.patchDownload+4-6
0003-psql-Improve-CREATE-INDEX-CONCURRENTLY-tab-completio.patchapplication/x-patch; name=0003-psql-Improve-CREATE-INDEX-CONCURRENTLY-tab-completio.patchDownload+9-10
#4Michael Paquier
michael@paquier.xyz
In reply to: Peter Eisentraut (#3)
Re: strange CREATE INDEX tab completion cases

On Mon, Jan 11, 2016 at 2:16 AM, Peter Eisentraut <peter_e@gmx.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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#5Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Peter Eisentraut (#3)
Re: strange CREATE INDEX tab completion cases

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: Alvaro Herrera (#5)
Re: strange CREATE INDEX tab completion cases

Alvaro Herrera <alvherre@2ndquadrant.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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers