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

From Amit Kapila
Subject Re: Pgoutput not capturing the generated columns
Date
Msg-id CAA4eK1Keq7hewXGe4mUHuCzEA5=ZR5wQK0+L5TU+zVFUMrmOFw@mail.gmail.com
Whole thread Raw
In response to RE: Pgoutput not capturing the generated columns  ("Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>)
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.



pgsql-hackers by date:

Previous
From: Peter Smith
Date:
Subject: Re: Pgoutput not capturing the generated columns
Next
From: Bykov Ivan
Date:
Subject: RE: [PoC] Partition path cache