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

From Hayato Kuroda (Fujitsu)
Subject RE: Pgoutput not capturing the generated columns
Date
Msg-id TYAPR01MB5692437A3A258ACAC6359C37F54B2@TYAPR01MB5692.jpnprd01.prod.outlook.com
Whole thread Raw
In response to Pgoutput not capturing the generated columns  (Rajendra Kumar Dangwal <dangwalrajendra888@gmail.com>)
Responses Re: Pgoutput not capturing the generated columns
List 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


pgsql-hackers by date:

Previous
From: Yasir
Date:
Subject: Re: Alias of VALUES RTE in explain plan
Next
From: Peter Smith
Date:
Subject: Re: Pgoutput not capturing the generated columns