Re: Improve pg_sync_replication_slots() to wait for primary to advance - Mailing list pgsql-hackers

>
>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