Re: Support ALTER SUBSCRIPTION ... ADD/DROP PUBLICATION ... syntax

On Wed, Jan 27, 2021 at 3:01 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > > While the new proposed syntax does seem to provide some ease for users
> > > but it has nothing which we can't do with current syntax. Also, in the
> > > current syntax, there is an additional provision for refreshing the
> > > existing publications as well. So, if the user has to change the
> > > existing subscription such that it has to (a) add new publication(s),
> > > (b) remove some publication(s), (c) refresh existing publication(s)
> > > then all can be done in one command whereas with your new proposed
> > > syntax user has to write three separate commands.
> >
> > IIUC the initial patch proposed here, it does allow ALTER SUBSCRIPTION
> > mysub1 ADD/DROP PUBLICATION mypub4 WITH (refresh = true);. Isn't this
> > option enough to achieve what we can with ALTER SUBSCRIPTION mysub1
> > SET PUBLICATION mypub1, mypub2 WITH (refresh = true);? Am I missing
> > something here?
> >
>
> I feel the SET syntax would allow refreshing existing publications as
> well whereas, in Add, it will be only for new Publications.

I think the patch v2-0001 will refresh all the publications, I mean
existing and newly added publications. IIUC the patch, it first
fetches all the available publications with the subscriptions and it
sees if that list has the given publication [1], if not, then adds it
to the existing publications list and returns that list [2]. If the
refresh option is specified as true with ALTER SUBSCRIPTION ... ADD
PUBLICATION, then it refreshes all the returned publications [3]. I
believe this is also true with ALTER SUBSCRIPTION ... DROP
PUBLICATION.

So, I think the new syntax, ALTER SUBSCRIPTION .. ADD/DROP PUBLICATION
will refresh the new and existing publications.

[1]
+
+/*
+ * merge current subpublications and user specified by add/drop publications.
+ *
+ * If addpub == true, we will add the list of publications into current
+ * subpublications. Otherwise, we will delete the list of publications from
+ * current subpublications.
+ */
+static List *
+merge_subpublications(HeapTuple tuple, TupleDesc tupledesc,
+ List *publications, bool addpub)

[2]
+ publications = merge_subpublications(tup,
RelationGetDescr(rel),

[3]
+ /* Refresh if user asked us to. */
+ if (refresh)
+ {
+ if (!sub->enabled)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("ALTER SUBSCRIPTION with
refresh is not allowed for disabled subscriptions"),
+ errhint("Use ALTER SUBSCRIPTION ...
SET PUBLICATION ... WITH (refresh = false).")));
+
+ /* Make sure refresh sees the new list of publications. */
+ sub->publications = publications;
+
+ AlterSubscription_refresh(sub, copy_data);

With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com