RE: Pgoutput not capturing the generated columns - Mailing 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