Re: Pgoutput not capturing the generated columns - Mailing list pgsql-hackers

On Tue, Oct 29, 2024 at 11:19 AM Hayato Kuroda (Fujitsu)
<kuroda.hayato@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.