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