Re: Support ALTER SUBSCRIPTION ... ADD/DROP PUBLICATION ... syntax - Mailing list pgsql-hackers

From japin
Subject Re: Support ALTER SUBSCRIPTION ... ADD/DROP PUBLICATION ... syntax
Date
Msg-id MEYP282MB16692A564887C2F44F35DEB2B6BC0@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM
Whole thread Raw
In response to Re: Support ALTER SUBSCRIPTION ... ADD/DROP PUBLICATION ... syntax  (japin <japinli@hotmail.com>)
List pgsql-hackers
On Tue, 26 Jan 2021 at 13:46, japin <japinli@hotmail.com> wrote:
> Hi, Bharath
>
> Thanks for your reviewing.
>
> On Tue, 26 Jan 2021 at 12:55, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> wrote:
>> On Tue, Jan 26, 2021 at 9:25 AM japin <japinli@hotmail.com> wrote:
>>> > I think it's more convenient. Any thoughts?
>>>
>>> Sorry, I forgot to attach the patch.
>>
>> As I mentioned earlier in [1], +1 from my end to have the new syntax
>> for adding/dropping publications from subscriptions i.e. ALTER
>> SUBSCRIPTION ... ADD/DROP PUBLICATION. But I'm really not sure why
>> that syntax was not added earlier. Are we missing something here?
>>
>
> Yeah, we should figure out why we do not support this syntax earlier.  It seems
> ALTER SUBSCRIPTION is introduced in 665d1fad99e, however the commit do not
> contains any discussion URL.
>
>> I would like to hear opinions from other hackers.
>>
>> [1] -
https://www.postgresql.org/message-id/CALj2ACVGDNZDQk3wfv%3D3zYTg%3DqKUaEa5s1f%2B9_PYxN0QyAUdCw%40mail.gmail.com
>>
>> Some quick comments on the patch, although I have not taken a deeper look at it:
>>
>> 1. IMO, it will be good if the patch is divided into say coding, test
>> cases and documentation
>
> Agreed.  I will refactor it in next patch.
>

Split v1 path into coding, test cases, documentation and tab-complete.

>> 2. Looks like AlterSubscription() is already having ~200 LOC, why
>> can't we have separate functions for each ALTER_SUBSCRIPTION_XXXX case
>> or at least for the new code that's getting added for this patch?
>
> I'm not sure it is necessary to provide a separate functions for each
> ALTER_SUBSCRIPTION_XXX, so I just followed current style.
>
>> 3. The new code added for ALTER_SUBSCRIPTION_ADD_PUBLICATION and
>> ALTER_SUBSCRIPTION_DROP_PUBLICATION looks almost same maybe with
>> little difference, so why can't we have single function
>> (alter_subscription_add_or_drop_publication or
>> hanlde_add_or_drop_publication or some better name?) and pass in a
>> flag to differentiate the code that differs for both commands.
>
> Agreed.

At present, I create a new function merge_subpublications() to merge the origin
publications and add/drop publications.  Thoughts?

-- 
Regrads,
Japin Li.
ChengDu WenWu Information Technology Co.,Ltd.


Attachment

pgsql-hackers by date:

Previous
From: "Hou, Zhijie"
Date:
Subject: RE: Parallel INSERT (INTO ... SELECT ...)
Next
From: Kyotaro Horiguchi
Date:
Subject: Re: shared tempfile was not removed on statement_timeout