Re: Added missing tab completion for alter subscription set option

On Wed, May 19, 2021 at 2:03 PM Bharath Rupireddy <
bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
>
> On Tue, May 18, 2021 at 9:21 PM Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)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