Обсуждение: [PATCH] Tab completion for ALTER TABLE … ADD …
[PATCH] Tab completion for ALTER TABLE … ADD …
От
ilmari@ilmari.org (Dagfinn Ilmari Mannsåker)
Дата:
Hi Hackers, The other day I noticed that there's no tab completion after ALTER TABLE … ADD, so here's a patch. In addition to COLUMN and all the table constraint types, it also completes with the list of unique indexes on the table after ALTER TABLE … ADD … USING INDEX. - ilmari From 231cccd2e84ef6dc2bf41423979efc4e760c013e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@ilmari.org> Date: Tue, 3 Aug 2021 12:23:07 +0100 Subject: [PATCH] =?UTF-8?q?Add=20tab=20completion=20for=20ALTER=20TABLE=20?= =?UTF-8?q?=E2=80=A6=20ADD=20=E2=80=A6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Complte COLUMN and table constraint types, and list of indexes on the table for ADD (UNQIUE|PRIMARY KEY) USING INDEX. --- src/bin/psql/tab-complete.c | 44 +++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c index 064892bade..476a72908f 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -776,6 +776,10 @@ static const SchemaQuery Query_for_list_of_collations = { " and pg_catalog.quote_ident(c1.relname)='%s'"\ " and pg_catalog.pg_table_is_visible(c2.oid)" +#define Query_for_unique_index_of_table \ +Query_for_index_of_table \ +" and i.indisunique" + /* the silly-looking length condition is just to eat up the current word */ #define Query_for_constraint_of_table \ "SELECT pg_catalog.quote_ident(conname) "\ @@ -2023,6 +2027,46 @@ psql_completion(const char *text, int start, int end) "OWNER TO", "SET", "VALIDATE CONSTRAINT", "REPLICA IDENTITY", "ATTACH PARTITION", "DETACH PARTITION", "FORCE ROW LEVEL SECURITY"); + /* ALTER TABLE xxx ADD */ + else if (Matches("ALTER", "TABLE", MatchAny, "ADD")) + COMPLETE_WITH("COLUMN", "CONSTRAINT", "CHECK", "UNIQUE", "PRIMARY KEY", + "EXCLUDE", "FOREIGN KEY"); + /* ALTER TABLE xxx ADD CONSTRAINT yyy */ + else if (Matches("ALTER", "TABLE", MatchAny, "ADD", "CONSTRAINT", MatchAny)) + COMPLETE_WITH("CHECK", "UNIQUE", "PRIMARY KEY", "EXCLUDE", "FOREIGN KEY"); + /* ALTER TABLE xxx ADD (CONSTRAINT yyy)? FOREIGN|PRIMARY */ + else if (Matches("ALTER", "TABLE", MatchAny, "ADD", "FOREIGN|PRIMARY") || + Matches("ALTER", "TABLE", MatchAny, "ADD", "CONSTRAINT", MatchAny, "FOREIGN|PRIMARY")) + COMPLETE_WITH("KEY"); + /* ALTER TABLE xxx ADD (CONSTRAINT yyy)? (PRIMARY KEY|UNIQUE) */ + else if (Matches("ALTER", "TABLE", MatchAny, "ADD", "PRIMARY", "KEY") || + Matches("ALTER", "TABLE", MatchAny, "ADD", "UNIQUE") || + Matches("ALTER", "TABLE", MatchAny, "ADD", "CONSTRAINT", MatchAny, "PRIMARY", "KEY") || + Matches("ALTER", "TABLE", MatchAny, "ADD", "CONSTRAINT", MatchAny, "UNIQUE")) + COMPLETE_WITH("(", "USING INDEX"); + /* ALTER TABLE xxx ADD (CONSTRAINT yyy)? ... USING INDEX */ + else if (Matches("ALTER", "TABLE", MatchAny, "ADD", "PRIMARY", "KEY", "USING", "INDEX")) + { + completion_info_charp = prev6_wd; + COMPLETE_WITH_QUERY(Query_for_unique_index_of_table); + } + else if (Matches("ALTER", "TABLE", MatchAny, "ADD", "UNIQUE", "USING", "INDEX")) + { + completion_info_charp = prev5_wd; + COMPLETE_WITH_QUERY(Query_for_unique_index_of_table); + } + else if (Matches("ALTER", "TABLE", MatchAny, "ADD", "CONSTRAINT", MatchAny, + "PRIMARY", "KEY", "USING", "INDEX")) + { + completion_info_charp = prev8_wd; + COMPLETE_WITH_QUERY(Query_for_unique_index_of_table); + } + else if (Matches("ALTER", "TABLE", MatchAny, "ADD", "CONSTRAINT", MatchAny, + "UNIQUE", "USING", "INDEX")) + { + completion_info_charp = prev7_wd; + COMPLETE_WITH_QUERY(Query_for_unique_index_of_table); + } /* ALTER TABLE xxx ENABLE */ else if (Matches("ALTER", "TABLE", MatchAny, "ENABLE")) COMPLETE_WITH("ALWAYS", "REPLICA", "ROW LEVEL SECURITY", "RULE", -- 2.30.2
Re: [PATCH] Tab completion for ALTER TABLE … ADD …
От
ilmari@ilmari.org (Dagfinn Ilmari Mannsåker)
Дата:
ilmari@ilmari.org (Dagfinn Ilmari Mannsåker) writes: > Hi Hackers, > > The other day I noticed that there's no tab completion after ALTER TABLE > … ADD, so here's a patch. In addition to COLUMN and all the table > constraint types, it also completes with the list of unique indexes on > the table after ALTER TABLE … ADD … USING INDEX. Added to the 2021-09 commitfest: https://commitfest.postgresql.org/34/3280/ - ilmari
On Tue, Aug 03, 2021 at 12:48:38PM +0100, Dagfinn Ilmari Mannsåker wrote: > The other day I noticed that there's no tab completion after ALTER TABLE > … ADD, so here's a patch. In addition to COLUMN and all the table > constraint types, it also completes with the list of unique indexes on > the table after ALTER TABLE … ADD … USING INDEX. I was reading this patch (not actually tested), and that's a clear improvement. One extra thing that could be done here is to complete with types for a ALTER TABLE ADD COLUMN foo. We could as well have a list of columns after UNIQUE or PRIMARY KEY, but that feels like extra cream on top of the cake. In short I am fine with what you have here. -- Michael
Вложения
Michael Paquier <michael@paquier.xyz> writes: > On Tue, Aug 03, 2021 at 12:48:38PM +0100, Dagfinn Ilmari Mannsåker wrote: >> The other day I noticed that there's no tab completion after ALTER TABLE >> … ADD, so here's a patch. In addition to COLUMN and all the table >> constraint types, it also completes with the list of unique indexes on >> the table after ALTER TABLE … ADD … USING INDEX. > > I was reading this patch (not actually tested), and that's a clear > improvement. One extra thing that could be done here is to complete > with types for a ALTER TABLE ADD COLUMN foo. That was easy enough to add (just a bit of extra fiddling to handle COLUMN being optional), done in the attached v2 patch. > We could as well have a list of columns after UNIQUE or PRIMARY KEY, > but that feels like extra cream on top of the cake. Doing a list of arbitrarily many comma-separated names is more complicated, so that can be the subject for another patch. > In short I am fine with what you have here. Thanks for the review. - ilmari From 0bdf91f2a66bf9393e6900db904bda1961c03350 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@ilmari.org> Date: Tue, 3 Aug 2021 12:23:07 +0100 Subject: [PATCH v2] =?UTF-8?q?Add=20tab=20completion=20for=20ALTER=20TABLE?= =?UTF-8?q?=20=E2=80=A6=20ADD=20=E2=80=A6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Complete with COLUMN plus the various table constraint types, list unique indexes on the table after ADD (UNQIUE|PRIMARY KEY) USING INDEX, and data types after ADD [COLUMN] <column_name>. --- src/bin/psql/tab-complete.c | 49 +++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c index 7cdfc7c637..bb7c3fc5cf 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -776,6 +776,10 @@ static const SchemaQuery Query_for_list_of_collations = { " and pg_catalog.quote_ident(c1.relname)='%s'"\ " and pg_catalog.pg_table_is_visible(c2.oid)" +#define Query_for_unique_index_of_table \ +Query_for_index_of_table \ +" and i.indisunique" + /* the silly-looking length condition is just to eat up the current word */ #define Query_for_constraint_of_table \ "SELECT pg_catalog.quote_ident(conname) "\ @@ -2019,6 +2023,51 @@ psql_completion(const char *text, int start, int end) "OWNER TO", "SET", "VALIDATE CONSTRAINT", "REPLICA IDENTITY", "ATTACH PARTITION", "DETACH PARTITION", "FORCE ROW LEVEL SECURITY"); + /* ALTER TABLE xxx ADD */ + else if (Matches("ALTER", "TABLE", MatchAny, "ADD")) + COMPLETE_WITH("COLUMN", "CONSTRAINT", "CHECK", "UNIQUE", "PRIMARY KEY", + "EXCLUDE", "FOREIGN KEY"); + /* ALTER TABLE xxx ADD CONSTRAINT yyy */ + else if (Matches("ALTER", "TABLE", MatchAny, "ADD", "CONSTRAINT", MatchAny)) + COMPLETE_WITH("CHECK", "UNIQUE", "PRIMARY KEY", "EXCLUDE", "FOREIGN KEY"); + /* ALTER TABLE xxx ADD (CONSTRAINT yyy)? FOREIGN|PRIMARY */ + else if (Matches("ALTER", "TABLE", MatchAny, "ADD", "FOREIGN|PRIMARY") || + Matches("ALTER", "TABLE", MatchAny, "ADD", "CONSTRAINT", MatchAny, "FOREIGN|PRIMARY")) + COMPLETE_WITH("KEY"); + /* ALTER TABLE xxx ADD (CONSTRAINT yyy)? (PRIMARY KEY|UNIQUE) */ + else if (Matches("ALTER", "TABLE", MatchAny, "ADD", "PRIMARY", "KEY") || + Matches("ALTER", "TABLE", MatchAny, "ADD", "UNIQUE") || + Matches("ALTER", "TABLE", MatchAny, "ADD", "CONSTRAINT", MatchAny, "PRIMARY", "KEY") || + Matches("ALTER", "TABLE", MatchAny, "ADD", "CONSTRAINT", MatchAny, "UNIQUE")) + COMPLETE_WITH("(", "USING INDEX"); + /* ALTER TABLE xxx ADD (CONSTRAINT yyy)? ... USING INDEX */ + else if (Matches("ALTER", "TABLE", MatchAny, "ADD", "PRIMARY", "KEY", "USING", "INDEX")) + { + completion_info_charp = prev6_wd; + COMPLETE_WITH_QUERY(Query_for_unique_index_of_table); + } + else if (Matches("ALTER", "TABLE", MatchAny, "ADD", "UNIQUE", "USING", "INDEX")) + { + completion_info_charp = prev5_wd; + COMPLETE_WITH_QUERY(Query_for_unique_index_of_table); + } + else if (Matches("ALTER", "TABLE", MatchAny, "ADD", "CONSTRAINT", MatchAny, + "PRIMARY", "KEY", "USING", "INDEX")) + { + completion_info_charp = prev8_wd; + COMPLETE_WITH_QUERY(Query_for_unique_index_of_table); + } + else if (Matches("ALTER", "TABLE", MatchAny, "ADD", "CONSTRAINT", MatchAny, + "UNIQUE", "USING", "INDEX")) + { + completion_info_charp = prev7_wd; + COMPLETE_WITH_QUERY(Query_for_unique_index_of_table); + } + /* ADD column_name must be last of the ALTER TABLE xxx ADD matches */ + else if (Matches("ALTER", "TABLE", MatchAny, "ADD", "COLUMN", MatchAny) || + (Matches("ALTER", "TABLE", MatchAny, "ADD", MatchAny) && + !Matches("ALTER", "TABLE", MatchAny, "ADD", "COLUMN"))) + COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_datatypes, NULL); /* ALTER TABLE xxx ENABLE */ else if (Matches("ALTER", "TABLE", MatchAny, "ENABLE")) COMPLETE_WITH("ALWAYS", "REPLICA", "ROW LEVEL SECURITY", "RULE", -- 2.30.2
On Fri, Aug 27, 2021 at 11:52:33AM +0100, Dagfinn Ilmari Mannsåker wrote: > That was easy enough to add (just a bit of extra fiddling to handle > COLUMN being optional), done in the attached v2 patch. This part was a bit misleading, as it would recommend a list of types when specifying just ADD CONSTRAINT for example, so I have removed it. An extra thing that felt a bit overdoing is the addition of KEY after PRIMARY/FOREIGN. > Doing a list of arbitrarily many comma-separated names is more > complicated, so that can be the subject for another patch. No objections to that. I have applied what we have now, as that's already an improvement. -- Michael
Вложения
Michael Paquier <michael@paquier.xyz> writes: > On Fri, Aug 27, 2021 at 11:52:33AM +0100, Dagfinn Ilmari Mannsåker wrote: >> That was easy enough to add (just a bit of extra fiddling to handle >> COLUMN being optional), done in the attached v2 patch. > > This part was a bit misleading, as it would recommend a list of types > when specifying just ADD CONSTRAINT for example, so I have removed > it. That was because I forgot to exclude all the other object types that can come after ADD. Attached is a patch that does that. I also moved it right next to the ALTER TABLE … ADD completion, and added a comment to keep the two lists in sync. > An extra thing that felt a bit overdoing is the addition of KEY after > PRIMARY/FOREIGN. Yeah, I guess people are unlikely to write out the whole PRIMARY or FOREIGN and only then hit tab to complete the rest. >> Doing a list of arbitrarily many comma-separated names is more >> complicated, so that can be the subject for another patch. > > No objections to that. I have applied what we have now, as that's > already an improvement. Thanks! - ilmari From a5fac64310b01ed32e281031a2c274e835463853 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@ilmari.org> Date: Mon, 30 Aug 2021 14:25:55 +0100 Subject: [PATCH 1/2] =?UTF-8?q?Complete=20type=20names=20after=20ALTER=20T?= =?UTF-8?q?ABLE=20=E2=80=A6=20ADD=20[COLUMN]=E2=80=88=E2=80=A6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since COLUMN is optional, we need to keep the list of object types to complete after ADD in sync with the list of words not to complete type names after. Add a comment to this effect. --- src/bin/psql/tab-complete.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c index 7c6af435a9..197f4c736c 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -2025,8 +2025,14 @@ psql_completion(const char *text, int start, int end) "DETACH PARTITION", "FORCE ROW LEVEL SECURITY"); /* ALTER TABLE xxx ADD */ else if (Matches("ALTER", "TABLE", MatchAny, "ADD")) + /* make sure to keep this list and the !Matches() below in sync */ COMPLETE_WITH("COLUMN", "CONSTRAINT", "CHECK", "UNIQUE", "PRIMARY KEY", "EXCLUDE", "FOREIGN KEY"); + /* ATER TABLE xxx ADD [COLUMN] yyy */ + else if (Matches("ALTER", "TABLE", MatchAny, "ADD", "COLUMN", MatchAny) || + (Matches("ALTER", "TABLE", MatchAny, "ADD", MatchAny) && + !Matches("ALTER", "TABLE", MatchAny, "ADD", "COLUMN|CONSTRAINT|CHECK|UNIQUE|PRIMARY|EXCLUDE|FOREIGN"))) + COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_datatypes, NULL); /* ALTER TABLE xxx ADD CONSTRAINT yyy */ else if (Matches("ALTER", "TABLE", MatchAny, "ADD", "CONSTRAINT", MatchAny)) COMPLETE_WITH("CHECK", "UNIQUE", "PRIMARY KEY", "EXCLUDE", "FOREIGN KEY"); -- 2.30.2
On Mon, Aug 30, 2021 at 02:38:19PM +0100, Dagfinn Ilmari Mannsåker wrote: > That was because I forgot to exclude all the other object types that can > come after ADD. Attached is a patch that does that. I also moved it > right next to the ALTER TABLE … ADD completion, and added a comment to > keep the two lists in sync. Looks fine to me, so applied while we are on it. -- Michael
Вложения
On Tue, 31 Aug 2021, at 04:20, Michael Paquier wrote: > On Mon, Aug 30, 2021 at 02:38:19PM +0100, Dagfinn Ilmari Mannsåker wrote: > > That was because I forgot to exclude all the other object types that can > > come after ADD. Attached is a patch that does that. I also moved it > > right next to the ALTER TABLE … ADD completion, and added a comment to > > keep the two lists in sync. > > Looks fine to me, so applied while we are on it. Thanks! - ilmari