Thread: [PATCH] Tab completion for ALTER TABLE … ADD …
[PATCH] Tab completion for ALTER TABLE … ADD …
From
ilmari@ilmari.org (Dagfinn Ilmari Mannsåker)
Date:
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 …
From
ilmari@ilmari.org (Dagfinn Ilmari Mannsåker)
Date:
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
Attachment
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
Attachment
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
Attachment
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