Re: pg_createsubscriber: Fix incorrect handling of cleanup flags - Mailing list pgsql-hackers

From David G. Johnston
Subject Re: pg_createsubscriber: Fix incorrect handling of cleanup flags
Date
Msg-id CAKFQuwaqWa3tRNMaRYpa4jE7BVSMb1tPTeiwM9GE7781nKOPEQ@mail.gmail.com
Whole thread Raw
In response to pg_createsubscriber: Fix incorrect handling of cleanup flags  (Nisha Moond <nisha.moond412@gmail.com>)
List pgsql-hackers
On Sun, May 4, 2025 at 8:45 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
Attached is the patch implementing the above proposed solution.
Reviews and feedback are most welcome.

I feel like this is just papering over the issue - which is that these two drop functions are being used for multiple differently named publications/slots yet take no care to ensure they only change made_publication and made_repslot if the name of the object being passed in matches the name of the object those two booleans are specifically tracking (the application created objects on the publisher).

Make it so they are only changed to false if the name matches the one the program created and the connection is the primary connection.  That targets the real issue and avoids using a branching boolean parameter.

It seems really odd to say: if (in_cleanup) "don't try again" - since by definition this is the last thing we are doing before we exit.  So really what this patch can do more simply is just remove the dbinfo->made_replslot=false and *made_publication=false lines altogether - which might be a valid option.

I'm partial to the latter really, I don't think the error message output for retrying a drop that may have previously failed would be an issue.

David J.

pgsql-hackers by date:

Previous
From: DIPESH DHAMELIYA
Date:
Subject: [PATCH] Allow parallelism for plpgsql return expression after commit 556f7b7
Next
From: Amit Kapila
Date:
Subject: Re: Add an option to skip loading missing publication to avoid logical replication failure