On Mon, Mar 10, 2025 at 5:02 PM Nisha Moond <nisha.moond...@gmail.com> wrote: > > On Mon, Mar 10, 2025 at 4:31 PM Shubham Khanna > <khannashubham1...@gmail.com> wrote: > > > > > 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 for the patch. Here are few comments for the new change - > 1) > +static void check_and_drop_existing_publications(PGconn *conn, struct > LogicalRepInfo *dbinfo); > - move the second parameter to the next line to maintain 80 char length. >
Fixed.
> 2)
> pg_log_error("could not drop publication \"%s\" in database \"%s\": %s",
> - dbinfo->pubname, dbinfo->dbname, PQresultErrorMessage(res));
> + pubname, dbinfo->dbname,
> + PQresultErrorMessage(res));
>
> You can keep "PQresultErrorMessage(res));" in the previous line, it
> will still be < 80 chars.
>
Fixed.
> 3) The new IF check -
> + if (strcmp(pubname, dbinfos.dbinfo->pubname) == 0)
> + dbinfo->made_publication = false; /* don't try again. */
>
> always compares dbinfo[0]->pubname, but it should compare 'pubname'
> with the respective database's publication. Please correct it as -
> if (strcmp(pubname, dbinfo->pubname) == 0)
>
>
> --
Fixed.
The attached patch contains the suggested changes.
Thanks and regards,
Shubham Khanna.
v16-0001-Support-for-dropping-all-publications-in-pg_crea.patch
Description: Binary data
