Re: Improve pg_sync_replication_slots() to wait for primary to advance

At 2025-12-10 13:07:34, "Ajin Cherian" <itsajin(at)gmail(dot)com> 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