Improved tab completion for FDW DDL
Hi,
Here is a patch which adds the below missing tab completions for the FDW
DDL commands. I noticed these were missing while playing around with
FDWs a while ago.
"ALTER SERVER <name>" with "RENAME TO"
"CREATE SERVER <name> FOREIGN DATA WRAPPER" with fdw name
"CREATE|ALTER USER MAPPING FOR <name> SERVER <name>" with "OPTIONS ("
Another completion which is currently missing but I am not sure if we
should add or not is completing "ALTER|CREATE|DROP USER" with "MAPPING
FOR", but since it might interfere with completing to username for
"ALTER|DROP USER" I am not sure we want it. What do you think?
Andreas
Attachments:
fdw-ddl-tab-completion.patchtext/x-diff; name=fdw-ddl-tab-completion.patchDownload
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 4c93ae9..46b555c 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1444,7 +1444,7 @@ psql_completion(const char *text, int start, int end)
COMPLETE_WITH_LIST3("MINVALUE", "MAXVALUE", "CYCLE");
/* ALTER SERVER <name> */
else if (TailMatches3("ALTER", "SERVER", MatchAny))
- COMPLETE_WITH_LIST3("VERSION", "OPTIONS", "OWNER TO");
+ COMPLETE_WITH_LIST4("VERSION", "OPTIONS", "OWNER TO", "RENAME TO");
/* ALTER SYSTEM SET, RESET, RESET ALL */
else if (TailMatches2("ALTER", "SYSTEM"))
COMPLETE_WITH_LIST2("SET", "RESET");
@@ -2015,8 +2015,12 @@ psql_completion(const char *text, int start, int end)
COMPLETE_WITH_LIST3("MINVALUE", "MAXVALUE", "CYCLE");
/* CREATE SERVER <name> */
+ /* Complete "CREATE SERVER <name>" with "FOREIGN DATA WRAPPER" */
else if (TailMatches3("CREATE", "SERVER", MatchAny))
COMPLETE_WITH_LIST3("TYPE", "VERSION", "FOREIGN DATA WRAPPER");
+ /* Complete "CREATE SERVER <name> FOREIGN DATA WRAPPER" with fdw name */
+ else if (TailMatches6("CREATE", "SERVER", MatchAny, "FOREIGN", "DATA", "WRAPPER"))
+ COMPLETE_WITH_QUERY(Query_for_list_of_fdws);
/* CREATE TABLE */
/* Complete "CREATE TEMP/TEMPORARY" with the possible temp objects */
@@ -2740,6 +2744,8 @@ psql_completion(const char *text, int start, int end)
COMPLETE_WITH_QUERY(Query_for_list_of_user_mappings);
else if (TailMatches5("CREATE|ALTER|DROP", "USER", "MAPPING", "FOR", MatchAny))
COMPLETE_WITH_CONST("SERVER");
+ else if (TailMatches7("CREATE|ALTER", "USER", "MAPPING", "FOR", MatchAny, "SERVER", MatchAny))
+ COMPLETE_WITH_CONST("OPTIONS (");
/*
* VACUUM [ FULL | FREEZE ] [ VERBOSE ] [ table ]
On Wed, Dec 30, 2015 at 01:21:06PM +0100, Andreas Karlsson wrote:
Hi,
Here is a patch which adds the below missing tab completions for the FDW DDL
commands. I noticed these were missing while playing around with FDWs a
while ago."ALTER SERVER <name>" with "RENAME TO"
"CREATE SERVER <name> FOREIGN DATA WRAPPER" with fdw name
"CREATE|ALTER USER MAPPING FOR <name> SERVER <name>" with "OPTIONS ("Another completion which is currently missing but I am not sure if we should
add or not is completing "ALTER|CREATE|DROP USER" with "MAPPING FOR", but
since it might interfere with completing to username for "ALTER|DROP USER" I
am not sure we want it. What do you think?
Is there a way to require some reasonable chunk--say, one that's
disambiguated from the name any known ROLE with LOGIN--of MAPPING
before completing with MAPPING FOR? I confess to not knowing how the
new system works in enough detail to know that off the top of my head.
Cheers,
David.
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Dec 30, 2015 at 9:21 PM, Andreas Karlsson <andreas@proxel.se> wrote:
Hi,
Here is a patch which adds the below missing tab completions for the FDW DDL
commands. I noticed these were missing while playing around with FDWs a
while ago."ALTER SERVER <name>" with "RENAME TO"
"CREATE SERVER <name> FOREIGN DATA WRAPPER" with fdw name
"CREATE|ALTER USER MAPPING FOR <name> SERVER <name>" with "OPTIONS ("Another completion which is currently missing but I am not sure if we should
add or not is completing "ALTER|CREATE|DROP USER" with "MAPPING FOR", but
since it might interfere with completing to username for "ALTER|DROP USER" I
am not sure we want it. What do you think?
You may want to use Matches() instead of TailMatches() for performance reasons.
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 01/04/2016 07:26 AM, Michael Paquier wrote:
You may want to use Matches() instead of TailMatches() for performance reasons.
Probably, but if so should we not also change the surrounding rules to
also use Matches()? I imitated the surrounding code to keep code
consistency and avoid errors.
I thought I saw some patch from you changing many of the rules from
TailMatches() to Matches(). Perhaps I should just update my patch after
your patch has been applied.
Andreas
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Andreas Karlsson <andreas@proxel.se> writes:
I thought I saw some patch from you changing many of the rules from
TailMatches() to Matches(). Perhaps I should just update my patch after
your patch has been applied.
Yes, I think it's important to get that set of patches from Michael
into the tree ASAP so that they can be precedent for the other tab
completion patches that are floating around. I am under the gun for
9.5.0 release today but will review and hopefully push Michael's
changes soon.
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
On 01/04/2016 07:26 AM, Michael Paquier wrote:
You may want to use Matches() instead of TailMatches() for performance reasons.
Here is an updated version which uses Matches().
Andreas
Attachments:
fdw-ddl-tab-completion-v2.patchtext/x-diff; name=fdw-ddl-tab-completion-v2.patchDownload
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 4d2bee1..34dabc5 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1544,7 +1544,7 @@ psql_completion(const char *text, int start, int end)
COMPLETE_WITH_LIST3("MINVALUE", "MAXVALUE", "CYCLE");
/* ALTER SERVER <name> */
else if (Matches3("ALTER", "SERVER", MatchAny))
- COMPLETE_WITH_LIST3("VERSION", "OPTIONS", "OWNER TO");
+ COMPLETE_WITH_LIST4("VERSION", "OPTIONS", "OWNER TO", "RENAME TO");
/* ALTER SYSTEM SET, RESET, RESET ALL */
else if (Matches2("ALTER", "SYSTEM"))
COMPLETE_WITH_LIST2("SET", "RESET");
@@ -2095,8 +2095,12 @@ psql_completion(const char *text, int start, int end)
COMPLETE_WITH_LIST3("MINVALUE", "MAXVALUE", "CYCLE");
/* CREATE SERVER <name> */
+ /* Complete "CREATE SERVER <name>" with "FOREIGN DATA WRAPPER" */
else if (Matches3("CREATE", "SERVER", MatchAny))
COMPLETE_WITH_LIST3("TYPE", "VERSION", "FOREIGN DATA WRAPPER");
+ /* Complete "CREATE SERVER <name> FOREIGN DATA WRAPPER" with fdw name */
+ else if (Matches6("CREATE", "SERVER", MatchAny, "FOREIGN", "DATA", "WRAPPER"))
+ COMPLETE_WITH_QUERY(Query_for_list_of_fdws);
/* CREATE TABLE --- is allowed inside CREATE SCHEMA, so use TailMatches */
/* Complete "CREATE TEMP/TEMPORARY" with the possible temp objects */
@@ -2803,6 +2807,8 @@ psql_completion(const char *text, int start, int end)
COMPLETE_WITH_QUERY(Query_for_list_of_user_mappings);
else if (Matches5("CREATE|ALTER|DROP", "USER", "MAPPING", "FOR", MatchAny))
COMPLETE_WITH_CONST("SERVER");
+ else if (Matches7("CREATE|ALTER", "USER", "MAPPING", "FOR", MatchAny, "SERVER", MatchAny))
+ COMPLETE_WITH_CONST("OPTIONS (");
/*
* VACUUM [ FULL | FREEZE ] [ VERBOSE ] [ table ]
On 01/04/2016 01:09 AM, David Fetter wrote:
On Wed, Dec 30, 2015 at 01:21:06PM +0100, Andreas Karlsson wrote:
Another completion which is currently missing but I am not sure if we should
add or not is completing "ALTER|CREATE|DROP USER" with "MAPPING FOR", but
since it might interfere with completing to username for "ALTER|DROP USER" I
am not sure we want it. What do you think?Is there a way to require some reasonable chunk--say, one that's
disambiguated from the name any known ROLE with LOGIN--of MAPPING
before completing with MAPPING FOR? I confess to not knowing how the
new system works in enough detail to know that off the top of my head.
No, and while it would not be too hard to build it would not be worth
doing just for this use case.
Andreas
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Andreas Karlsson <andreas@proxel.se> writes:
On 01/04/2016 01:09 AM, David Fetter wrote:
On Wed, Dec 30, 2015 at 01:21:06PM +0100, Andreas Karlsson wrote:
Another completion which is currently missing but I am not sure if we should
add or not is completing "ALTER|CREATE|DROP USER" with "MAPPING FOR", but
since it might interfere with completing to username for "ALTER|DROP USER" I
am not sure we want it. What do you think?
Is there a way to require some reasonable chunk--say, one that's
disambiguated from the name any known ROLE with LOGIN--of MAPPING
before completing with MAPPING FOR? I confess to not knowing how the
new system works in enough detail to know that off the top of my head.
No, and while it would not be too hard to build it would not be worth
doing just for this use case.
The way we've solved other similar cases would translate like this:
instead of the "query for user names" just returning user names, add
on "UNION 'MAPPING FOR'". So if you do
# alter user <TAB>
where you're now offered
alice joe postgres
you'd instead get
alice joe postgres MAPPING FOR
Dunno if it's worth the trouble though.
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
The second part is not necessary, because there is already code that
completes FDW names after "FOREIGN DATA WRAPPER". So this already works.
The other two parts are valid improvements, but they should be done
consistently across commands. So please
- Also complete RENAME TO in ALTER FOREIGN DATA WRAPPER.
- Also complete OPTIONS in FOREIGN DATA WRAPPER and SERVER commands.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 01/11/2016 02:39 AM, Peter Eisentraut wrote:
The second part is not necessary, because there is already code that
completes FDW names after "FOREIGN DATA WRAPPER". So this already works.
Good spot, thanks. I have no idea why I did not think it worked. Maybe
it just did not work in 9.2 and I failed when trying to reproduce it on
master.
- Also complete RENAME TO in ALTER FOREIGN DATA WRAPPER.
Done.
- Also complete OPTIONS in FOREIGN DATA WRAPPER and SERVER commands.
Done.
Andreas
Attachments:
fdw-ddl-tab-completion-v3.patchtext/x-diff; name=fdw-ddl-tab-completion-v3.patchDownload
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index c3c77bd..3d8cdf4 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1417,7 +1417,7 @@ psql_completion(const char *text, int start, int end)
/* ALTER FOREIGN DATA WRAPPER <name> */
else if (Matches5("ALTER", "FOREIGN", "DATA", "WRAPPER", MatchAny))
- COMPLETE_WITH_LIST4("HANDLER", "VALIDATOR", "OPTIONS", "OWNER TO");
+ COMPLETE_WITH_LIST5("HANDLER", "VALIDATOR", "OPTIONS", "OWNER TO", "RENAME TO");
/* ALTER FOREIGN TABLE <name> */
else if (Matches4("ALTER", "FOREIGN", "TABLE", MatchAny))
@@ -1544,7 +1544,10 @@ psql_completion(const char *text, int start, int end)
COMPLETE_WITH_LIST3("MINVALUE", "MAXVALUE", "CYCLE");
/* ALTER SERVER <name> */
else if (Matches3("ALTER", "SERVER", MatchAny))
- COMPLETE_WITH_LIST3("VERSION", "OPTIONS", "OWNER TO");
+ COMPLETE_WITH_LIST4("VERSION", "OPTIONS", "OWNER TO", "RENAME TO");
+ /* ALTER SERVER <name> VERSION <version>*/
+ else if (Matches5("ALTER", "SERVER", MatchAny, "VERSION", MatchAny))
+ COMPLETE_WITH_CONST("OPTIONS");
/* ALTER SYSTEM SET, RESET, RESET ALL */
else if (Matches2("ALTER", "SYSTEM"))
COMPLETE_WITH_LIST2("SET", "RESET");
@@ -1993,7 +1996,7 @@ psql_completion(const char *text, int start, int end)
/* CREATE FOREIGN DATA WRAPPER */
else if (Matches5("CREATE", "FOREIGN", "DATA", "WRAPPER", MatchAny))
- COMPLETE_WITH_LIST2("HANDLER", "VALIDATOR");
+ COMPLETE_WITH_LIST3("HANDLER", "VALIDATOR", "OPTIONS");
/* CREATE INDEX --- is allowed inside CREATE SCHEMA, so use TailMatches */
/* First off we complete CREATE UNIQUE with "INDEX" */
@@ -2372,6 +2375,10 @@ psql_completion(const char *text, int start, int end)
else if (TailMatches3("FOREIGN", "DATA", "WRAPPER") &&
!TailMatches4("CREATE", MatchAny, MatchAny, MatchAny))
COMPLETE_WITH_QUERY(Query_for_list_of_fdws);
+ /* applies in CREATE SERVER */
+ else if (TailMatches4("FOREIGN", "DATA", "WRAPPER", MatchAny) &&
+ HeadMatches2("CREATE", "SERVER"))
+ COMPLETE_WITH_CONST("OPTIONS");
/* FOREIGN TABLE */
else if (TailMatches2("FOREIGN", "TABLE") &&
@@ -2816,6 +2823,8 @@ psql_completion(const char *text, int start, int end)
COMPLETE_WITH_QUERY(Query_for_list_of_user_mappings);
else if (Matches5("CREATE|ALTER|DROP", "USER", "MAPPING", "FOR", MatchAny))
COMPLETE_WITH_CONST("SERVER");
+ else if (Matches7("CREATE|ALTER", "USER", "MAPPING", "FOR", MatchAny, "SERVER", MatchAny))
+ COMPLETE_WITH_CONST("OPTIONS");
/*
* VACUUM [ FULL | FREEZE ] [ VERBOSE ] [ table ]
On 1/18/16 8:36 PM, Andreas Karlsson wrote:
On 01/11/2016 02:39 AM, Peter Eisentraut wrote:
The second part is not necessary, because there is already code that
completes FDW names after "FOREIGN DATA WRAPPER". So this already works.Good spot, thanks. I have no idea why I did not think it worked. Maybe
it just did not work in 9.2 and I failed when trying to reproduce it on
master.- Also complete RENAME TO in ALTER FOREIGN DATA WRAPPER.
Done.
- Also complete OPTIONS in FOREIGN DATA WRAPPER and SERVER commands.
Done.
committed
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 01/23/2016 01:03 PM, Peter Eisentraut wrote:
committed
Thanks!
Andreas
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers