On Wed, May 19, 2021 at 2:03 PM Bharath Rupireddy <
bharath.rupireddyforpostg...@gmail.com> wrote:
>
> On Tue, May 18, 2021 at 9:21 PM Alvaro Herrera <alvhe...@alvh.no-ip.org>
wrote:
> >
> > On 2021-May-14, vignesh C wrote:
> >
> > > While I was reviewing one of the logical decoding features, I found
> > > Streaming and binary options were missing in tab completion for the
> > > alter subscription set option, the attached patch has the changes for
> > > the same.
> > > Thoughts?
> >
> > I wish we didn't have to keep knowledge in the psql source on which
> > option names are to be used for each command. If we had some function
> > SELECT pg_completion_options('alter subscription set');
> > that returned the list of options usable for each command, we wouldn't
> > have to ... psql would just retrieve the list of options for the current
> > command.
> >
> > Maintaining such a list does not seem hard -- for example we could just
> > have a function alongside parse_subscription_option() that returns the
> > names that are recognized by that one. If we drive the implementation
> > of both off a single struct, it would never be outdated.
>
> Yeah, having something similar to table_storage_parameters works better.
>
> While on this, I found that all the options are not listed for CREATE
> SUBSCRIPTION command in tab-complete.c, missing ones are binary and
> streaming:
> else if (HeadMatches("CREATE", "SUBSCRIPTION") && TailMatches("WITH",
"("))
> COMPLETE_WITH("copy_data", "connect", "create_slot", "enabled",
> "slot_name", "synchronous_commit");
>
Modified.
> Similarly, CREATE and ALTER PUBLICATION don't have
> publish_via_partition_root option:
> else if (HeadMatches("CREATE", "PUBLICATION") && TailMatches("WITH",
"("))
> COMPLETE_WITH("publish");
>
Modified.
> I think having some structures like below in subscriptioncmds.h,
> publicationcmds.h and using them in tab-complete.c would make more
> sense.
This approach has few disadvantages that Tom Lane has pointed out in [1],
Let's use the existing way of adding options directly for tab completion.
Thanks for the comments, Attached v4 patch has the fixes for the same.
[1] -
https://www.postgresql.org/message-id/3690759.1621527026%40sss.pgh.pa.us
Regards,
Vignesh
From f5bb987ff172f65680605af5e9df895a49310e0e Mon Sep 17 00:00:00 2001
From: vignesh <vignes...@gmail.com>
Date: Fri, 14 May 2021 11:37:05 +0530
Subject: [PATCH v4] Add tab completion for missing options in
PUBLICATION and SUBSCRIPTION commands.
Tab completion for the options streaming and binary were missing in case of
"CREATE SUBSCRIPTION WITH" and "ALTER SUBSCRIPTION SET" commands.
Tab completion for the option publish_via_partition_root was missing in case
of "CREATE PUBLICATION WITH" and "ALTER PUBLICATION SET" commands.
This patch fixes them.
---
src/bin/psql/tab-complete.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 6598c5369a..8314dfa226 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1646,7 +1646,7 @@ psql_completion(const char *text, int start, int end)
COMPLETE_WITH("(", "TABLE");
/* ALTER PUBLICATION <name> SET ( */
else if (HeadMatches("ALTER", "PUBLICATION", MatchAny) && TailMatches("SET", "("))
- COMPLETE_WITH("publish");
+ COMPLETE_WITH("publish", "publish_via_partition_root");
/* ALTER SUBSCRIPTION <name> */
else if (Matches("ALTER", "SUBSCRIPTION", MatchAny))
COMPLETE_WITH("CONNECTION", "ENABLE", "DISABLE", "OWNER TO",
@@ -1665,7 +1665,7 @@ psql_completion(const char *text, int start, int end)
COMPLETE_WITH("(", "PUBLICATION");
/* ALTER SUBSCRIPTION <name> SET ( */
else if (HeadMatches("ALTER", "SUBSCRIPTION", MatchAny) && TailMatches("SET", "("))
- COMPLETE_WITH("slot_name", "synchronous_commit");
+ COMPLETE_WITH("binary", "slot_name", "streaming", "synchronous_commit");
/* ALTER SUBSCRIPTION <name> SET PUBLICATION */
else if (HeadMatches("ALTER", "SUBSCRIPTION", MatchAny) && TailMatches("SET", "PUBLICATION"))
{
@@ -2638,7 +2638,7 @@ psql_completion(const char *text, int start, int end)
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables, NULL);
/* Complete "CREATE PUBLICATION <name> [...] WITH" */
else if (HeadMatches("CREATE", "PUBLICATION") && TailMatches("WITH", "("))
- COMPLETE_WITH("publish");
+ COMPLETE_WITH("publish", "publish_via_partition_root");
/* CREATE RULE */
/* Complete "CREATE [ OR REPLACE ] RULE <sth>" with "AS ON" */
@@ -2758,8 +2758,9 @@ psql_completion(const char *text, int start, int end)
COMPLETE_WITH("WITH (");
/* Complete "CREATE SUBSCRIPTION <name> ... WITH ( <opt>" */
else if (HeadMatches("CREATE", "SUBSCRIPTION") && TailMatches("WITH", "("))
- COMPLETE_WITH("copy_data", "connect", "create_slot", "enabled",
- "slot_name", "synchronous_commit");
+ COMPLETE_WITH("binary", "copy_data", "connect", "create_slot",
+ "enabled", "slot_name", "streaming",
+ "synchronous_commit");
/* CREATE TRIGGER --- is allowed inside CREATE SCHEMA, so use TailMatches */
--
2.25.1
