Re: alter subscription drop publication fixes

On Wed, May 12, 2021 at 9:55 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
> While I was reviewing one of the logical decoding features, I found a
> few issues in alter subscription drop publication.

Thanks!

> Alter subscription drop publication does not support copy_data option,
> that needs to be removed from tab completion.

+1. You may want to also change set_publication_option(to something
like drop_pulication_option with only refresh option) for the drop in
the docs? Because "Additionally, refresh options as described under
REFRESH PUBLICATION may be specified." doesn't make sense.

> Dropping all the publications present in the subscription using alter
> subscription drop publication would throw "subscription must contain
> at least one publication". This message was slightly confusing to me.
> As even though some publication was present on the subscription I was
> not able to drop. Instead I feel we could throw an error message
> something like "dropping specified publication will result in
> subscription without any publication, this is not supported".

-1 for that long message. The intention of that error was to throw an
error if all the publications of a subscription are dropped. If that's
so confusing, then you could just let the error message be
"subscription must contain at least one publication", add an error
detail "Subscription without any publication is not allowed to
exist/is not supported." or "Removing/Dropping all the publications
from a subscription is not allowed/supported." or some other better
wording.

> merge_publications can be called after validation of the options
> specified, I think we should check if the options specified are
> correct or not before checking the actual publications.

+1. That was a miss in the original feature.

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