On Wed, Nov 29, 2023 at 7:33 AM Hayato Kuroda (Fujitsu) <kuroda.hay...@fujitsu.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.