Re: Added missing tab completion for alter subscription set option

Поиск
Список
Период
Сортировка
От vignesh C
Тема Re: Added missing tab completion for alter subscription set option
Дата
Msg-id CALDaNm2J5dL30eH6kUjhMmeWpU+NUAg4B9sGuB+DAf0oBVk66g@mail.gmail.com
обсуждение исходный текст
Ответ на Re: Added missing tab completion for alter subscription set option  (Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>)
Ответы Re: Added missing tab completion for alter subscription set option  (Michael Paquier <michael@paquier.xyz>)
Список pgsql-hackers


On Wed, May 19, 2021 at 2:03 PM Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> wrote:
>
> On Tue, May 18, 2021 at 9:21 PM Alvaro Herrera <alvherre@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
Вложения

В списке pgsql-hackers по дате отправления:

Предыдущее
От: vignesh C
Дата:
Сообщение: Re: Added missing tab completion for alter subscription set option
Следующее
От: Mark Dilger
Дата:
Сообщение: Re: Fixing the docs for ALTER SUBSCRIPTION ... ADD/DROP PUBLICATION