On Wed, Aug 19, 2026 at 11:13 AM Ajin Cherian <[email protected]> wrote:
>
>
> I have incorporated your comments and your changes in a new patch v16.
>
Please find a few minor comments:
1)
+ if (!PQgetisnull(res, i, i_remotelsn))
+ {
+ appendPQExpBufferStr(buf, ", ");
+ appendStringLiteralConn(buf, PQgetvalue(res, i, i_remotelsn), conn);
+ appendPQExpBufferStr(buf, "::pg_catalog.pg_lsn");
+ }
+ else
+ appendPQExpBufferStr(buf, ", NULL");
It is recommended to typecast even NULL to datatype to remove any
ambiguity in function signature matching (for future cases). IIUC,
Null is treated as unknown datatype.
Suggestion:
appendPQExpBufferStr(buf, ", NULL::pg_catalog.pg_lsn");
2)
+ * Create a replication origin with a specific ID and name, optionally
+ * restoring its remote_lsn.
+ *
+ * The created replication origin is advanced if need_advance is true. It can
+ * be set only in binary upgrade mode.
'optionally restoring its remote_lsn' and 'origin is advanced' seems
like 2 different steps while both are same and are dependent upon
'need_advance'. Suggestion:
Create a replication origin with a specific ID and name, optionally
restoring its remote_lsn when need_advance is true. need_advance can
be set only in binary upgrade mode.
3)
- Commit timestamps and origin data are not preserved during the upgrade.
+ Commit timestamps are not preserved during the upgrade.
We can make it sligtly more clear if you agree. Suggestion:
Commit timestamps (which include the origin of the commit) are not
preserved during the upgrade.
4)
+ old subscriber was up to. Migration of the subscription's replication
+ origin is supported for all cluster versions.
Shall we say:
all supported cluster versions
5)
+ configured to a value greater than or equal to the number of tracked
+ replication origins present in the old cluster (reflected in
+ <link
linkend="view-pg-replication-origin-status"><structname>pg_replication_origin_status</structname></link>).
+ The new cluster must contain no replication origins.
To give better continuity in this paragraph, the last sentence can be
changed to:
Furthermore, the new cluster must not contain any pre-existing
replication origins."
thanks
Shveta
