Re: [PATCH] Preserve replication origin OIDs in pg_upgrade

From: shveta malik <shveta(dot)malik(at)gmail(dot)com>
To: Rui Zhao <zhaorui126(at)gmail(dot)com>
Cc: Ajin Cherian <itsajin(at)gmail(dot)com>, Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org, Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com>, shveta malik <shveta(dot)malik(at)gmail(dot)com>
Subject: Re: [PATCH] Preserve replication origin OIDs in pg_upgrade
Date: 2026-07-08 04:07:51
Message-ID: CAJpy0uD88AsDe=DJsnd5pD2_J9hDdSpwL+b8xcMfMJ+X9JZtSQ@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Jul 6, 2026 at 6:40 PM Rui Zhao <zhaorui126(at)gmail(dot)com> wrote:
>
> Hi Ajin,
>
> Shveta's right, sorry for the noise -- git am --3way applies v10 cleanly on
> HEAD (9d1188f298) and it builds. "git apply --check" was tripping on the
> shifted pg_proc.dat hunk on my end.
>
> Confirmed Shveta's (6), and it's a crash, not just a bad read:

Thanks for verifying it.

> cluster->sub_retain_dead_tuples = (strcmp(PQgetvalue(res, 0,
> i_retain_dead_tuples), "t") == 0);
>
> With no subscriptions the < 1900 query returns zero rows, PQgetvalue(res, 0,
> ...) returns NULL for a nonexistent row, and the strcmp segfaults (reproduced
> with a small libpq program). Upgrading a v17/v18 cluster that has no
> subscriptions hits it. Shveta's "SELECT false" fix is right.
>
> A few more comments from re-reading v10:
>
> 1) The doc hunk updates this sentence:
>
> - Commit timestamps and origin data are not preserved during
> the upgrade.
> + Commit timestamps are not preserved during the upgrade.
>
> but the twin comment in pg_upgrade.c (~line 222) is left unchanged and needs
> the same edit:
>
> * upgrade. Additionally, commit timestamps and origin data are not
> * preserved during the upgrade. So, even after creating the slot, the
>
> 2) dumpReplicationOrigins:
>
> + appendPQExpBufferStr(buf,
> + "SELECT o.*, os.remote_lsn "
> + "FROM pg_catalog.pg_replication_origin o "
> + "LEFT OUTER JOIN pg_catalog.pg_replication_origin_status os ON
> o.roident = os.local_id ");
>
> No ORDER BY, so the output order is whatever the heap returns; every other
> query in pg_dumpall.c has one. ORDER BY o.roident would keep the dump stable.

+1

>
> 3) Trivia: trailing space in the section header --
>
> + fprintf(OPF, "--\n-- Replication Origins \n--\n\n");
>
> and a stray blank line at the end of replorigin_create_with_id():
>
> + false /* WAL log */);
> + }
> +
> +}
>
> Also, per my earlier mail, the >= 905 / >= 90500 gates can come out now that
> 14d8418083 raised the pg_upgrade floor to v10.
>
> Thanks,
> Rui

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Ewan Young 2026-07-08 04:16:27 Re: Reject ill-formed range bounds histograms in pg_restore_attribute_stats()
Previous Message shveta malik 2026-07-08 04:01:59 Re: [PATCH] Preserve replication origin OIDs in pg_upgrade