On Tue, Oct 29, 2024 at 11:19 AM Hayato Kuroda (Fujitsu)
<kuroda.hay...@fujitsu.com> wrote:
>
> 01. fetch_remote_table_info()
>
> `bool *remotegencolpresent` is accessed unconditionally, but it can cause
> crash
> if NULL is passed to the function. Should we add an Assert to verify it?
>
This is a static function being called from just one place, so don't
think this is required.
> 02. fetch_remote_table_info()
>
> ```
> + if (server_version >= 180000)
> + *remotegencolpresent |= DatumGetBool(slot_getattr(slot, 5,
> &isnull));
> +
> ```
>
> Can we add Assert(!isnull) like other parts?
>
> 03. fetch_remote_table_info()
>
> Also, we do not have to reach here once *remotegencolpresent becomes true.
> Based on 02 and 03, how about below?
>
> ```
> if (server_version >= 180000 && !(*remotegencolpresent))
> {
> *remotegencolpresent |=
> DatumGetBool(slot_getattr(slot, 5, &isnull));
> Assert(!isnull);
> }
> ```
>
Yeah, we can follow this suggestion but better to add a comment for the same.
--
With Regards,
Amit Kapila.
