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?
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
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?
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.
With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com
