Re: [HACKERS] strange CREATE INDEX tab completion cases

[Michael Paquier]

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

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

Re: [HACKERS] strange CREATE INDEX tab completion cases

[Tom Lane]

Alvaro Herrera  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

Re: [HACKERS] strange CREATE INDEX tab completion cases

[Alvaro Herrera]

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 Herrerahttp://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

Re: [HACKERS] strange CREATE INDEX tab completion cases

[Peter Eisentraut]

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.
From bce311ab08053aa6d21b31625a1be85a33138300 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Sun, 10 Jan 2016 11:24:51 -0500
Subject: [PATCH 1/3] psql: Update tab completion comment

This just updates a comment to match the code.

from Michael Paquier
---
 src/bin/psql/tab-complete.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 4d2bee1..dcbe515 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1999,7 +1999,8 @@ psql_completion(const char *text, int start, int end)
 	/* First off we complete CREATE UNIQUE with "INDEX" */
 	else if (TailMatches2("CREATE", "UNIQUE"))
 		COMPLETE_WITH_CONST("INDEX");
-	/* If we have CREATE|UNIQUE INDEX, then add "ON" and existing indexes */
+	/* If we have CREATE|UNIQUE INDEX, then add "ON", "CONCURRENTLY",
+	   and existing indexes */
 	else if (TailMatches2("CREATE|UNIQUE", "INDEX"))
 		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_indexes,
    " UNION SELECT 'ON'"
-- 
2.7.0

From d61d232b429e911d73598d9615fc73397fb44bda Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Sun, 10 Jan 2016 11:43:27 -0500
Subject: [PATCH 2/3] psql: Fix CREATE INDEX tab completion

The previous code supported a syntax like CREATE INDEX name
CONCURRENTLY, which never existed.  Mistake introduced in commit
37ec19a15ce452ee94f32ebc3d6a9a45868e82fd.  Remove the addition of
CONCURRENTLY at that point.
---
 src/bin/psql/tab-complete.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index dcbe515..52336aa 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -2009,13 +2009,12 @@ psql_completion(const char *text, int start, int end)
 	else if (TailMatches3("INDEX", MatchAny, "ON") ||
 			 TailMatches2("INDEX|CONCURRENTLY", "ON"))
 		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tm, NULL);
-	/* If we have CREATE|UNIQUE INDEX  CONCURRENTLY, then add "ON" */
-	else if (TailMatches3("INDEX", MatchAny, "CONCURRENTLY") ||
-			 TailMatches2("INDEX", "CONCURRENTLY"))
+	/* If we have CREATE|UNIQUE INDEX CONCURRENTLY, then add "ON" */
+	else if (TailMatches2("INDEX", "CONCURRENTLY"))
 		COMPLETE_WITH_CONST("ON");
-	/* If we have CREATE|UNIQUE INDEX , then add "ON" or "CONCURRENTLY" */
+	/* If we have CREATE|UNIQUE INDEX , then add "ON" */
 	else if (TailMatches3("CREATE|UNIQUE", "INDEX", MatchAny))
-		COMPLETE_WITH_LIST2("CONCURRENTLY", "ON");
+		COMPLETE_WITH_CONST("ON");
 
 	/*
 	 * Complete INDEX  ON  with a list of table columns (which
-- 
2.7.0

From 84725481c29ae2d3e7af0ca7b4e3b58de48c56e4 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Sun, 10 Jan 2016 12:04:28 -0500
Subject: [PATCH 3/3] psql: Improve CREATE INDEX CONCURRENTLY tab completion

The completion of CREATE INDEX CONCURRENTLY was lacking in several ways
compared to a plain CREATE INDEX command:

- CREATE INDEX  ON completes table names, but didn't with
  CONCURRENTLY.

- CREATE INDEX completes ON and existing index names, but with
  CONCURRENTLY it only completed ON.

- CREATE INDEX  completes ON, but didn't with CONCURRENTLY.

These are now all fixed.
---
 src/bin/psql/tab-complete.c | 15 +--
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 52336aa..97580b9 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -2005,15 +2005,18 @@ psql_completion(const char *text, int start, int end)
 		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_indexes,
    " UNION SELECT 'ON'"
    " UNION SELECT 'CONCURRENTLY'");
-	/* Complete ... INDEX [] ON with a list of tables  */
-	else if (TailMatches3("INDEX", MatchAny, "ON") ||
+	/* Complete ... INDEX|CONCURRENTLY [] ON with a list of tables  */
+	else if (TailMatches3("INDEX|CONCURRENTLY", MatchAny, "ON") ||
 			 TailMatches2("INDEX|CONCURRENTLY", "ON"))
 		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tm, NULL);
-	/* If we have CREATE|UNIQUE INDEX CONCURRENTLY, then add "ON" */
+	/* If we have CREATE|UNIQUE INDEX CONCURRENTLY, then add "ON" and existing
+	   indexes */
 	else if (TailMatches2("INDEX", "CONCURRENTLY"))
-		COMPLETE_WITH_CONST("ON");
-	/* If we have CREATE|UNIQUE INDEX , then add "ON" */
-	else if (TailMatches3("CREATE|UNIQUE", "INDEX", MatchAny))
+		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_indexes,
+   " UNION SELECT 'ON'");
+	/* If we have CREATE|UNIQUE INDEX [CONCURRENTLY] , then add "ON" */
+	else if 

Re: [HACKERS] strange CREATE INDEX tab completion cases

[Michael Paquier]

On Sat, Dec 12, 2015 at 11:17 AM, Peter Eisentraut  wrote:
> These two tab completion pieces look strange to me:
>
> /* If we have CREATE|UNIQUE INDEX  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 , 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 
date: Wed, 17 Feb 2010 04:09:40 +
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


20151213_psql_completion_index.patch
Description: binary/octet-stream

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

[HACKERS] strange CREATE INDEX tab completion cases

[Peter Eisentraut]

These two tab completion pieces look strange to me:

/* If we have CREATE|UNIQUE INDEX  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 , 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