At 2025-12-10 13:07:34, "Ajin Cherian" <[email protected]> wrote: > >I'm not sure if this is much of an improvement, I like the current >approach and matches with similar coding patterns in the code base. > >Attaching v34 addressing the above comments. >
Hi, Few comments for v34. 1 - 0002 ``` --- a/src/backend/replication/logical/slotsync.c +++ b/src/backend/replication/logical/slotsync.c @@ -39,6 +39,12 @@ * the last cycle. Refer to the comments above wait_for_slot_activity() for * more details. * + * If the SQL function pg_sync_replication is used to sync the slots, and if ``` Typo, it should be "pg_sync_replication_slots()" instead of "pg_sync_replication". 2 - 0002 ``` + /* + * The syscache access in fetch_or_refresh_remote_slots() needs a + * transaction env. + */ ``` Typo, it should be "fetch_remote_slots()" instead of "fetch_or_refresh_remote_slots()". 3 - 0002 ``` + appendStringInfo(&query, "'%s'", slot_name); ``` Instead of manually add single quotes to slot name, consider using quote_literal_cstr(). While I was reviewing patch v32, Ajin Cherian had already submitted patch v34, but these issues still persisted. Best regards, -- Yilin Zhang
