Re: Pgoutput not capturing the generated columns

Hi Shubham, here are my review comments for patch v19-0001.

======
src/backend/replication/pgoutput/pgoutput.c

1.
  /*
  * Columns included in the publication, or NULL if all columns are
  * included implicitly.  Note that the attnums in this bitmap are not
+ * publication and include_generated_columns option: other reasons should
+ * be checked at user side.  Note that the attnums in this bitmap are not
  * shifted by FirstLowInvalidHeapAttributeNumber.
  */
  Bitmapset  *columns;
You replied [1] "The attached Patches contain all the suggested
changes." but as I previously commented [2, #1], since there is no
change to the interpretation of the 'columns' BMS caused by this
patch, then I expected this comment would be unchanged (i.e. same as
HEAD code). But this fix was missed in v19-0001.

OTOH, if you do think there was a reason to change the comment then
the above is still not good because "are not publication and
include_generated_columns option" wording doesn't make sense.

======
src/test/subscription/t/011_generated.pl

Observation -- I added (in nitpicks diffs) some more comments for
'tab1' (to make all comments consistent with the new tests added). But
when I was doing that I observed that tab1 and tab3 test scenarios are
very similar. It seems only the subscription parameter is not
specified (so 'include_generated_cols' default wll be tested). IIRC
the default for that parameter is "false", so tab1 is not really
testing that properly -- e.g. I thought maybe to test the default
parameter it's better the subscriber-side 'b' should be not-generated?
But doing that would make 'tab1' the same as 'tab2'. Anyway, something
seems amiss -- it seems either something is not tested or is duplicate
tested. Please revisit what the tab1 test intention was and make sure
we are doing the right thing for it...

======
99.
The attached nitpicks diff patch has some tweaked comments.

======
[1] https://www.postgresql.org/message-id/CAHv8Rj%2BR0cj%3Dz1bTMAgQKQWx1EKvkMEnV9QsHGvOqTdnLUQi1A%40mail.gmail.com
[2] https://www.postgresql.org/message-id/CAHut%2BPtVfrbx0jb42LCmS%3D-LcMTtWxm%2BvhaoArkjg7Z0mvuXbg%40mail.gmail.com


Kind Regards,
Peter Smith.
Fujitsu Australia.