Re: [PoC] pg_upgrade: allow to upgrade publisher node

On Wed, Sep 20, 2023 at 12:16 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Wed, Sep 20, 2023 at 11:00 AM Hayato Kuroda (Fujitsu)
> <kuroda.hayato@fujitsu.com> wrote:
> >
> > Dear Amit,
>
> +int
> +count_old_cluster_logical_slots(void)
> +{
> + int dbnum;
> + int slot_count = 0;
> +
> + for (dbnum = 0; dbnum < old_cluster.dbarr.ndbs; dbnum++)
> + slot_count += old_cluster.dbarr.dbs[dbnum].slot_arr.nslots;
> +
> + return slot_count;
> +}
>
> In this code, aren't we assuming that 'slot_arr.nslots' will be zero
> for versions <=PG16? On my Windows machine, this value is not zero but
> rather some uninitialized negative value which makes its caller try to
> allocate some undefined memory and fail. I think you need to
> initialize this in get_old_cluster_logical_slot_infos() for lower
> versions.
>

+{ oid => '8046', descr => 'for use by pg_upgrade',
+  proname => 'binary_upgrade_validate_wal_records',
+  prorows => '10', proretset => 't', provolatile => 's', prorettype => 'bool',
+  proargtypes => 'pg_lsn', proallargtypes => '{pg_lsn,bool}',
+  proargmodes => '{i,o}', proargnames => '{start_lsn,is_ok}',
+  prosrc => 'binary_upgrade_validate_wal_records' },

In this many of the fields seem bogus. For example, we don't need
prorows => '10', proretset => 't' for this function. Similarly
proargmodes also look incorrect as we don't have any out parameter.

--
With Regards,
Amit Kapila.