As Ashutosh suggests I will go more for the backpatching approach because the synchronized_standby_slots parameter impacts the last 2 major versions and this problem is critical on production environments.
Best regards Fabrice On Fri, Sep 26, 2025 at 9:00 AM Ashutosh Sharma <[email protected]> wrote: > Hi, > > On Wed, Sep 24, 2025 at 3:45 PM Amit Kapila <[email protected]> > wrote: > > > > On Wed, Sep 24, 2025 at 12:14 PM Ashutosh Sharma <[email protected]> > wrote: > > > > > > Hi Amit, > > > > > > On Tue, Sep 23, 2025 at 1:00 PM Shlok Kyal <[email protected]> > wrote: > > > > > > > > On Tue, 23 Sept 2025 at 09:55, Amit Kapila <[email protected]> > wrote: > > > > > > > > > > On Fri, Sep 12, 2025 at 2:34 PM Shlok Kyal < > [email protected]> wrote: > > > > > > > > > > > > I have attached the updated v4 patch > > > > > > > > > > > > > > > > +# Cannot be set synchronized_standby_slots to a reserved slot name > > > > > +($result, $stdout, $stderr) = $primary->psql('postgres', > > > > > + "ALTER SYSTEM SET > synchronized_standby_slots='pg_conflict_detection'"); > > > > > +ok( $stderr =~ > > > > > + m/WARNING: replication slot name "pg_conflict_detection" is > reserved/, > > > > > + "Cannot use a reserverd slot name"); > > > > > + > > > > > +# Cannot be set synchronized_standby_slots to slot name with > invalid characters > > > > > +($result, $stdout, $stderr) = $primary->psql('postgres', > > > > > + "ALTER SYSTEM SET synchronized_standby_slots='invalid*'"); > > > > > +ok( $stderr =~ > > > > > + m/WARNING: replication slot name "invalid\*" contains invalid > character/, > > > > > + "Cannot use a invalid slot name"); > > > > > > > > > > These tests can be present in some sql file. I think you have kept > > > > > these in the .pl file to keep it along with other tests but I think > > > > > these are better suited for some .sql file. > > > > > > > > > Thanks for reviewing the patch. > > > > I have moved the tests to the guc.sql file. I have attached the > updated patch. > > > > > > > > > > Are we planning to wait for [1] to go in first, since this also > > > depends on ReplicationSlotValidateName? > > > > > > > I think we can go either way. It somewhat depends on whether we want > > to backpatch this change. The argument for having this as a HEAD-only > > patch is that, we have a similar behavior for other variables like > > default_table_access_method and default_tablespace in HEAD and > > back-branches. We want to change to a better behavior for this GUC, so > > better to keep this as a HEAD-only patch. Do you or others have > > thoughts on this matter? > > > > If we decide to do this as a HEAD-only patch, then I think we can push > > this without waiting for the other patch to commit as we have ample > > time to get that done and we already have a solution as well for it. > > OTOH, if we want to backpatch this then I would prefer to wait for the > > other patch to be committed first. > > > > Although it's a behaviour change at GUC level, to me it doesn't look > like something that would break the user applications, so I'm inclined > toward backpatching it, but I'd definitely want to hear if others see > potential compatibility risks that I might be missing. > > -- > With Regards, > Ashutosh Sharma. >
