RE: Pgoutput not capturing the generated columns

From: "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>
To: 'Shubham Khanna' <khannashubham1197(at)gmail(dot)com>
Cc: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Rajendra Kumar Dangwal <dangwalrajendra888(at)gmail(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>, "euler(at)eulerto(dot)com" <euler(at)eulerto(dot)com>, Peter Smith <smithpb2250(at)gmail(dot)com>
Subject: RE: Pgoutput not capturing the generated columns
Date: 2024-10-29 05:49:38
Message-ID: TYAPR01MB5692437A3A258ACAC6359C37F54B2@TYAPR01MB5692.jpnprd01.prod.outlook.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Dear Shubham,

Thanks for updating the patch! Here are my comments for v44.

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?

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);
}
```

04. pgoutput_column_list_init()

+ if (att->attgenerated)
+ {
+ if (bms_is_member(att->attnum, cols))
+ gencolpresent = true;
+
continue;
+ }

I'm not sure it is correct. Why do you skip the generated column even when it is in
the column list? Also, can you add comments what you want to do?

Best regards,
Hayato Kuroda
FUJITSU LIMITED

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Smith 2024-10-29 06:00:02 Re: Pgoutput not capturing the generated columns
Previous Message Yasir 2024-10-29 05:32:02 Re: Alias of VALUES RTE in explain plan