MIME-Version: 1.0
References: 
 <CAJpy0uBT8JbEGE0xvm-Wxh1g_VVgC=whKqChZo-uB+VOa_YCTw@mail.gmail.com>
 <CAE9k0Pkk6q72X3Rc3MUo7PxU46UcCzLfMhM02PGDUmAue9cDGg@mail.gmail.com>
 <TY4PR01MB17718104B91F2945BE727467694102@TY4PR01MB17718.jpnprd01.prod.outlook.com>
 <CAE9k0P=dgCEaKE6+vSCQp8TgrYOi_RqQkDTScdWzFSECsPQn9w@mail.gmail.com>
 <TY4PR01MB17718F364B2CB8108B34FA5FC94112@TY4PR01MB17718.jpnprd01.prod.outlook.com>
 <CAJpy0uCnasi4MSQB=nrjPSv4U_0rb2Z-cg_wazUGQ-P_VnRZeA@mail.gmail.com>
 <CAA4eK1JLwL_aauy+nrrobC3Sftv369N3N6DaidBpNJdiOwXG+g@mail.gmail.com>
 <CAJpy0uBR-8-d__LgfqqUbFvLBvzL6bY_kjUmBWAVsLLu1QZEPA@mail.gmail.com>
 <CAE9k0P=uMVGV5TXTVdjKzu+0gFLMKHNY2usnAzK0L2cjbmPHBg@mail.gmail.com>
 <CANhcyEWcziK1eR=A4_TW_PbeBAgfBacck2R=sYOqAs5=5g+2=Q@mail.gmail.com>
 <CAE9k0PkxvD_Rbz1qm3ZHOX_n-JXaOiPV2wusvb6KVwQqifpt6w@mail.gmail.com>
In-Reply-To: 
 <CAE9k0PkxvD_Rbz1qm3ZHOX_n-JXaOiPV2wusvb6KVwQqifpt6w@mail.gmail.com>
From: Shlok Kyal <shlok.kyal.oss@gmail.com>
Date: Tue, 16 Jun 2026 12:09:47 +0530
Message-ID: 
 <CANhcyEXPYAsLRJOwY6dqtLn82yhyPGv8kCtkc_HcwkbADr_j-Q@mail.gmail.com>
Subject: Re: synchronized_standby_slots behavior inconsistent with
 quorum-based synchronous replication
To: Ashutosh Sharma <ashu.coek88@gmail.com>
Cc: shveta malik <shveta.malik@gmail.com>,
 Amit Kapila <amit.kapila16@gmail.com>,
	"Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>,
 Ajin Cherian <itsajin@gmail.com>,
	SATYANARAYANA NARLAPURAM <satyanarlapuram@gmail.com>,
	PostgreSQL-development <pgsql-hackers@postgresql.org>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CANhcyEXPYAsLRJOwY6dqtLn82yhyPGv8kCtkc_HcwkbADr_j-Q%40mail.gmail.com>
Precedence: bulk

On Mon, 15 Jun 2026 at 16:09, Ashutosh Sharma <ashu.coek88@gmail.com> wrote=
:
>
> Hi Shlok,
>
> Thanks for reviewing the patch and sharing your feedback - all your
> comments are well noted. Please find my responses below.
>
> On Fri, Jun 12, 2026 at 6:33=E2=80=AFPM Shlok Kyal <shlok.kyal.oss@gmail.=
com> wrote:
> >
> > Hi Ashutosh,
> >
> > I have reviewed the patches. Here are some comments:
> >
> > 1. Should we update the doc for function "pg_logical_slot_get_changes".=
 It says:
> > ```
> > If the specified slot is a logical failover slot then the function will
> > not return until all physical slots specified in
> > <link linkend=3D"guc-synchronized-standby-slots"><varname>synchronized_=
standby_slots</varname></link>
> > have confirmed WAL receipt.
> > ```
> > This line "return until all physical slots specified in" seems wrong
> > after introduction of "FIRST/ANY"
> >
>
> Agreed, it indeed needs correction, it has been addressed in the attached=
 patch.
>
> > 2. Similarly, should we update the doc for function
> > "pg_replication_slot_advance"? It also says:
> > ```
> > If the specified slot is a
> > logical failover slot then the function will not return until all
> > physical slots specified in
> > <link linkend=3D"guc-synchronized-standby-slots"><varname>synchronized_=
standby_slots</varname></link>
> > have confirmed WAL receipt.
> > ```
>
> Same as comment 1, this has been corrected in the attached patch as well
>
> >
> > 3. Comment in syncrep_gram.y states:
> >  * syncrep_gram.y - Parser for synchronous_standby_names
> >
> > Since synchronosed_standby_slots is reusing this, should we update this=
 comment?
> >
>
> Agreed, update was due. This has been taken care of in the attached patch=
.
>
> > 4. Similarly for syncrep_scanner.l, we have comment:
> >  * syncrep_scanner.l
> >  *   a lexical scanner for synchronous_standby_names
> >
> >  Should we update it?
>
> Agreed and updated in the attached patch.
>
> >
> > 5. enum "SyncStandbySlotsState" and stuct "SyncStandbySlotsStateInfo" s=
hould be
> > mentioned in typedefs.list, so that pgindent works properly.
> >
>
> These have been added to the typdefs.list
>
> PFA attached patches with all these changes.
>
Hi Ashutosh,

I checked the CFbot and saw the test was failing [1].

It is failing for the test:
not ok 4     - guc_privs                                 171 ms

diff -U3 /__w/postgresql/postgresql/src/test/modules/unsafe_tests/expected/=
guc_privs.out
/__w/postgresql/postgresql/build/testrun/unsafe_tests/regress/results/guc_p=
rivs.out
--- /__w/postgresql/postgresql/src/test/modules/unsafe_tests/expected/guc_p=
rivs.out
2026-06-15 11:08:26.979460240 +0000
+++ /__w/postgresql/postgresql/build/testrun/unsafe_tests/regress/results/g=
uc_privs.out
2026-06-15 11:20:01.829423596 +0000
@@ -590,8 +590,7 @@
 -- Cannot set synchronized_standby_slots to an invalid slot name
 ALTER SYSTEM SET synchronized_standby_slots=3D'invalid*';
 ERROR:  invalid value for parameter "synchronized_standby_slots": "invalid=
*"
-DETAIL:  replication slot name "invalid*" contains invalid character
-HINT:  Replication slot names may only contain lower case letters,
numbers, and the underscore character.
+DETAIL:  syntax error at or near "*"
 -- Can set synchronized_standby_slots to a non-existent slot name
 ALTER SYSTEM SET synchronized_standby_slots=3D'missing';
 -- Reset the GUC

In HEAD, validate_sync_standby_slots() called SplitIdentifierString()
and then ReplicationSlotValidateNameInternal() for each element, which
produced the slot-name-specific error for inputs such as 'invalid*'.
With patch synchronized_standby_slots validation use the syncrep
parser and a syntax error is reported instead.
I think we should update the corresponding expected file.

[1]: https://github.com/postgresql-cfbot/postgresql/actions/runs/2754199642=
6/job/81406065089

Thanks,
Shlok Kyal