Re: [PATCH] Preserve replication origin OIDs in pg_upgrade

On Tue, May 12, 2026 at 4:32 AM Zsolt Parragi <zsolt.parragi@percona.com> wrote:
>
> A few more minor comments:
>
> binary_upgrade_replorigin_advance seems like dead code in the patch
> now, it has no callers since the patch removes its only use.
>

Removed.

> + ReplOriginId node;
> ....
> + node = PG_GETARG_OID(0);
>
> ReplOriginId is uint16, this silently truncates it. Since this is a
> generic callable function shouldn't there be at least a check about
> it?

Added check.

>
> Also, seems like the same function
> (binary_upgrade_create_replication_origin) locks-unlocks-locks
> ReplicationOriginRelationId, that doesn't seem the best approach with
> a single-use helper function? (first in
> replorigin_create_with_reploriginid, then
> binary_upgrade_create_replication_origin reaquires it)

Rewrote the function and removed replorigin_create_with_reploriginid
to do all the work in binary_upgrade_create_replication_origin()
itself.

>
> + if (PQntuples(res) > 0 && archDumpFormat == archNull)
> + fprintf(OPF, "--\n-- Replication Origins \n--\n\n");
>
> The caller also checks the format, it is redundant.
>

Fixed.

> + /* Get replication origins in current database. */
> + appendPQExpBufferStr(buf,
>
> Isn't pg_replication_origin a shared catalog?

Fixed.

Also changed the definition of
binary_upgrade_create_replication_origin to proparallel => 'u', as
parallel execution was causing a failure on FreeBSD in cfbot.

regards,
Ajin Cherian
Funitsu Australia