Re: Adding a '--clean-publisher-objects' option to 'pg_createsubscriber' utility.

On Tue, Mar 18, 2025 at 5:17 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Tue, Mar 18, 2025 at 4:01 PM Shubham Khanna
> <khannashubham1197@gmail.com> wrote:
> >
> > On Tue, Mar 18, 2025 at 12:07 PM David G. Johnston
> > <david.g.johnston@gmail.com> wrote:
> > >
> >
> > > It would be good if we could get this to play nicely with —dry-run; maybe connecting to the source for the
queriesinstead of the target.  That would help alleviate my issue with the current auto-drop behavior. 
> > >
> >
> ...
> > IIUC, for testing with --dry-run, we can directly check the relevant
> > stdout logs (e.g., "dropping publication 'test_pub1' ...") to verify
> > the call without actually dropping the publications.
> > However, IMO, using --dry-run alone would miss code coverage for the
> > actual drop publication execution part.
> >
>
> I don't understand --dry-run part of conversation here. As per
> existing code (or with the patch), we seem to be already printing the
> publications to be dropped in dry-run mode.
>
> *
> - drop_publication(conn, &dbinfo[i]);
> + if (dbinfos.remove_objects & OBJECT_PUBLICATIONS)
> + drop_all_publications(conn, &dbinfo[i]);
> + else
> + drop_publication(conn, &dbinfo[i], dbinfo[i].pubname);
>
> I was expecting a call like the existing function
> check_and_drop_existing_subscriptions(). The new function should check
> if the user requested to remove the publication then it should query
> the publisher, otherwise, just remove the one specified by dbinfo.
> Also, the core drop_publication() function should take the required
> parameters instead of dbinfo after this patch. That would simplify the
> code.
>
> --

I have updated the function drop_all_publications() to
check_and_drop_publications(), which now checks if the user requested
to remove the publication; if so, it queries the publisher. Otherwise,
it removes the one specified by dbinfo. Additionally, the
drop_publication() function now takes the required parameters directly
instead of dbinfo, simplifying the code.

The attached patch contains the suggested changes.

Thanks and regards,
Shubham Khanna.