On Mon, 15 Jun 2026 at 16:09, Ashutosh Sharma <[email protected]> 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 PM Shlok Kyal <[email protected]> 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="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="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" > > should 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_privs.out --- /__w/postgresql/postgresql/src/test/modules/unsafe_tests/expected/guc_privs.out 2026-06-15 11:08:26.979460240 +0000 +++ /__w/postgresql/postgresql/build/testrun/unsafe_tests/regress/results/guc_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='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='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/27541996426/job/81406065089 Thanks, Shlok Kyal
