Re: issue with synchronized_standby_slots

Hi,

On Wed, Sep 24, 2025 at 3:45 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Wed, Sep 24, 2025 at 12:14 PM Ashutosh Sharma <ashu.coek88@gmail.com> wrote:
> >
> > Hi Amit,
> >
> > On Tue, Sep 23, 2025 at 1:00 PM Shlok Kyal <shlok.kyal.oss@gmail.com> wrote:
> > >
> > > On Tue, 23 Sept 2025 at 09:55, Amit Kapila <amit.kapila16@gmail.com> wrote:
> > > >
> > > > On Fri, Sep 12, 2025 at 2:34 PM Shlok Kyal <shlok.kyal.oss@gmail.com> 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.