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

On Wed, Nov 29, 2023 at 7:33 AM Hayato Kuroda (Fujitsu)
<kuroda(dot)hayato(at)fujitsu(dot)com> wrote:
>
> > > Actually, we do not expect that it won't input NULL. IIUC all of slots have
> > > slot_name, and subquery uses its name. But will it be kept forever? I think we
> > > can avoid any risk.
> > >
> > > > I've not tested it yet but even if it returns NULL, perhaps
> > > > get_old_cluster_logical_slot_infos() would still set curr->caught_up
> > > > to false, no?
> > >
> > > Hmm. I checked the C99 specification [1] of strcmp, but it does not define the
> > > case when the NULL is input. So it depends implementation.
> >
> > I think PQgetvalue() returns an empty string if the result value is null.
> >
>
> Oh, you are right... I found below paragraph from [1].
>
> > An empty string is returned if the field value is null. See PQgetisnull to distinguish
> > null values from empty-string values.
>
> So I agree what you said - current code can accept NULL.
> But still not sure the error message is really good or not.
> If we regard an empty string as false, the slot which has empty name will be reported like:
> "The slot \"\" has not consumed the WAL yet" in check_old_cluster_for_valid_slots().
> Isn't it inappropriate?
>

I see your point that giving a better message (which would tell the
actual problem) to the user in this case also has a value. OTOH, as
you said, this case won't happen in practical scenarios, so I am fine
either way with a slight tilt toward retaining a better error message
(aka the current way). Sawada-San/Bharath, do you have any suggestions
on this?

--
With Regards,
Amit Kapila.