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
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 |