Re: Identify missing publications from publisher while create/alter subscription.

On Fri, Jan 22, 2021 at 10:14 AM japin <japinli(at)hotmail(dot)com> wrote:
>
>
> On Fri, 22 Jan 2021 at 00:51, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
> > On Thu, Jan 21, 2021 at 6:56 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
> >>
> >> Hi,
> >>
> >> Creating/altering subscription is successful when we specify a
> >> publication which does not exist in the publisher. I felt we should
> >> throw an error in this case, that will help the user to check if there
> >> is any typo in the create subscription command or to create the
> >> publication before the subscription is created.
> >> If the above analysis looks correct, then please find a patch that
> >> checks if the specified publications are present in the publisher and
> >> throws an error if any of the publications is missing in the
> >> publisher.
> >> Thoughts?
> >
> > I was having similar thoughts (while working on the logical
> > replication bug on alter publication...drop table behaviour) on why
> > create subscription succeeds without checking the publication
> > existence. I checked in documentation, to find if there's a strong
> > reason for that, but I couldn't. Maybe it's because of the principle
> > "first let users create subscriptions, later the publications can be
> > created on the publisher system", similar to this behaviour
> > "publications can be created without any tables attached to it
> > initially, later they can be added".
> >
> > Others may have better thoughts.
> >
> > If we do check publication existence for CREATE SUBSCRIPTION, I think
> > we should also do it for ALTER SUBSCRIPTION ... SET PUBLICATION.
> >
>
> Agreed. Current patch do not check publication existence for
> ALTER SUBSCRIPTION ... SET PUBLICATION ... WITH (refresh = false).
>
> > I wonder, why isn't dropping a publication from a list of publications
> > that are with subscription is not allowed?
> >
> > Some comments so far on the patch:
> >
> > 1) I see most of the code in the new function check_publications() and
> > existing fetch_table_list() is the same. Can we have a generic
> > function, with maybe a flag to separate out the logic specific for
> > checking publication and fetching table list from the publisher.
>
> +1
>
> > 2) Can't we know whether the publications exist on the publisher with
> > the existing (or modifying it a bit if required) query in
> > fetch_table_list(), so that we can avoid making another connection to
> > the publisher system from the subscriber?
>
> IIUC, the patch does not make another connection, it just execute a new
> query in already connection. If we want to check publication existence
> for ALTER SUBSCRIPTION ... SET PUBLICATION ... WITH (refresh = false)
> we should make another connection.
>
> > 3) If multiple publications are specified in the CREATE SUBSCRIPTION
> > query, IIUC, with your patch, the query fails even if at least one of
> > the publications doesn't exist. Should we throw a warning in this case
> > and allow the subscription be created for other existing
> > publications?
> >
>
> +1. If all the publications do not exist, we should throw an error.

I also felt if any of the publications are not there, we should throw an error.

Regards,
Vignesh
EnterpriseDB: http://www.enterprisedb.com