On Mon, Mar 10, 2025 at 3:09 PM Nisha Moond <nisha.moond...@gmail.com> wrote: > > On Mon, Mar 10, 2025 at 12:11 PM Shubham Khanna > <khannashubham1...@gmail.com> wrote: > > > > On Thu, Mar 6, 2025 at 9:27 AM Peter Smith <smithpb2...@gmail.com> wrote: > > > > > > 6. > > > - dbinfo->pubname, dbinfo->dbname, PQresultErrorMessage(res)); > > > - dbinfo->made_publication = false; /* don't try again. */ > > > + pubname, dbname, PQresultErrorMessage(res)); > > > + dbinfos.dbinfo->made_publication = false; /* don't try again. */ > > > > > > Something about this flag assignment seems odd to me. IIUC > > > 'made_publications' is for removing the cleanup_objects_atexit to be > > > able to remove the special publication implicitly made by the > > > pg_createsubscriber. But, AFAIK we can also get to this code via the > > > --cleanup-existing-publication switch, so actually it might be one of > > > the "existing" publication DROPS that has failed. If so, then the > > > "don't try again comment" is misleading because it may not be that > > > same publication "again" at all. > > > > > > > I agree with your point. The current comment is misleading, as the > > failure could be related to an existing publication drop via > > --cleanup-existing-publications now --drop-all-publications, not just > > the publication created by pg_createsubscriber. > > This issue is still open, and I will address it in the next version of > > the patch. > > > > 1) We should set "made_publication = false" only if the failure occurs > for the special publication implicitly created by pg_createsubscriber. > If the error is in any other publication, this special publication > should still be cleaned up by the cleanup_objects_atexit. >
Fixed. > 2) This part of code has another bug: > "dbinfos.dbinfo->made_publication = false;" incorrectly modifies > dbinfo[0], even if the failure occurs in other databases (dbinfo[1], > etc.). Since cleanup_objects_atexit() checks made_publication per > database, this could lead to incorrect behavior. > Solution: Pass the full 'dbinfo' structure to > drop_publication_by_name() , and use it accordingly. > > -- Fixed. The attached patch contains the suggested changes. Thanks and regards, Shubham Khanna.
v15-0001-Support-for-dropping-all-publications-in-pg_crea.patch
Description: Binary data
